Last Comment Bug 395585 - OOM problems in nsPNGEncoder.cpp
: OOM problems in nsPNGEncoder.cpp
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla10
Assigned To: Benoit Jacob [:bjacob] (mostly away)
: Milan Sreckovic [:milan]
Depends on:
  Show dependency treegraph
Reported: 2007-09-09 10:12 PDT by Mats Palmgren (:mats)
Modified: 2011-10-14 03:59 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

null the imagebuffer pointer then we free it (958 bytes, patch)
2011-10-07 13:05 PDT, Benoit Jacob [:bjacob] (mostly away)
netzen: review+
Details | Diff | Splinter Review

Description User image Mats Palmgren (:mats) 2007-09-09 10:12:50 PDT
A couple of places we should check if 'new' fails:,257#255

I think the error handling here is insufficient:

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?
Comment 1 User image Benoit Jacob [:bjacob] (mostly away) 2011-10-07 12:48:26 PDT
operator new is now explicitly infallible,

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...
Comment 2 User image Benoit Jacob [:bjacob] (mostly away) 2011-10-07 13:05:28 PDT
Created attachment 565621 [details] [diff] [review]
null the imagebuffer pointer then we free it

Brian seems to be the last to have touched this file -> asking for review.
Comment 3 User image Brian R. Bondy [:bbondy] 2011-10-07 19:27:51 PDT
Comment on attachment 565621 [details] [diff] [review]
null the imagebuffer pointer then we free it

Looks good.
Comment 4 User image Benoit Jacob [:bjacob] (mostly away) 2011-10-11 14:48:08 PDT
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?
Comment 5 User image Benoit Jacob [:bjacob] (mostly away) 2011-10-13 05:12:13 PDT
Comment 6 User image Ed Morley [:emorley] 2011-10-14 03:59:07 PDT

Note You need to log in before you can comment on or make changes to this bug.