613 bytes, application/vnd.mozilla.xul+xml
2.66 KB, patch
Samuel Sidler (old account; do not CC): approval22.214.171.124+
Samuel Sidler (old account; do not CC): approval126.96.36.199+
|Details | Diff | Splinter Review|
Report only mentions 3.0.x. Does this not affect 3.5.x, or did they only confirm it on 3.0.x?
Created attachment 392159 [details] XUL file as described Doesn't crash my 3.0.10. Are particular contents of the binding.xml file needed?
(In reply to comment #2) > Doesn't crash my 3.0.10. Are particular contents of the binding.xml file > needed? dveditz followed-up with TippingPoint on that, but they have yet to respond, afaict.
Annoyingly enough, it doesn't seem to crash under valgrind, though it crashes instantly without valgrind. Looks like it involves JS having a pointer to an nsTreeColumn whose mColumns pointer is dangling (pointing to freed memory). It makes sense that the nsTreeColumns is gone; we've reframed the tree when we set the binding. But the nsTreeColumns destructor should have nulled out the columns pointer back to it. Presumably it got detached from the nsTreeColumns some other way.
Created attachment 393090 [details] [diff] [review] patch Well, I looked at the code, and looked for places that might have left dangling pointers, and found two, and fixed them, and then it stopped crashing. Plus a new assertion for bonus points.
So, for what it's worth, this seems like the sort of patch that people reading patches being landed would have a pretty good chance of figuring out how to exploit. Any thoughts on when I should or shouldn't land it, or other ways to disguise it?
People are apparently already looking at this area, so let's land ASAP
Fixed on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/ec77f4fe64a5
... well, actually, not really, since the same problem existed in a different place before that.
Comment on attachment 393090 [details] [diff] [review] patch Approved for 188.8.131.52 and 184.108.40.206. a=ss for release-drivers
Fixed on mozilla-1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/57be70578fac
Fix checked in to CVS trunk (for 220.127.116.11), 2009-08-08 20:20:10 -0700.
Verified fixed in 18.104.22.168 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:22.214.171.124pre) Gecko/2009081305 GranParadiso/3.0.14pre (.NET CLR 3.5.30729). Testcase no longer crashes as it does when tested with 126.96.36.199 build.
Verified fixed for 188.8.131.52 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:184.108.40.206pre) Gecko/20090817 Shiretoko/3.5.3pre (.NET CLR 3.5.30729).
(In reply to comment #13) > ... well, actually, not really, since the same problem existed in a different > place before that. ... probably introduced when the code was first written in bug 221619, but even then it could have been moved from elsewhere (though it doesn't look like it).
This landed on mozilla-central before 1.9.2 branched, but wasn't a blocker at the time we did the mass-marking, so marking status1.9.2:beta1-fixed . http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ec77f4fe64a5
verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20090925 Namoroka/3.6b1pre
Comment on attachment 393090 [details] [diff] [review] patch Approved for 220.127.116.11, a=dveditz for release-drivers
Need to make sure this doesn't break TB 2 before landing on 18.104.22.168
(In reply to comment #23) > Need to make sure this doesn't break TB 2 before landing on 22.214.171.124 I built TB 2 and played around with various tree elements and couldn't see any issues... (In reply to comment #22) > (From update of attachment 393090 [details] [diff] [review]) > Approved for 126.96.36.199, a=dveditz for release-drivers Checking in layout/xul/base/src/tree/src/nsTreeColumns.cpp; /cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeColumns.cpp,v <-- nsTreeColumns.cpp new revision: 188.8.131.52; previous revision: 184.108.40.206 done Checking in layout/xul/base/src/tree/src/nsTreeColumns.h; /cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeColumns.h,v <-- nsTreeColumns.h new revision: 220.127.116.11; previous revision: 18.104.22.168 done