OOM null deref introduced by fix for bug 514872

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dveditz, Assigned: Gijs)

Tracking

({crash, fixed1.9.0.18, regression})

Trunk
crash, fixed1.9.0.18, regression
Points:
---
Bug Flags:
wanted1.9.0.x +

Firefox Tracking Flags

(status1.9.2 beta5-fixed, blocking1.9.1 .8+, status1.9.1 .8-fixed)

Details

Attachments

(1 attachment)

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.
(Assignee)

Comment 1

8 years ago
Created attachment 411161 [details] [diff] [review]
Patch moving the memcpy below the nullcheck

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+
(Assignee)

Comment 2

8 years ago
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: --- → ?
status1.9.1: --- → wanted
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
(Assignee)

Comment 6

8 years ago
http://hg.mozilla.org/mozilla-central/rev/cbaeccb42882

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/13a02fce20e5
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
status1.9.2: --- → final-fixed
Resolution: --- → FIXED
blocking1.9.1: ? → .7+
(Assignee)

Comment 7

8 years ago
(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.
(Assignee)

Comment 9

8 years ago
(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! :-)
(Assignee)

Comment 10

8 years ago
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
status1.9.1: wanted → .8-fixed
Keywords: fixed1.9.0.18
You need to log in before you can comment on or make changes to this bug.