584653 - Non-canonical NaN from 64-bit leopard's asin() causes "Assertion failure: l.asBits <= JSVAL_SHIFTED_TAG_MAX_DOUBLE"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

In a 64-bit shell built from moo, this testcase triggers an assertion failure.  Is asin returning a nonstandard NaN, perhaps?

-Math.asin(4);

Assertion failure: l.asBits <= JSVAL_SHIFTED_TAG_MAX_DOUBLE, at ../jsval.h:533

Comment 1

[Sean Stangl [:sstangl]]

I cannot reproduce locally.

Comment 2

[Jesse Ruderman]

Still happens with moo rev 6347cf00d3ab.  I'm using Mac OS X 10.5.

Comment 3

[Luke Wagner [:luke]]

(In reply to comment #0)
> Is asin returning a nonstandard NaN, perhaps?

I don't think so; if it was, this would reproduce on TM.

Comment 4

[Jesse Ruderman]

I can reproduce on TM (rev 35fd077aad03).

Comment 5

[Luke Wagner [:luke]]

Wow.  This is lame.  asin is returning a non-canonical nan, but only on OS X 10.5 64-bit builds.  Not 10.6, and not 32-bit 10.5.  Just 32-bit 10.5.

Comment 6

[Luke Wagner [:luke]]

So I filed bug 584809 to, based on platform, canonicalize what comes out of math libraries.

Comment 7

[Igor Bukanov]

making this closed as it may hurt users of nighties.

Comment 8

[Luke Wagner [:luke]]

Do we have 64-bit OS X 10.5 nightlies?

Comment 9

[Igor Bukanov]

(In reply to comment #8)
> Do we have 64-bit OS X 10.5 nightlies?

I meant users of any TM or MC build like myself ;)

Comment 10

[Brendan Eich [:brendan]]

I confirmed with [e-mail] that 10.6 fixes this, and he says other libm functions are ok.

My first reaction is to ask: must we penalize all our Mac OS X builds on account of only one bug in one transcendental implementation in OS X 10.5? I hope not!

But good news is that ggaren dug deeper:

ggaren: so, 10.5 does indeed return a signaling nan for acos and asin (but nothing else, it seems)
[3:58pm] ggaren: however, the bit pattern seems innocuous:
[3:58pm] ggaren: (gdb) x/2x &acos_
[3:58pm] ggaren: 0xbffff4e0:0x000000220x7ff80000
[3:58pm] ggaren: (gdb) x/2x &asin_
[3:58pm] ggaren: 0xbffff4e8:0x000000220x7ff80000
[3:59pm] ggaren: (that's two words, written backwards due to little endianness)
[3:59pm] ggaren: so, the high word is the standard NaN signature
[3:59pm] ggaren: it's only the low word that's goofy
[4:01pm] ggaren: technically, a 64bit client of webkit on 10.5 might get into trouble
[4:01pm] ggaren: (safari is 32bit on 10.5)
brendaneich: can i cite you in the (restricted) bug?
[4:05pm] ggaren: sure
[4:05pm] brendaneich: thx, still scary but if the low bits are always 220 (are they?) then ok
[4:05pm] ggaren: i didn't test exhaustively -- only 2.0 and 4.0
[4:06pm] brendaneich: same bits?
[4:06pm] ggaren: yup
[4:06pm] brendaneich: thx
[4:06pm] ggaren: 0...220

Now I'm even less concerned. We can jsapi-test for this too and bail if we get any other bits.

/be

Comment 11

[Brendan Eich [:brendan]]

[4:07pm] ggaren: oops -- i mean 0...22
[4:07pm] brendaneich: right -- 0x
[4:07pm] ggaren: (the following 0 belongs to the 0x of the next word, so ignore it)
[4:07pm] brendaneich: i goofed that above, np

/be

Comment 12

[Luke Wagner [:luke]]

Attachment: ssshhhh

Thanks Brendan!  That is really useful information.  Yeah, if only the low bits are set then we are safe:
 - on 32-bit, we only look at the high 32-bits, so non-canonical nans with only the low 32 bits set will still look like and be treated as nans.
 - on 64-bit, we look at the whole 64-bit value when deciding whether something is a double.  However, there is a dead space between the canonical nan 0xfff0000000000000 and the first tagged value (the int32 0) 0xfff1000000000000, so we can just raise this "max valid double" constant to 0xfff00000ffffffff so that, as with the 32-bit case, non-canonical nans with only the low bits set will be treated as nans.

I'm not who I should

Comment 13

[Luke Wagner [:luke]]

Allow me to expound on the last sentance of comment 12.  I meant to type "I'm not sure who I should have review this" and then got distracted, and then chose Brendan, and then clicked 'save changes'.

Comment 14

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/3381eda6b5ae

Comment 15

[Daniel Veditz [:dveditz]]

How does an incorrect asin() return value lead to an exploitable condition in Firefox? Does it make the value look like the wrong type and screw up JITted code?

Comment 16

[Jeff Walden [:Waldo]]

(In reply to comment #10)
> must we penalize all our Mac OS X builds on account of only one bug in one
> transcendental implementation in OS X 10.5?

It's always an option (if a distasteful one, and one thankfully not yet necessary, it seems) to ship a custom libm (again) to avoid the issue, although I wonder how much performance/optimization effort has gone into the ones out there that are compatible with our licenses.

Comment 17

[Luke Wagner [:luke]]

(In reply to comment #15)
> How does an incorrect asin() return value lead to an exploitable condition in
> Firefox? Does it make the value look like the wrong type and screw up JITted
> code?

Yes, except not just JITted code.  It would only be exploitable if somehow the input of asin() determined the payload of the resulting qnan.  Until comment 10, this was an unknown (we already had the classic nan-boxing assumption broken, who knew how bad it was).  However, comment 10 assures us that only the low bits (which are not part of the type tag) are set, which means that, while non-canonical, the resulting jsval will always be interpreted as a double, never as some other type.  So, not exploitable.  The patch was necessary, however, to loosen the definition of the value range for doubles to include these "qnans with only the low bits set".

Comment 18

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/3381eda6b5ae

Comment 19

[Luke Wagner [:luke]]

Can this be opened?

Comment 20

[Robert Sayre]

I don't think so. There are lots of vulnerable Android devices out there, I expect. Maybe even a Samsung i9000 Galaxy. :P

Comment 21

[Luke Wagner [:luke]]

Vulnerable how?  This was a problem with asin() on 64-bit OS X 10.5.

Comment 22

[Robert Sayre]

(In reply to comment #21)
> Vulnerable how?  This was a problem with asin() on 64-bit OS X 10.5.

Oh, sorry. I should probably at least read the full bug title before commenting. I had the bug mixed up with a different NaN bug. No opinion on this one.