Closed
Bug 1277122
Opened 9 years ago
Closed 9 years ago
[Static Analysis][Dereference Null Return Value] Missing null checks on some pixman_image_create_bits() calls
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: CID 1295437)
Attachments
(1 file, 1 obsolete file)
2.74 KB,
patch
|
seth
:
review+
lsalzman
:
review+
|
Details | Diff | Splinter Review |
pixman_image_create_bits() is fallible and will return null on failure. Most call sites null-check, but three don't.
![]() |
Assignee | |
Comment 1•9 years ago
|
||
Attachment #8758507 -
Flags: review?(seth)
Attachment #8758507 -
Flags: review?(lsalzman)
![]() |
Assignee | |
Updated•9 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 2•9 years ago
|
||
I forgot to say: seth, please review the image/ change; lsalzman, please review the gfx/ change.
Comment 3•9 years ago
|
||
Comment on attachment 8758507 [details] [diff] [review]
Add missing null checks for pixman_image_create_bits()
Review of attachment 8758507 [details] [diff] [review]:
-----------------------------------------------------------------
This will leak pixman images if src succeeds but dst does not. You need to make sure you pixman_image_unref(src) in that case before returning.
The same problem is repeated in both gfx/ and image/.
Attachment #8758507 -
Flags: review?(lsalzman) → review-
![]() |
Assignee | |
Comment 4•9 years ago
|
||
Addresses lsalzman's comment.
Attachment #8758541 -
Flags: review?(seth)
Attachment #8758541 -
Flags: review?(lsalzman)
![]() |
Assignee | |
Updated•9 years ago
|
Attachment #8758507 -
Attachment is obsolete: true
Attachment #8758507 -
Flags: review?(seth)
Updated•9 years ago
|
Attachment #8758541 -
Flags: review?(lsalzman) → review+
Comment 5•9 years ago
|
||
Comment on attachment 8758541 [details] [diff] [review]
Add missing null checks for pixman_image_create_bits()
Review of attachment 8758541 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good!
::: gfx/2d/DrawTargetCairo.cpp
@@ +1967,5 @@
> pixman_image_create_bits(srcFormat,
> srcSurf->GetSize().width, srcSurf->GetSize().height,
> (uint32_t*)srcMap.GetData(), srcMap.GetStride());
> + if (!src) {
> + pixman_image_unref(dst);
It'd be lovely to have a RAII wrapper for this but this bug isn't really the place to do it.
::: image/FrameAnimator.cpp
@@ +808,5 @@
> aSrcRect.width, aSrcRect.height,
> reinterpret_cast<uint32_t*>(const_cast<uint8_t*>(aSrcData)),
> aSrcRect.width * 4);
> + if (!src) {
> + return NS_ERROR_OUT_OF_MEMORY;
We don't check return values, but what else is new? Shouldn't be problematic, anyway, as it's not clear there's any interesting recovery we could do.
Attachment #8758541 -
Flags: review?(seth) → review+
![]() |
Assignee | |
Comment 6•9 years ago
|
||
> > + if (!src) {
> > + pixman_image_unref(dst);
>
> It'd be lovely to have a RAII wrapper for this but this bug isn't really the
> place to do it.
Yeah... I see 70 instances of pixman_image_unref() in the code but only 4 of them are in C++ code so the benefit would be low.
![]() |
Assignee | |
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/50e32be6552151fd71dd828b30b24749b5fbf4d4
Bug 1277122 - Add missing null checks for pixman_image_create_bits(). r=seth,lsalzman.
Comment 8•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•