679593 - Possible JSScript double-free

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

The problem is in js_CloneFunctionObject. Consider the cross-compartment case where we're clonging |fun|. We create a new function |cfun| and do:
  cfun->u = fun->getFunctionPrivate()->u;
For an interpreted script, this makes cfun->script() == fun->script(). Then we do
  JSScript *cscript = js_CloneScript(cx, script);
and, after a null check on cscript, make |cfun->script() == cscript|.

Say that we fail to allocate cscript. Then we'll have two allocated function objects, in different compartments, pointing to the same script. This is bad because of the compartment thing and also because each function object finalizer will free the script, so it will be freed twice.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: fix

This was totally my fault. I changed this recently and it was a bad idea. This should fix it.

Comment 2

[Mounir Lamouri (:mounir)]

http://hg.mozilla.org/mozilla-central/rev/c8eea83232b2

Comment 3

[Johnny Stenback (:jst)]

Is this patch good for aurora and beta? If so, please request approvals.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 554933 [details] [diff] [review]
fix

There's a very slight possibility that this is broken in beta. It's definitely broken in aurora. I'm not sure how significant a problem this actually is, but it's unlikely to regress anything so we should probably get it in.

Comment 5

[Johnny Stenback (:jst)]

Bill, can you get this landed on aurora, we've only got a few days left!

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Sorry, forgot about this.

https://hg.mozilla.org/releases/mozilla-aurora/rev/391c8ebfa0a6

Comment 7

[Johnny Stenback (:jst)]

This landed in aurora before the most recent uplift, which means it's fixed for 8! Marking so.

Comment 8

[u279076]

qa- as nothing to do for QA fix verification -- please set to qa+ if this is not the case.

Comment 9

[Daniel Veditz [:dveditz]]

The 1.9.2 code is significantly different and there's no testcase, assuming 3.6.x is OK.