Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Embedding `drm_driver` in `struct nouveau_drm` introduces an exploitable Use-After-Free vulnerability when the device is unplugged or unbound while userspace holds open file descriptors. --
--- Patch [3]: [PATCH v4 3/5] drm/nouveau: Fix drm_driver struct/nouveau.atomic parameter handling --- commit 6aa4a539bc58f0cd546203fa854c1edf2120c1be Author: Lyude Paul <[email protected]> drm/nouveau: Fix drm_driver struct/nouveau.atomic parameter handling This commit moves the drm_driver struct from a static global variable into the dynamically allocated nouveau_drm structure. It also handles conditionally enabling atomic modesetting per-device. > diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h > b/drivers/gpu/drm/nouveau/nouveau_drv.h > --- a/drivers/gpu/drm/nouveau/nouveau_drv.h > +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h > @@ -216,6 +216,7 @@ struct nouveau_drm { > > struct nouveau_cli client; > struct drm_device *dev; > + struct drm_driver drm_driver; > > struct list_head clients; [ ... ] > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c [ ... ] > @@ -744,12 +741,14 @@ nouveau_drm_device_new(const struct drm_driver > *drm_driver, struct device *paren > if (!drm) > return ERR_PTR(-ENOMEM); > > + drm->drm_driver = driver_stub; > drm->nvkm = device; > > - drm->dev = drm_dev_alloc(drm_driver, parent); > + drm->dev = drm_dev_alloc(&drm->drm_driver, parent); [Severity: High] Does embedding drm_driver inside nouveau_drm create a use-after-free risk if the device is unplugged or unbound while userspace holds an open file descriptor? If a user opens the DRM device node, the open file descriptor holds a reference to the drm_device. If the device is then hot-unplugged or unbound via sysfs, nouveau_drm_device_del() will unconditionally free the nouveau_drm struct: nouveau_drm_device_del() ... kfree(drm); However, the drm_device remains alive due to the file descriptor's reference. When the user later closes the file descriptor, the DRM core will dereference the dangling dev->driver pointer: drm_file_free() ... if (dev->driver->postclose) dev->driver->postclose(dev, file); Could this lead to memory corruption since the drm_driver struct was already freed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
