807156 - Crash at illegal address with Bus Error

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 71290adea0c1 (run with --ion-eager):


function assertEqArray(actual, expected) {
    for (var i = 0; i < actual.length; ++i) {
        if (actual[i] != expected[i]) {}
    }
}
assertEqArray(
	/(?:(?:a*?(")(c)")?)*/.exec('"c"'), [ '"c"', '"', "c" ]
);

Comment 1

[Christian Holler (:decoder)]

No trace available here, the crash seems to happen already when the pc is on an illegal position. Marking sec-critical.

The actual crash is a bus error also in the emulation VM, so it's not a qemu userspace problem:

qemu: unhandled CPU exception 0x7 - aborting
R00=00000002 R01=f5c0d050 R02=f5c0d050 R03=f5c11150
R04=f5d00230 R05=ffffff85 R06=00000000 R07=f5d00230
R08=00000001 R09=ffffff81 R10=00000002 R11=00000000
R12=f5c16508 R13=f6fdde1c R14=f5c16508 R15=f5af07c4
PSR=20070010 --C- A usr32
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted

Comment 2

[Marty Rosenberg [:mjrosenb]]

Additional info:
this is with a debug build, not too sure if that was mentioned elsewhere, but I am bad at looking.

running this on real hardware results in a Trace/breakpoint trap rather than a SIGBUS.  Quickly looking at the actual location of the crash shows that it is one of the runtime asserts that we add to ensure that the stack is aligned.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

If this is an Ion bug it will only affect Firefox 18 and higher.

Comment 4

[Marty Rosenberg [:mjrosenb]]

Attachment: /home/mrosenberg/patches/stackAlign-r0.patch

It looks like it'll be fine if the stack isn't aligned.

Comment 5

[Marty Rosenberg [:mjrosenb]]

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/3136d5ea2399

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3136d5ea2399

Comment 7

[bhavana bajaj [:bajaj]]

(In reply to Marty Rosenberg [:mjrosenb] from comment #5)
> Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/3136d5ea2399

Please uplift the patch once we are comfortable with the bake-time/testing on m-c.Thanks !

Comment 8

[Robert Kaiser]

Marty, I guess we had enough baking time for this now, did we get enough testing out of this on 19/20 that we can possibly uplift to 18 (Beta)?

Comment 9

[Marty Rosenberg [:mjrosenb]]

my bad.  This was only half the patch.  I never qrefreshed the setter case, so that never actually landed.
Here is the second half of it
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5582c77a06
Since the patch is just removing a check, I can't imagine that this introduces any bugs whatsoever.

Comment 10

[Andrew McCreight [:mccr8]]

So this is essentially bogus debug-only assertion that you just fixed by removing? In that case we can probably remove the security-sensitive flag on it and bug 810996.

Comment 12

[Marty Rosenberg [:mjrosenb]]

yes.  I was initially afraid that the assert wasn't actually bogus, and bad things would happen without it, but it was unlikely to begin with, and testing seems to have confirmed that.

Comment 13

[Ed Morley [:emorley]]

(In reply to Marty Rosenberg [:mjrosenb] from comment #9)
> my bad.  This was only half the patch.  I never qrefreshed the setter case,
> so that never actually landed.
> Here is the second half of it
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5582c77a06
> Since the patch is just removing a check, I can't imagine that this
> introduces any bugs whatsoever.

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

This part is fixed on mozilla20 only; the first part mozilla20 and mozilla19.

Comment 14

[Alex Keybl [:akeybl]]

No longer a security issue, no need to continue tracking.