Closed
Bug 705875
Opened 13 years ago
Closed 13 years ago
XPConnect is passing null references into xpcom interfaces
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: jruderman, Assigned: bholley)
References
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(3 files, 1 obsolete file)
1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
2. Load the testcase
(typeof window.getInterface) changes from "undefined" to "function" when the cycle collector runs. And calling the function crashes [@ nsGlobalWindow::GetInterface].
Maybe a regression from recent cycle collector changes?
|
Reporter | |
Comment 1•13 years ago
|
||
I doubt this has anything to do with cycle collection, and everything to do with what fuzzPriv.CC does to get at dom window utils.
window.QueryInterface(Components.interfaces.nsIInterfaceRequestor);
window.getInterface(null);
in an unprivileged context is enough to crash my build.
I'm going to guess this is fallout from Bug 692342.
Assignee: nobody → bobbyholley+bmo
Blocks: 692342
Component: DOM → XPConnect
OS: Mac OS X → All
QA Contact: general → xpconnect
Hardware: x86_64 → All
aIID is a null reference.
Summary: Crash with cycle collector, window.getInterface → XPConnect is passing null references into xpcom interfaces
|
|
Reporter | |
Comment 6•13 years ago
|
||
Thanks for the reduction, Kyle.
Attachment #577365 -
Attachment is obsolete: true
|
|
Reporter | |
Comment 7•13 years ago
|
||
Will the fix for bug 605271 (or new-dom-bindings) prevent this QI?
Yes, but even with that there's still an actual XPConnect bug here.
|
Assignee | |
Comment 9•13 years ago
|
||
This is a regression from http://hg.mozilla.org/mozilla-central/rev/c428312abbc7 . The rest of the changes in that patch should be fine.
Pushed this to try: https://tbpl.mozilla.org/?tree=Try&rev=898f757866f3
When it goes green I'll flag for review. Hopefully we don't rely on passing null IIDs anywhere.
(In reply to Bobby Holley (:bholley) from comment #9)
> When it goes green I'll flag for review. Hopefully we don't rely on passing
> null IIDs anywhere.
We'd better not, because that's not valid C++!
|
|
Assignee | |
Comment 11•13 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #10)
> (In reply to Bobby Holley (:bholley) from comment #9)
> > When it goes green I'll flag for review. Hopefully we don't rely on passing
> > null IIDs anywhere.
>
> We'd better not, because that's not valid C++!
No, I mean in the case where we pass an IID pointer rather than an IID reference. I think we support both?
(In reply to Bobby Holley (:bholley) from comment #11)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #10)
> > (In reply to Bobby Holley (:bholley) from comment #9)
> > > When it goes green I'll flag for review. Hopefully we don't rely on passing
> > > null IIDs anywhere.
> >
> > We'd better not, because that's not valid C++!
>
> No, I mean in the case where we pass an IID pointer rather than an IID
> reference. I think we support both?
Oh, idk.
|
|
Assignee | |
Comment 13•13 years ago
|
||
Comment on attachment 578305 [details] [diff] [review]
Bug 705875 - Check for null IID pointers and references in XPCConvert. v1
All the tests pass on linux, which is good enough for me.
Flagging khuey for review.
Attachment #578305 -
Flags: review?(khuey)
Attachment #578305 -
Flags: review?(khuey) → review+
|
|
Assignee | |
Comment 14•13 years ago
|
||
This ended up with a bunch of oranges on mochitest-oth, which all seem to be related to some stuff already on inbound. But I rebased and am doing another try push (for mochitest-o only) to be sure:
https://tbpl.mozilla.org/?tree=Try&rev=8a115e35ad03
|
|
Assignee | |
Comment 15•13 years ago
|
||
Looks good. Pushed to mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/59fe691e50d3
Flags: in-testsuite+
Target Milestone: --- → mozilla11
|
||
Comment 16•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•