Closed Bug 527400 Opened 15 years ago Closed 14 years ago

OOM null deref introduced by fix for bug 514872

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta5-fixed
blocking1.9.1 --- .8+
status1.9.1 --- .8-fixed

People

(Reporter: dveditz, Assigned: Gijs)

References

Details

(Keywords: crash, fixed1.9.0.18, regression)

Attachments

(1 file)

The patches in bug 514872 fixed the problem of forgetting to copy the expression, but it does so before checking whether the allocation succeeds. If for some reason it fails (and if it never does why are we checking at all?) then we'll crash on a null deref doing memcpy(0,...)

Not likely in practice, and we've talked about making allocation failures instantly fatal anyway, but in the meantime static source checkers like Coverity are going to complain at us.
I'm really appalled at having missed the one thing there possibly was to miss about this fix... Anyway, let's get this done. :-\
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #411161 - Flags: review?(jwalden+bmo)
Attachment #411161 - Flags: review?(jwalden+bmo) → review+
Blah, I dropped the ball on this - sorry! The tree is currently restricted - should we attempt to still get this for 3.6? It's pretty harmless, as far as I can tell... dveditz, what do you reckon?
Comment on attachment 411161 [details] [diff] [review]
Patch moving the memcpy below the nullcheck

Doesn't block 1.9.2, but is wanted on that branch and safe to take.
Attachment #411161 - Flags: approval1.9.2?
We'll want this on the other branches that got 514872, too.
blocking1.9.1: --- → ?
Flags: wanted1.9.0.x+
Attachment #411161 - Flags: approval1.9.1.7?
Attachment #411161 - Flags: approval1.9.0.17?
Attachment #411161 - Flags: approval1.9.2?
Attachment #411161 - Flags: approval1.9.2+
Attachment #411161 - Flags: approval1.9.1.7?
Attachment #411161 - Flags: approval1.9.1.7+
Attachment #411161 - Flags: approval1.9.0.17?
Attachment #411161 - Flags: approval1.9.0.17+
Comment on attachment 411161 [details] [diff] [review]
Patch moving the memcpy below the nullcheck

a=beltzner for all branches, please land on 1917 and 19017 when the builds are done for 1916 and 19016
http://hg.mozilla.org/mozilla-central/rev/cbaeccb42882

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/13a02fce20e5
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
blocking1.9.1: ? → .7+
(In reply to comment #5)
> (From update of attachment 411161 [details] [diff] [review])
> a=beltzner for all branches, please land on 1917 and 19017 when the builds are
> done for 1916 and 19016

Gah, missed that train (well, .7beta is out, according to the testing announcement in .planning). From the announcement there of .6 being released (the 16th) there was an 8-day window to check this in - but you wrote "when the builds are done", not when the release is announced - is there a better thing than the release announcement I should be looking for in terms of the .7 builds being Done so I can land this for .8 ?

Also, this bug was originally marked blocking 1.9.1.7+, and is now .8+, but there was no comment on this bug about it being pushed back (or asking me to land it soon please because...). I'm not trying to shift blame here, but I'm also not really clear on how this blocking mechanism then works. :-)
.7 and .17 were pushed forward for a few specific fixes, and are being shipped from the previous relbranch - this bug specifically wasn't pushed back. You're free to land on 1.9.0/1.9.1 now for 1.9.0.18/1.9.1.8.
(In reply to comment #8)
> .7 and .17 were pushed forward for a few specific fixes, and are being shipped
> from the previous relbranch - this bug specifically wasn't pushed back. You're
> free to land on 1.9.0/1.9.1 now for 1.9.0.18/1.9.1.8.

OK, great, thanks for clarifying! Will push those changes as soon as I can get to it inbetween the goings-on these days... Merry Christmas! :-)
Checking in nsWildCard.cpp;
/cvsroot/mozilla/xpfe/components/filepicker/src/nsWildCard.cpp,v  <--  nsWildCard.cpp
new revision: 1.7; previous revision: 1.6
done


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/076725c55412
You need to log in before you can comment on or make changes to this bug.