Re: [PATCH v2 1/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem

[Joe Moriarty <joe.moriarty@xxxxxxxxxx>]

On 2/19/2018 6:57 AM, Daniel Vetter wrote:
> On Mon, Feb 12, 2018 at 02:51:41PM -0500, Joe Moriarty wrote:
> > The Parfait (version 2.1.0) static code analysis tool found the
> > following NULL pointer dereference problem.
> >
> > - drivers/gpu/drm/drm_drv.c
> > Any calls to drm_minor_get_slot() could result in the return of a NULL
> > pointer when an invalid DRM device type is encountered.  2 helper
> > functions where added for pointer manipulation (drm_minor_get_slot()
> > and drm_minor_set_minor()) along with checks for valid pointers for
> > struct drm_device variables throughout this module.
> >
> > Signed-off-by: Joe Moriarty <joe.moriarty@xxxxxxxxxx>
> > Reviewed-by: Steven Sistare <steven.sistare@xxxxxxxxxx>
>
> We do not ask for an invalid minor (userspace can't do that, it would be a
> kernel bug). BUG_ON for the invalid case instead of all these changes
> acceptable to shut up your checker?
> -Daniel
Daniel,

I did the following and the static checker liked it:

	default:
-		return NULL;
+		BUG();
	}

I will make the change in the patch and resubmit.

Joe

> > ---
> >   drivers/gpu/drm/drm_drv.c | 38 ++++++++++++++++++++++++++++++++++----
> >   1 file changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 9acc1e157813..dee6a4470e2c 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -99,10 +99,36 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
> >   	case DRM_MINOR_CONTROL:
> >   		return &dev->control;
> >   	default:
> > +		DRM_ERROR("Error in %s: Invalid dev, type = %d\n",
> > +			  __func__, type);
> >   		return NULL;
> >   	}
> >   }
> >   
> > +static inline int drm_minor_set_minor(struct drm_device *dev,
> > +				      unsigned int type,
> > +				      struct drm_minor *minor)
> > +{
> > +	struct drm_minor **slot = drm_minor_get_slot(dev, type);
> > +	int retval = -ENODEV;
> > +
> > +	if (slot) {
> > +		retval = 0;
> > +		*slot = minor;
> > +	}
> > +	return retval;
> > +}
> > +
> > +static inline struct drm_minor *drm_minor_get_minor(struct drm_device *dev,
> > +						    unsigned int type)
> > +{
> > +	struct drm_minor **slot = drm_minor_get_slot(dev, type);
> > +
> > +	if (slot)
> > +		return *slot;
> > +	return NULL;
> > +}
> > +
> >   static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> >   {
> >   	struct drm_minor *minor;
> > @@ -137,8 +163,9 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> >   		goto err_index;
> >   	}
> >   
> > -	*drm_minor_get_slot(dev, type) = minor;
> > -	return 0;
> > +	r = drm_minor_set_minor(dev, type, minor);
> > +	if (r == 0)
> > +		return r;
> >   
> >   err_index:
> >   	spin_lock_irqsave(&drm_minor_lock, flags);
> > @@ -155,6 +182,9 @@ static void drm_minor_free(struct drm_device *dev, unsigned int type)
> >   	unsigned long flags;
> >   
> >   	slot = drm_minor_get_slot(dev, type);
> > +	if (!slot)
> > +		return;
> > +
> >   	minor = *slot;
> >   	if (!minor)
> >   		return;
> > @@ -177,7 +207,7 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> >   
> >   	DRM_DEBUG("\n");
> >   
> > -	minor = *drm_minor_get_slot(dev, type);
> > +	minor = drm_minor_get_minor(dev, type);
> >   	if (!minor)
> >   		return 0;
> >   
> > @@ -209,7 +239,7 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
> >   	struct drm_minor *minor;
> >   	unsigned long flags;
> >   
> > -	minor = *drm_minor_get_slot(dev, type);
> > +	minor = drm_minor_get_minor(dev, type);
> >   	if (!minor || !device_is_registered(minor->kdev))
> >   		return;
> >   
> > --
> > 2.15.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel