Re: [PATCH v2 3/5] drm/nouveau: Fix drm_driver struct/nouveau.atomic parameter handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dynamically allocating `struct drm_driver` with `devm_kzalloc` introduces a Use-After-Free vulnerability when the physical device is unbound.

Pre-existing issues:
- [High] The core driver structure `struct nouveau_drm` is allocated with `devm_kzalloc`, leading to a preexisting Use-After-Free when the device is unbound.
--

--- Patch [3]: [PATCH v2 3/5] drm/nouveau: Fix drm_driver struct/nouveau.atomic parameter handling ---
commit 9eb1e724469599d6b2baf3bcec061d6d7b31c8bc
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau: Fix drm_driver struct/nouveau.atomic parameter handling

This commit addresses issues with how the nouveau.atomic module parameter is
handled. It replaces shared static drm_driver structures with dynamically
allocated ones using devm_kzalloc to allow safely setting per-device driver
flags.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 5565cf2367aa9..ae489dc1b1943 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
[ ... ]
> @@ -737,12 +734,18 @@ nouveau_drm_device_new(const struct drm_driver *drm_driver, struct device *paren
>  		{}
>  	};
>  	struct nouveau_drm *drm;
> +	struct drm_driver *drm_driver;
>  	int ret;
>  
>  	drm = devm_kzalloc(parent, sizeof(*drm), GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but does allocating struct nouveau_drm with
devm_kzalloc() cause a use-after-free on device unbind?

If a userspace process holds a DRM file descriptor open while the physical
device is unbound via sysfs or hot-unplug, devres will free this memory.
Since the DRM device lifetime is reference-counted and extended by the open
descriptor, subsequent ioctls or the final drm_release() might dereference
the freed memory via dev->dev_private.

>  	if (!drm)
>  		return ERR_PTR(-ENOMEM);
>  
> +	drm_driver = devm_kzalloc(parent, sizeof(*drm_driver), GFP_KERNEL);

[Severity: High]
Does allocating drm_driver with devm_kzalloc() introduce a similar
use-after-free vulnerability?

If the device is unbound while a DRM file descriptor is open, devres will
free the drm_driver struct. Since the DRM device outlives the physical
device in this scenario, when userspace eventually closes the file
descriptor, the DRM core cleans up by calling driver hooks (like postclose
or release) through dev->driver, which would now point to freed memory.

Could this be avoided by using the static const struct drm_driver and
modifying the drm->dev->driver_features field instead of allocating a new
driver struct for each device?

> +	if (!drm_driver)
> +		return ERR_PTR(-ENOMEM);
> +	*drm_driver = driver_stub;
> +
>  	drm->nvkm = device;
>  
>  	drm->dev = drm_dev_alloc(drm_driver, parent);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730195523.1467423-1-lyude@redhat.com?part=3