989503 - TypeObject property set left in a bad state after OOM

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jason Orendorff [:jorendorff]]

Discovered by decoder. I can reproduce in a Mac debug build of rev 0a0ef0583c2a with the following command line, but the number that's 663 for me was 668 for decoder. YMMV.

$JS -e 'const libdir = "js/src/jit-test/lib/";' -f js/src/jit-test/lib//prolog.js -e 'oomAfterAllocations(663);' -f js/src/jit-test/tests/ion/bug735869.js

Segmentation fault: 11

The bug is in TypeObject::getProperty:

    ...
    uint32_t propertyCount = basePropertyCount();
    Property **pprop = HashSetInsert<jsid,Property,Property>
        (cx->typeLifoAlloc(), propertySet, propertyCount, id);
    if (!pprop) {
        markUnknown(cx);
        return nullptr;
    }

    JS_ASSERT(!*pprop);

    setBasePropertyCount(propertyCount);
    if (!addProperty(cx, id, pprop)) {
        markUnknown(cx);
        return nullptr;
    }
    ...

When addProperty fails, we've already called HashSetInsert and setBasePropertyCount. That leaves the hash set with an entry that's null. The next time we try to look something up, we crash at null.

I can't trivially fix because both addProperty and HashSetInsert are fallible and have side effects -- whichever one's harder to roll back needs to go second...

Not exploitable in the case I'm hitting. I think crashing at null is the worst that can happen here, but with low confidence. Unhide if you can confirm.

Comment 1

[Jan de Mooij [:jandem]]

Setting NI from Brian.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: patch

This should fix this bug, refactors addProperty so that the fallible stuff happens before the hash table is modified.

Comment 3

[Brian Hackett [Laid off!]]

NULL deref, not s-s.

Comment 4

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/013c4b1e63d2

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/013c4b1e63d2