Closed Bug 395585 Opened 13 years ago Closed 8 years ago
OOM problems in ns
A couple of places we should check if 'new' fails: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/modules/libpr0n/encoders/png/nsPNGEncoder.cpp&rev=1.5&root=/cvsroot&mark=266,257#255 I think the error handling here is insufficient: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/modules/libpr0n/encoders/png/nsPNGEncoder.cpp&rev=1.5&root=/cvsroot&mark=574#561 If we leave 'that->mImageBuffer' non-null I think other code (the destructor, Close()) might free it twice, and other methods use "mImageBuffer != nsnull" to check if it's valid... We should set it to nsnull after PR_Free?
operator new is now explicitly infallible, https://developer.mozilla.org/en/Infallible_memory_allocation so the first issue here is fixed. The second issue seems to still be a real possible bug, so let me make a 1-line patch...
Brian seems to be the last to have touched this file -> asking for review.
Attachment #565621 - Flags: review?(netzen)
Comment on attachment 565621 [details] [diff] [review] null the imagebuffer pointer then we free it Looks good.
Attachment #565621 - Flags: review?(netzen) → review+
Maybe we need to do a post-mortem here. This bug had 'OOM' in its title. OOM bugs are probably a lot more likely than the average to be security sensitive. Can we have automatic queries to make sure that at least bugs explicitly mentioning OOM issues don't fall off the radar?
Assignee: nobody → bjacob
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.