As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 508774 - nsGlobalModalWindow traverses mArguments twice
: nsGlobalModalWindow traverses mArguments twice
Status: RESOLVED FIXED
: fixed1.9.0.18
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: P2 major (vote)
: mozilla1.9.2a1
Assigned To: Peter Van der Beken [:peterv]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 194404 CVE-2009-3988
  Show dependency treegraph
 
Reported: 2009-08-06 04:02 PDT by Peter Van der Beken [:peterv]
Modified: 2009-12-03 06:36 PST (History)
4 users (show)
samuel.sidler+old: blocking1.9.0.18+
samuel.sidler+old: wanted1.9.0.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.8+
.8-fixed


Attachments
v1 (735 bytes, patch)
2009-08-06 04:02 PDT, Peter Van der Beken [:peterv]
jst: review+
jst: superreview+
Details | Diff | Splinter Review
v1 (1.9.1 branch version) (1.01 KB, patch)
2009-12-02 03:34 PST, Peter Van der Beken [:peterv]
peterv: review+
peterv: superreview+
dveditz: approval1.9.1.8+
dveditz: approval1.9.0.18+
Details | Diff | Splinter Review

Description User image Peter Van der Beken [:peterv] 2009-08-06 04:02:10 PDT
Created attachment 392911 [details] [diff] [review]
v1

nsGlobalModalWindow traverses mArguments, which is already traversed in its base class (nsGlobalWindow). This means the cycle collector might be collecting live objects. It doesn't traverse its own member mReturnValue but does unlink it.

I found this from code inspection and I don't have a testcase, so no idea how bad it is. We should be nulling out pointers when unlinking so I don't think we'll end up with stale pointers.
Comment 1 User image Mike Beltzner [:beltzner, not reading bugmail] 2009-08-06 06:22:53 PDT
Peter: do you think this should block the alpha? That's what P1 blockers mean at this time ...
Comment 2 User image Peter Van der Beken [:peterv] 2009-08-06 06:58:10 PDT
Nope.
Comment 3 User image Peter Van der Beken [:peterv] 2009-08-13 01:32:29 PDT
http://hg.mozilla.org/mozilla-central/rev/a4c48ea78e74
Comment 4 User image Peter Van der Beken [:peterv] 2009-12-02 03:34:50 PST
Created attachment 415604 [details] [diff] [review]
v1 (1.9.1 branch version)

This fixes the leaks from bug 504862. Safe patch, has been on trunk since beginning of August.
Comment 5 User image Daniel Veditz [:dveditz] 2009-12-02 15:13:17 PST
Comment on attachment 415604 [details] [diff] [review]
v1 (1.9.1 branch version)

Approved for 1.9.1.7 and 1.9.0.17, a=dveditz for release-drivers

Please land this before bug 504862
Comment 6 User image Peter Van der Beken [:peterv] 2009-12-03 06:36:24 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e72d4faef77f

Note You need to log in before you can comment on or make changes to this bug.