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

[Joe Moriarty]

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.moria...@oracle.com>
Reviewed-by: Steven Sistare <steven.sist...@oracle.com>

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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