linux-kernel - Re: [PATCH 1/3] drivers/gpu/drm: Move a dereference below a NULL test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1229728328.6544.10.camel@gaiman.anholt.net>
Date:	Fri, 19 Dec 2008 15:12:08 -0800
From:	Eric Anholt <eric@...olt.net>
To:	Dave Airlie <airlied@...il.com>
Cc:	Julia Lawall <julia@...u.dk>, airlied@...ux.ie,
	dri-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 1/3] drivers/gpu/drm: Move a dereference below a NULL
	test

On Sat, 2008-12-20 at 08:35 +1000, Dave Airlie wrote:
> On Sat, Dec 20, 2008 at 3:10 AM, Julia Lawall <julia@...u.dk> wrote:
> > From: Julia Lawall <julia@...u.dk>
> >
> > If the NULL test is necessary, then the dereference should be moved below
> > the NULL test.
> >
> > The semantic patch that makes this change is as follows:
> > (http://www.emn.fr/x-info/coccinelle/).  The result has been modified to
> > move the initialization of driver down closer to where it is used.
> >
> > // <smpl>
> > @@
> > type T;
> > expression E;
> > identifier i,fld;
> > statement S;
> > @@
> >
> > - T i = E->fld;
> > + T i;
> >  ... when != E
> >      when != i
> >  if (E == NULL) S
> > + i = E->fld;
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@...u.dk>
> >
> > ---
> >  drivers/gpu/drm/drm_drv.c |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 0b9f316..4bdfc98 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -297,7 +297,7 @@ EXPORT_SYMBOL(drm_init);
> >  */
> >  static void drm_cleanup(struct drm_device * dev)
> >  {
> > -       struct drm_driver *driver = dev->driver;
> > +       struct drm_driver *driver;
> >
> >        DRM_DEBUG("\n");
> >
> > @@ -324,8 +324,9 @@ static void drm_cleanup(struct drm_device * dev)
> >                dev->agp = NULL;
> >        }
> >
> > -       if (dev->driver->unload)
> > -               dev->driver->unload(dev);
> > +       driver = dev->driver;
> > +       if (driver->unload)
> > +               driver->unload(dev);
> 
> Huh?
> 
> The original test is to check it dev->driver->unload exists, not it
> dev->driver exists.
> 
> I think your test  parameters are wrong.

No, it was valid, but I think the better patch is:

From 1e0a24cfe75a328db5a50dbcdaaf0eb461638b6b Mon Sep 17 00:00:00 2001
From: Eric Anholt <eric@...olt.net>
Date: Fri, 19 Dec 2008 15:07:11 -0800
Subject: [PATCH] drm: Avoid use-before-null-test on dev in drm_cleanup().

Signed-off-by: Eric Anholt <eric@...olt.net>

---
 drivers/gpu/drm/drm_drv.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 373e3de..febb517 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -294,8 +294,6 @@ EXPORT_SYMBOL(drm_init);
  */
 static void drm_cleanup(struct drm_device * dev)
 {
-	struct drm_driver *driver = dev->driver;
-
 	DRM_DEBUG("\n");
 
 	if (!dev) {
@@ -330,7 +328,7 @@ static void drm_cleanup(struct drm_device * dev)
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 
-	if (driver->driver_features & DRIVER_GEM)
+	if (dev->driver->driver_features & DRIVER_GEM)
 		drm_gem_destroy(dev);
 
 	drm_put_minor(&dev->primary);
-- 
1.5.6.5


-- 
Eric Anholt
eric@...olt.net                         eric.anholt@...el.com



Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)