626064 - Invalid read near stack in js::MarkRangeConservatively

Status: RESOLVED WORKSFORME

(Core :: JavaScript: GC, defect)

Description

[Bob Clary [:bc] (inactive)]

Testing cross fuzz on Mac with valgrind I saw this during startup before the cross fuzz page loaded. I haven't tried to reproduce yet. This was with a build from 2011-01-14

==66122== Invalid read of size 4
==66122==    at 0x65D9E8A: js::MarkRangeConservatively(JSTracer*, unsigned long const*, unsigned long const*) (jsgc.cpp:743)
==66122==    by 0x65D9F4F: js::MarkThreadDataConservatively(JSTracer*, JSThreadData*) (jsgc.cpp:760)
==66122==    by 0x65DA018: js::MarkConservativeStackRoots(JSTracer*) (jsgc.cpp:793)
==66122==    by 0x65DA0C9: js::MarkRuntime(JSTracer*) (jsgc.cpp:1646)
==66122==    by 0x65DA66D: MarkAndSweep(JSContext*, JSGCInvocationKind) (jsgc.cpp:2408)
==66122==    by 0x65DAD23: GCUntilDone(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2734)
==66122==    by 0x65DAF39: js_GC(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2805)
==66122==    by 0x6552E23: JS_GC (jsapi.cpp:2545)
==66122==    by 0x5A9B87B: nsXPConnect::Collect() (nsXPConnect.cpp:407)
==66122==    by 0x5A94DE7: nsXPConnect::GarbageCollect() (nsXPConnect.cpp:415)
==66122==    by 0x551B230: nsJSContext::CC(nsICycleCollectorListener*) (nsJSEnvironment.cpp:3632)
==66122==    by 0x551B2ED: nsJSContext::IntervalCC() (nsJSEnvironment.cpp:3737)
==66122==  Address 0xbfffa200 is not stack'd, malloc'd or (recently) free'd

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Did you build without --enable-valgrind?

Comment 2

[Bob Clary [:bc] (inactive)]

Adding configure options from /work/mozilla/builds/hg.mozilla.org/sisyphus/mozconfig/2.0.0/mozconfig-firefox-darwin-intel32-debug:
  --with-macos-sdk=/Developer/SDKs/MacOSX10.5.sdk
  --enable-application=browser
  --enable-debug
  --disable-optimize
  --enable-debug-symbols=-gdwarf-2
  --enable-libxul
  --disable-install-strip
  --enable-tests
  --enable-logrefcnt
  --with-valgrind=yes
  --enable-valgrind=yes
  --enable-accessibility
  --disable-installer
  --enable-official-branding

Comment 3

[Bob Clary [:bc] (inactive)]

This wasn't reproducible when loading a blank home page but was reproducible when loading my modified cross fuzz. Note the command used was:

valgrind --dsymutil=yes --tool=memcheck --smc-check=all --trace-children=yes
--track-origins=yes ...

Comment 4

[Jesse Ruderman]

I'm seeing this with my fuzzer, too.

Comment 5

[Julian Seward [:jseward]]

