Crash [@ js::jit::AssemblerBufferWithConstantPool]

RESOLVED DUPLICATE of bug 1026919

Status

()

defect
--
critical
RESOLVED DUPLICATE of bug 1026919
6 years ago
3 years ago

People

(Reporter: gkw, Assigned: dougc)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
ARM
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [jsbugmon:], crash signature)

Attachments

(2 attachments)

Posted file stack
for (var f in []) {}
function testMathyFunction(f, inputs) {
    var results = [];
    for (var j = 0; j < inputs.length; ++j) {
        for (var k = 0; k < inputs.length; ++k) {
            try {
                results.push(f(inputs[j], inputs[k]));
            } catch (e) {
                results.push(e);
            }
        }
    }
    uneval(results);
}
s = newGlobal();
s["testMathyFunction"] = this["testMathyFunction"].bind(this);
function f(code) {
    try {
        evalcx(code, s);
    } catch (e) {}
};
f("\
    (function(){})();\
    testMathyFunction([\
        Number.MAX_VALUE,\
        Number.MIN_VALUE,\
        Math.PI,\
    ]);\
");
f("\
    m = (function(){});\
    testMathyFunction(a, [\
        Number.MIN_VALUE,,0xffffffff,,1/0,0x80000000,0xf,\
        -0x100000000,-0,0x100000000,-1/0,0x100000001,\
        Number.MAX_VALUE,0/0,,Math.PI,0x100000001,\
    ]);\
")

crashes js debug shell on m-c changeset 77b5c6edfe96 with --ion-eager at js::jit::AssemblerBufferWithConstantPool

My configure flags are:

CC="gcc -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" AR=ar CXX="g++ -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" sh ./configure --target=arm-linux-gnueabi --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/5959fac49afe
user:        Douglas Crosher
date:        Sun Nov 10 04:20:22 2013 +1100
summary:     Bug 906964 - ARM: leave some head-room in the double pools to help avoid bailing out which causes compilation failure for asm.js code. r=mjrosenb

s-s because I'm not sure what this testcase is trying to do. Douglas, is bug 906964 a likely regressor?
Flags: needinfo?(dtc-moz)
This is a possible fix, but I am not familiar enough with the code to be sure.  Let me try to reproduce it and test this.  Marty might be working on the buffer pools and be familiar with this?

The bisected patch was just a hack, but was not expected to cause problems.  It would be an easy decision to back it out, but let me explore it a little more first.
Flags: needinfo?(dtc-moz) → needinfo?(mrosenberg)
(In reply to Douglas Crosher [:dougc] from comment #1)
> Created attachment 8340754 [details] [diff] [review]
> Guard against a null perforatedNode
> 
> This is a possible fix, but I am not familiar enough with the code to be
> sure.  Let me try to reproduce it and test this.  Marty might be working on
> the buffer pools and be familiar with this?
> 
> The bisected patch was just a hack, but was not expected to cause problems. 
> It would be an easy decision to back it out, but let me explore it a little
> more first.

I can reproduce the crash and this patch alone is not adequate to fix the problem, so perhaps it would be best to backout the hack patch in bug 906964 and wait for a comprehensive solution.   This patch might still be part of a solution, and it might be that the patch in bug 906964 just changed the packing of the pools and tickled the failure here.
Flags: needinfo?(mrosenberg)
(In reply to Douglas Crosher [:dougc] from comment #2)
> I can reproduce the crash and this patch alone is not adequate to fix the
> problem, so perhaps it would be best to backout the hack patch in bug 906964
> and wait for a comprehensive solution.   This patch might still be part of a
> solution, and it might be that the patch in bug 906964 just changed the
> packing of the pools and tickled the failure here.

Can we find out if that's the case?
Flags: needinfo?(dtc-moz)
(In reply to Jan de Mooij [:jandem] from comment #3)
> (In reply to Douglas Crosher [:dougc] from comment #2)
> > I can reproduce the crash and this patch alone is not adequate to fix the
> > problem, so perhaps it would be best to backout the hack patch in bug 906964
> > and wait for a comprehensive solution.   This patch might still be part of a
> > solution, and it might be that the patch in bug 906964 just changed the
> > packing of the pools and tickled the failure here.
> 
> Can we find out if that's the case?

I understand Marty is working of the buffer pools so I will defer to him.
Flags: needinfo?(dtc-moz)
Flags: needinfo?(mrosenberg)
Do you know how bad this problem is?  Is it just a null deref or something worse?
(In reply to Jan de Mooij [:jandem] from comment #3)
> (In reply to Douglas Crosher [:dougc] from comment #2)
> > I can reproduce the crash and this patch alone is not adequate to fix the
> > problem, so perhaps it would be best to backout the hack patch in bug 906964
> > and wait for a comprehensive solution.   This patch might still be part of a
> > solution, and it might be that the patch in bug 906964 just changed the
> > packing of the pools and tickled the failure here.
> 
> Can we find out if that's the case?

The problem should be analysed, but the patch in bug 906964 is being backed out and it might be informative to see if fuzzing can still tickle this crash.
Assignee: general → nobody
QA Contact: general
> to see if fuzzing can still tickle this crash.

This no longer crashes on m-c rev c8d5a871ae32 compiled with the patch for bug 952810. FIXED by the backout?
Flags: needinfo?(dtc-moz)
I suggest re-evaluating this bug while checking and testing the changes in bug 760642, before closing it.
Depends on: 760642
Flags: needinfo?(dtc-moz)
assigning to Gary for the suggested retesting
Assignee: nobody → gary
Group: javascript-core-security
Flags: needinfo?(mrosenberg)
Keywords: sec-audit
Bumping to :dougc, as he's working on rewriting the assembly buffer. Please feel free to change this as necessary.
Assignee: gary → dtc-moz
Depends on: 972710
Whiteboard: [jsbugmon:update]
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Group: javascript-core-security
:dougc, is bug 1026919 a likely further fix for this?
Flags: needinfo?(dtc-moz)
(In reply to Gary Kwong [:gkw] [:nth10sd] PTO Jul 19 to 27 from comment #12)
> :dougc, is bug 1026919 a likely further fix for this?

Yes, this issue looks resolved by bug 1026919. The failing line is still there, but the perforatedNode, now named perforatedSlice, is obtain just a few lines above and I can not see how it could be null or invalid.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(dtc-moz)
Resolution: --- → DUPLICATE
Duplicate of bug: 1026919
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.