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 <[email protected]> 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/[email protected]?part=3
