Re: [RESEND PATCH] drm: Fix NULL pointer dereference in drm_minor_alloc() on error path

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 25.08.26 um 10:44 schrieb shechenglong:
> When drm_sysfs_minor_alloc() fails (e.g., due to -ENOMEM), the
> following Oops occurs because an ERR_PTR is passed to put_device():
>
>    BUG: kernel NULL pointer dereference, address: 0000000000000030
>    #PF: supervisor read access in kernel mode
>    #PF: error_code(0x0000) - not-present page
>    RIP: 0010:kobject_put+0xd/0x60
>    Call Trace:
>     drm_minor_alloc_release+0x1c/0x50 [drm]
>     drm_managed_release+0x96/0x160 [drm]
>     drm_dev_init+0x269/0x330 [drm]
>     drm_dev_alloc+0x3f/0x80 [drm]
>     virtio_gpu_probe+0x40/0x180 [virtio_gpu]
>     virtio_dev_probe+0x1fd/0x360
>     ...
>     do_syscall_64+0xaf/0x500
>     entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> The call path that leads to this crash is:
>
>    virtio_gpu_probe()
>      drm_dev_alloc()
>        drm_dev_init()
>          drm_minor_alloc()              // allocates minor
>            drm_sysfs_minor_alloc()      // returns ERR_PTR(-ENOMEM)
>            minor->kdev = ERR_PTR(-ENOMEM)  // stored directly
>            return -ENOMEM
>          // drm_dev_init() fails, triggers cleanup:
>          drm_managed_release()
>            drm_minor_alloc_release(dev, minor)
>              put_device(minor->kdev)     // ERR_PTR(-ENOMEM) dereferenced
>                kobject_put()             // crashes at address 0x30
>
> drm_minor_alloc() assigns the return value of drm_sysfs_minor_alloc()
> directly to minor->kdev, even when it is an error pointer.  The
> subsequent cleanup unconditionally calls put_device(minor->kdev),
> which dereferences the error pointer and causes a NULL-pointer
> dereference (offset 0x30 into a non-page-mapped area).
>
> Fix this by using a temporary variable to hold the result of
> drm_sysfs_minor_alloc().  If the allocation fails, we return the error
> immediately, leaving minor->kdev as NULL (the whole minor structure is
> zero-allocated).  put_device(NULL) is explicitly allowed and safe.
>
> Fixes: f96306f9892b ("drm: manage drm_minor cleanup with drmm_")
>
> Signed-off-by: shechenglong <shechenglong@xfusion.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for the fix.

Best regards
Thomas

> ---
>   drivers/gpu/drm/drm_drv.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 675675480..2348fbb21 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -143,6 +143,7 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data)
>   static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type)
>   {
>   	struct drm_minor *minor;
> +	struct device *kdev;
>   	int r;
>   
>   	minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
> @@ -164,9 +165,11 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type)
>   	if (r)
>   		return r;
>   
> -	minor->kdev = drm_sysfs_minor_alloc(minor);
> -	if (IS_ERR(minor->kdev))
> -		return PTR_ERR(minor->kdev);
> +	kdev = drm_sysfs_minor_alloc(minor);
> +	if (IS_ERR(kdev))
> +		return PTR_ERR(kdev);
> +
> +	minor->kdev = kdev;
>   
>   	*drm_minor_get_slot(dev, type) = minor;
>   	return 0;

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)