Last Comment Bug 678982 - potential null pointer dereference in gfx/layers/d3d10/ImageLayerD3D10.cpp
: potential null pointer dereference in gfx/layers/d3d10/ImageLayerD3D10.cpp
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla12
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks: cppcheck
  Show dependency treegraph
 
Reported: 2011-08-15 07:33 PDT by David Volgyes
Modified: 2012-02-01 14:00 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
ImageLayerD3D10.diff (870 bytes, patch)
2011-08-15 07:33 PDT, David Volgyes
no flags Details | Diff | Splinter Review
fix per comment 1 (2.07 KB, patch)
2012-01-18 14:51 PST, :aceman
bas: review+
Details | Diff | Splinter Review
fix per nit (2.10 KB, patch)
2012-01-18 15:12 PST, :aceman
acelists: review+
Details | Diff | Splinter Review

Description David Volgyes 2011-08-15 07:33:53 PDT
Created attachment 553172 [details] [diff] [review]
ImageLayerD3D10.diff

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622232440

Steps to reproduce:

cppcheck 1.49 (http://cppcheck.sourceforge.net/) found a plenty of potential null pointer dereference. This is one of them.


Actual results:

in the line #53 there is a check for 'aSurface' is null or not,
but at line #65 there is an unchecked aSurface->
which is a potential null pointer dereference.



Expected results:

Well, it depends. I do not know the code well enough, but you definitely should check the pointer before dereference. A possible fix is attached, but this fix seems ugly for me. (But I have no better idea.)
Comment 1 Bas Schouten (:bas.schouten) 2011-08-15 07:50:25 PDT
Realistically, aSurface should never be NULL there, and if it is, we've got bigger problems! The right solution is probably to remove the NULL check at the top, and add a 'if (!aSurface) { return NULL; }' to the top of that function. I doubt that wouldn't break further down the flow though.
Comment 2 Ed Morley [:emorley] 2011-08-31 06:08:59 PDT
Unassigning from David per his request (bug 679610 comment 4).

For anyone looking at this bug, feel free to take it, David has very kindly provided a patch, but will not have time to follow it through.
Comment 3 :aceman 2012-01-18 14:51:20 PST
Created attachment 589659 [details] [diff] [review]
fix per comment 1
Comment 4 Bas Schouten (:bas.schouten) 2012-01-18 14:55:47 PST
Comment on attachment 589659 [details] [diff] [review]
fix per comment 1

Review of attachment 589659 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/d3d10/ImageLayerD3D10.cpp
@@ +49,5 @@
>  SurfaceToTexture(ID3D10Device *aDevice,
>                   gfxASurface *aSurface,
>                   const gfxIntSize &aSize)
>  {
> +  if (!aSurface) { return NULL; }

nit: as per coding style

if (!aSurface) {
  return NULL;
}
Comment 5 :aceman 2012-01-18 15:12:57 PST
Created attachment 589667 [details] [diff] [review]
fix per nit

Carrying over review=bas.schouten .
Comment 7 Ed Morley [:emorley] 2012-01-19 17:49:00 PST
https://hg.mozilla.org/mozilla-central/rev/7e7800f6e68b

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