[PATCH v2 1/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem
Joe Moriarty
joe.moriarty at oracle.com
Tue Feb 20 17:54:05 UTC 2018
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 at oracle.com>
>> Reviewed-by: Steven Sistare <steven.sistare at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
More information about the dri-devel
mailing list