(In reply to comment #1)
> Did you build without --enable-valgrind?

That stops it from complaining that some of the stuff it comes
across on the stack is uninitialised.  But this is different --
a complaint about reading at an address it shouldn't.

I see this routinely on x64-{linux,darwin} runs and assumed it
was some wierdness to do with the ELF-amd64 ABI, which allows a
128-byte area below %rsp to be accessible.  But you're saying
it occurs on a 32-bit process?  Then it's got to be something else
since ELF-x86 doesn't allow any bytes below %esp to be accessible.

Comment 6

[Julian Seward [:jseward]]

(In reply to comment #4)
> I'm seeing this with my fuzzer, too.

Do you have a simple test case?

Comment 7

[Jesse Ruderman]

I don't have a testcase, sorry.

Comment 8

[David Mandelin [:dmandelin]]

This is kind of interesting, but I don't think it's security-sensitive or an immediate problem. The worst it would do is cause us to mark something that's garbage, delaying it's collection by 1 GC cycle. Correct me if I'm wrong.

I would like to know what's happening here eventually.

Comment 9

[Bob Clary [:bc] (inactive)]

64bit linux valgrind unittests I get the following and similar:

==7425== Invalid read of size 8
==7425==    at 0x6E77F51: js::MarkRangeConservatively(JSTracer*, unsigned long const*, unsigned long const*) (jsgc.cpp:743)
==7425==    by 0x6E78015: js::MarkThreadDataConservatively(JSTracer*, JSThreadData*) (jsgc.cpp:760)
==7425==    by 0x6E78149: js::MarkConservativeStackRoots(JSTracer*) (jsgc.cpp:793)
==7425==    by 0x6E79D4C: js::MarkRuntime(JSTracer*) (jsgc.cpp:1650)
==7425==    by 0x6E7B5A4: MarkAndSweep(JSContext*, JSGCInvocationKind) (jsgc.cpp:2407)
==7425==    by 0x6E7C12C: GCUntilDone(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2733)
==7425==    by 0x6E7C34A: js_GC(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2804)
==7425==    by 0x6DECA72: JS_GC (jsapi.cpp:2547)
==7425==    by 0x62F0335: nsXPConnect::Collect() (nsXPConnect.cpp:407)
==7425==    by 0x62F03A4: nsXPConnect::GarbageCollect() (nsXPConnect.cpp:415)
==7425==    by 0x5D53048: nsJSContext::CC(nsICycleCollectorListener*) (nsJSEnvironment.cpp:3630)
==7425==    by 0x5D53255: nsJSContext::IntervalCC() (nsJSEnvironment.cpp:3735)
==7425==  Address 0x7feff9200 is not stack'd, malloc'd or (recently) free'd

during 

mochitest-chrome:
chrome://mochitests/content/chrome/content/events/test/test_bug602962.xul

mochitest-plain: chunk 2 of 10:
/tests/content/events/test/test_bug574663.html

mochitest-plain: chunk 3 of 10:
/tests/content/xbl/test/test_bug310107.html

mochitest-plain: chunk 8 of 10:
/tests/dom/tests/mochitest/general/test_497898.html

Comment 10

[Bob Clary [:bc] (inactive)]

(In reply to comment #9)

I was wrong in thinking this was new. Several of these have repeated and may be of interest.

12/3 and 1/13 mochitest-plain:
/tests/content/events/test/test_bug574663.html

1/14 mochitest-plain:
/tests/content/xbl/test/test_bug310107.html

12/10 and 12/29 mochitest-plain:
/tests/embedding/test/test_window_open_units.html

12/16 and 1/10 mochitest-chrome
chrome://mochitests/content/chrome/content/events/test/test_bug602962.xul

12/20 and 1/13 mochitest-plain
/tests/content/events/test/test_bug574663.html

12/28 and 1/20 mochitest-plain
/tests/dom/tests/mochitest/general/test_497898.html

Comment 11

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stdout output

There is a sort-of reliable way of getting this. When running Valgrind with Firefox compiled with the mozconfig below, head to:

http://www.html5canvastutorials.com/advanced/html5-canvas-pixel-mouseover-tutorial/

Mouseover a little, a slow script dialog will appear. It is this dialog that triggers the invalid read.

Initially I got this invalid read when activating Mozmill in Firefox under Valgrind. I also hit the assertion in bug 636889:

Assertion failure: !cx->iterValue.isMagic(JS_NO_ITER_VALUE)

but appearance of the assert isn't reliable.

CLI command:
LD_LIBRARY_PATH=. valgrind --smc-check=all --db-attach=yes ./firefox-bin

source changeset e56ecd8b3a68 off m-c
===

. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/valg-obj-ff-dbg-64-mc
mk_add_options MOZ_MAKE_FLAGS="-s -j6"
ac_add_options --enable-debug
ac_add_options --enable-optimize="-O"
#ac_add_options --enable-tests

ac_add_options --disable-jemalloc
ac_add_options --enable-valgrind

ac_add_options --with-ccache

Comment 12

[Matthew Gregan [:kinetik]]

I can reproduce this reliably on amd64 using the testcase in bug 686520.

Comment 13

[Igor Bukanov]

(In reply to Matthew Gregan (:kinetik) from comment #12)
> I can reproduce this reliably on amd64 using the testcase in bug 686520.

You mean you can reproduce the valgrind warning, right?

Comment 14

[Matthew Gregan [:kinetik]]

Right.  I wasn't sure if it was useful to report given comment 5.

Comment 15

[Igor Bukanov]

(In reply to Matthew Gregan (:kinetik) from comment #14)
> Right.  I wasn't sure if it was useful to report given comment 5.

I am not sure about the consequences of this valgrind-reported invalid read. Can it result in accessing a page that is unmapped or read-protected? If yes, then we definitely need to avoid those reads. But if not, a simplest solution would be to silence valgrind warnings.

Comment 16

[Julian Seward [:jseward]]

I think we should figure out what's going on.  Without that there's no
way to assess the seriousness.

In both traces above the bad address is of the form x200.  I don't
know if that's significant.  It's not a page boundary.

==66122==  Address 0xbfffa200 is not stack'd, malloc'd or (recently) free'd
==7425==  Address 0x7feff9200 is not stack'd, malloc'd or (recently) free'd

Comment 17

[Andrew McCreight [:mccr8]]

We no longer use conservative marking.