Closed Bug 563243 Opened 10 years ago Closed 10 years ago

(64-bit) Invalid read / write when testcase is run in valgrind

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking1.9.2 --- .14+
status1.9.2 --- .14-fixed
blocking1.9.1 --- .17+
status1.9.1 --- .17-fixed

People

(Reporter: gkw, Assigned: mrbkap)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical?][critsmash:resolved] fixed-in-tracemonkey)

Attachments

(4 files, 1 obsolete file)

Attached file valgrind log
(function () {
    for (a in (function () {
        yield Array.reduce()
    })()) function () {}
})()

causes invalid reads and writes when run in a 1.9.2 64-bit Linux shell without -j on 1.9.2 tip. (Pass into 1.9.2 shell as a CLI argument, e.g. ./js a.js)

A valgrind log is attached. This does not seem to occur with TM tip. Setting s-s because I'm not sure how bad this is.
This shows up as a crash when jsfunfuzz is being run.
Keywords: crash
Group: core-security
Tested on Ubuntu Linux 10.04 AMD64 on this 1.9.2 changeset:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d1aab61eb130
Whiteboard: [sg:critical?]
Attached patch Proposed fixSplinter Review
We haven't been able to assume anything about vp[2 + n] for n >= argc since 2006 or so... We don't actually fill missing args for fast natives.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #443173 - Flags: review?(jorendorff)
Note: the changes to the slow version weren't necessary, but the two versions should probably match each other in behavior.
Attachment #443173 - Flags: review?(jorendorff) → review+
Keywords: valgrind
Whiteboard: [sg:critical?] → [sg:critical?][critsmash:patch]
When did this regress? Does it affect any of our shipping branches?
This is a regression from bug 412296. It affects all active branches (note: it does *not* affect 1.9.0).
Blocks: 412296
(In reply to comment #7)
> This is a regression from bug 412296. It affects all active branches (note: it
> does *not* affect 1.9.0).

Strange - might just be me, but I couldn't seem to reproduce on TM tip, as per comment 0.
http://hg.mozilla.org/tracemonkey/rev/0f5867192284

Yeah, I saw that, but I've definitely reproduced it on all branches. Not sure what you're seeing.
Whiteboard: [sg:critical?][critsmash:patch] → [sg:critical?][critsmash:patch] fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/0f5867192284
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?][critsmash:patch] fixed-in-tracemonkey → [sg:critical?][critsmash:resolved] fixed-in-tracemonkey
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Blocks: 563133
Fixes bug 563133 for branches
blocking1.9.1: ? → .17+
blocking1.9.2: ? → .14+
Duplicate of this bug: 563133
Attached patch fix for 192 (obsolete) — Splinter Review
The patch is a trivial backport of the tm patch. Here is a plain diff between tm and this patch to confirm this:

24c24
<      if (!js_ComputeThis(cx, vp + 2))
---
>      if (!js_ComputeThis(cx, JS_FALSE, vp + 2))
59,60c59,60
< @@ -4333,15 +4325,8 @@ js_generic_native_method_dispatcher(JSCo
<      js_GetTopStackFrame(cx)->thisv = argv[-1];
---
> @@ -4463,15 +4455,8 @@ js_generic_native_method_dispatcher(JSCo
>      js_GetTopStackFrame(cx)->thisp = JSVAL_TO_OBJECT(argv[-1]);
Attachment #505425 - Flags: approval1.9.2.14?
The previous attachment had an unrelated diff.
Attachment #505425 - Attachment is obsolete: true
Attachment #505425 - Flags: approval1.9.2.14?
Attachment #505426 - Flags: approval1.9.2.14?
Comment on attachment 505426 [details] [diff] [review]
fix for 192 анд 191

The patch applies to 191 as-is
Attachment #505426 - Attachment description: fix for 192 for real → fix for 192 анд 191
Attachment #505426 - Flags: approval1.9.1.17?
Comment on attachment 505426 [details] [diff] [review]
fix for 192 анд 191

Approved for 1.9.2.14 and 1.9.1.17. Please land this as soon as possible!
Attachment #505426 - Flags: approval1.9.2.14?
Attachment #505426 - Flags: approval1.9.2.14+
Attachment #505426 - Flags: approval1.9.1.17?
Attachment #505426 - Flags: approval1.9.1.17+
Group: core-security
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.