Closed Bug 414755 Opened 12 years ago Closed 12 years ago

Missing SAVE_SP_AND_PC in STORE_(NUMBER|INT|UINT)

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: igor, Assigned: igor)

Details

(Keywords: verified1.8.1.13, Whiteboard: [sg:critical?])

Attachments

(2 files)

The macros STORE_(NUMBER|INT|UINT) from jsinterp.c do not call SAVE_SP_AND_PC before calling potentially GC-triggering js_NewDoubleValue. This leads to a GC hazard as demonstrated with the following example:

~/m/ff/mozilla/js/src $ cat ~/m/y.js

var expect = f();

gczeal(2);

var actual = f();

if (expect !== actual)
    throw "Hazard: actual="+actual+" expect="+expect;

function f()
{
    var a = 1e10;
    var b = 2e10;
    var c = 3e10;
    
    return (a*2) * ((b*2) * c); 
}

~/m/ff/mozilla/js/src $ ./Linux_All_DBG.OBJ/js ~/m/y.js
uncaught exception: Hazard: actual=1.44e+42 expect=2.4e+31

Note that so far I was not able to discover a way to get a crash with that. But it can be used to read a memory image of allocated structs.
Attached patch v1Splinter Review
The patch just adds the missing calls to SAVE_SP_AND_PC.
Attachment #300249 - Flags: review?(brendan)
Attachment #300249 - Flags: review?(brendan)
Attachment #300249 - Flags: review+
Attachment #300249 - Flags: approval1.9+
I checked in the patch from comment 1 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%252Fcvsroot&date=explicit&mindate=1201671180&maxdate=1201671331&who=igor%25mir2.org
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: blocking1.8.1.13?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.13?
Flags: blocking1.8.1.13+
Whiteboard: [sg:critical?]
Comment on attachment 300249 [details] [diff] [review]
v1

The patch applies to the 181 branch as is.
Attachment #300249 - Flags: approval1.8.1.13?
Comment on attachment 300249 [details] [diff] [review]
v1

approved for 1.8.1.13, a=dveditz for release-drivers
Attachment #300249 - Flags: approval1.8.1.13? → approval1.8.1.13+
Flags: in-testsuite+
v
Status: RESOLVED → VERIFIED
I checked in the patch from comment 1 to MOZILLA_1_8_BRANCH:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.95; previous revision: 3.181.2.94
done
Keywords: fixed1.8.1.13
v 1.8.1 linux|mac
Group: security
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-414755.js,v  <--  regress-414755.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.