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
> ---
> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
