Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Hot-unplug during an open file descriptor leads to a use-after-free of nouveau_cli via the DRM_NOUVEAU_NVIF ioctl. -- --- Patch [1]: [PATCH] drm/nouveau: fix autosuspend cleanup during teardown --- commit ae87ae00f3257ee8e11b10260263c7423ec1666c Author: Guangshuo Li <[email protected]> drm/nouveau: fix autosuspend cleanup during teardown nouveau_drm_device_init() calls pm_runtime_use_autosuspend(), but nouveau_drm_device_fini() does not call the matching pm_runtime_dont_use_autosuspend(). Cc: [email protected] > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 4d1ad718e09b7..42daad06fdbe3 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -585,6 +585,7 @@ nouveau_drm_device_fini(struct nouveau_drm *drm) > if (nouveau_pmops_runtime()) { > pm_runtime_get_sync(dev->dev); > pm_runtime_forbid(dev->dev); > + pm_runtime_dont_use_autosuspend(dev->dev); > } > > nouveau_led_fini(dev); [Severity: High] This is a pre-existing issue, but could the subsequent client cleanup in nouveau_drm_device_fini() lead to a use-after-free if the device is hot-unplugged while userspace still has an open file descriptor? If a hot-unplug occurs, nouveau_drm_device_fini() unconditionally frees the client structures a bit further down in the function: list_for_each_entry_safe(cli, temp_cli, &drm->clients, head) { ... nouveau_cli_fini(cli); kfree(cli); } If userspace then issues a DRM_NOUVEAU_NVIF ioctl on the still-open file descriptor, nouveau_drm_ioctl() handles it: ret = pm_runtime_get_sync(dev->dev); if (ret < 0 && ret != -EACCES) return ret; ... switch (cmd) { case DRM_NOUVEAU_NVIF: ret = nouveau_abi16_ioctl(filp, (void __user *)arg, _IOC_SIZE(cmd)); break; Because runtime PM is disabled upon device removal, pm_runtime_get_sync() returns -EACCES, which the ioctl handler ignores. The ioctl proceeds directly to nouveau_abi16_ioctl(), bypassing the standard drm_dev_is_unplugged() check. This eventually calls nouveau_abi16_get(), which accesses the freed memory: struct nouveau_cli *cli = nouveau_cli(file_priv); mutex_lock(&cli->mutex); Can this sequence lead to memory corruption or panics during teardown? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
