Closed Bug 989503 Opened 6 years ago Closed 6 years ago

TypeObject property set left in a bad state after OOM

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: jorendorff, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Setting NI from Brian.
Flags: needinfo?(bhackett1024)
Attached patch patchSplinter Review
This should fix this bug, refactors addProperty so that the fallible stuff happens before the hash table is modified.
Assignee: general → bhackett1024
Attachment #8399460 - Flags: review?(jdemooij)
Flags: needinfo?(bhackett1024)
NULL deref, not s-s.
Group: javascript-core-security
Blocks: 988953
Attachment #8399460 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/013c4b1e63d2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.