1308556 - AddressSanitizer: heap-use-after-free [@ bool BaselineStackBuilder::write<JS::Value>] with READ of size 8

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision ea104eeb14cc (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2, run with --fuzzing-safe --thread-count=2 --ion-eager):

egc = 80;
(function(global) {
  dump = global.print;
})(this);
var gTestcases = new Array();
var gTc = gTestcases.length;
function TestCase(n, d, e, a) {
  this.description = d;
  ({}).constructor.defineProperty(gTestcases, gTc++, { value: this, });
}
TestCase.prototype.dump = function () {
  dump('description: ' + toPrinted(this.description) + ' ' );
};
function toPrinted(value) {
  value = String(value);
  value = value.replace(/\\n/g, 'NL')
               .replace(/[^\x20-\x7E]+/g, escapeString);
}
function escapeString (str) {}
function jsTestDriverEnd() {
  for (var i = 0; i < gTestcases.length; i++)
    gTestcases[i].dump();
}
var SECTION = "15.5.4.6-2";
for ( var u = 0x00A1, i = 0; u <= 0x00FF; u++, i++ ) {
  new TestCase(   
    SECTION,
    "TEST_STRING.indexOf( " + String.fromCharCode(u) + " )"
  );
}
gczeal(9,egc)
jsTestDriverEnd();



Backtrace:

==28494==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000028608 at pc 0x000001f446d6 bp 0x7ffde79daeb0 sp 0x7ffde79daea8
READ of size 8 at 0x619000028608 thread T0
    #0 0x1f446d5 in bool BaselineStackBuilder::write<JS::Value>(JS::Value const&) js/src/jit/BaselineBailouts.cpp:203:9
    #1 0x1f446d5 in BaselineStackBuilder::writeValue(JS::Value const&, char const*) js/src/jit/BaselineBailouts.cpp:236
    #2 0x1f446d5 in InitFromBailout(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JSFunction*>, JS::Handle<JSScript*>, js::jit::IonScript*, js::jit::SnapshotIterator&, bool, BaselineStackBuilder&, JS::MutableHandle<JS::GCVector<JS::Value, 0ul, js::TempAllocPolicy> >, JS::MutableHandle<JSFunction*>, unsigned char**, js::jit::ExceptionBailoutInfo const*) js/src/jit/BaselineBailouts.cpp:1279
    #3 0x1f446d5 in js::jit::BailoutIonToBaseline(JSContext*, js::jit::JitActivation*, js::jit::JitFrameIterator&, bool, js::jit::BaselineBailoutInfo**, js::jit::ExceptionBailoutInfo const*) js/src/jit/BaselineBailouts.cpp:1572
    #4 0x1f2c9fa in js::jit::InvalidationBailout(js::jit::InvalidationBailoutStack*, unsigned long*, js::jit::BaselineBailoutInfo**) js/src/jit/Bailouts.cpp:129:23

0x619000028608 is located 136 bytes inside of 1024-byte region [0x619000028580,0x619000028980)
freed by thread T0 here:
    #0 0x511070 in __interceptor_free /srv/repos/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38
    #1 0x1f3d772 in js_free(void*) /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/clang/sanitizer/asan/type/opt/dist/include/js/Utility.h:257:5
    #2 0x1f3d772 in BaselineStackBuilder::enlarge() js/src/jit/BaselineBailouts.cpp:149
    #3 0x1f3d772 in BaselineStackBuilder::subtract(unsigned long, char const*) js/src/jit/BaselineBailouts.cpp:182
    #4 0x1f3d772 in bool BaselineStackBuilder::write<JS::Value>(JS::Value const&) js/src/jit/BaselineBailouts.cpp:201
    #5 0x1f3d772 in BaselineStackBuilder::writeValue(JS::Value const&, char const*) js/src/jit/BaselineBailouts.cpp:236
    #6 0x1f3d772 in InitFromBailout(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JSFunction*>, JS::Handle<JSScript*>, js::jit::IonScript*, js::jit::SnapshotIterator&, bool, BaselineStackBuilder&, JS::MutableHandle<JS::GCVector<JS::Value, 0ul, js::TempAllocPolicy> >, JS::MutableHandle<JSFunction*>, unsigned char**, js::jit::ExceptionBailoutInfo const*) js/src/jit/BaselineBailouts.cpp:1279
    #7 0x1f3d772 in js::jit::BailoutIonToBaseline(JSContext*, js::jit::JitActivation*, js::jit::JitFrameIterator&, bool, js::jit::BaselineBailoutInfo**, js::jit::ExceptionBailoutInfo const*) js/src/jit/BaselineBailouts.cpp:1572
    #8 0x1f2c9fa in js::jit::InvalidationBailout(js::jit::InvalidationBailoutStack*, unsigned long*, js::jit::BaselineBailoutInfo**) js/src/jit/Bailouts.cpp:129:23

previously allocated by thread T0 here:
    #0 0x51152c in __interceptor_calloc /srv/repos/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:66
    #1 0x1f35385 in js_calloc(unsigned long) /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/clang/sanitizer/asan/type/opt/dist/include/js/Utility.h:235:12
    #2 0x1f35385 in BaselineStackBuilder::init() js/src/jit/BaselineBailouts.cpp:117
    #3 0x1f35385 in js::jit::BailoutIonToBaseline(JSContext*, js::jit::JitActivation*, js::jit::JitFrameIterator&, bool, js::jit::BaselineBailoutInfo**, js::jit::ExceptionBailoutInfo const*) js/src/jit/BaselineBailouts.cpp:1508
    #4 0x1f2c9fa in js::jit::InvalidationBailout(js::jit::InvalidationBailoutStack*, unsigned long*, js::jit::BaselineBailoutInfo**) js/src/jit/Bailouts.cpp:129:23

SUMMARY: AddressSanitizer: heap-use-after-free js/src/jit/BaselineBailouts.cpp:203:9 in bool BaselineStackBuilder::write<JS::Value>(JS::Value const&)
Shadow bytes around the buggy address:
  0x0c327fffd0a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffd0b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c327fffd0c0: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fffd0d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
==28494==ABORTING


Testcase is intermittent, please assign manually during triage. Marking s-s and sec-critical due to use-after-free.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

This is a (very subtle) regression from bug 1290337. It changed BaselineStackBuilder::writeValue to take |const value&| instead of |Value|, so the following code now had a memory hazard (blFrame is allocated in the same buffer, writeValue can resize that buffer):

    if (!builder.writeValue(*blFrame->valueSlot(i), "ArgVal"))
        return false;

The patch fixes this by copying the Value into a local, cleans up the inferface of BufferPointer<T> a bit, adds an assert to catch such cases in the future, and adds some MOZ_MUST_USE annotations and missing OOM checks.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

Comment 3

[Tooru Fujisawa [:arai]]

Comment on attachment 8799382 [details] [diff] [review]
Patch

Review of attachment 8799382 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for catching this!

Comment 4

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b987bb9ad1a19f04b7223cca8aa098eb5f7782ed

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b987bb9ad1a1