Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Debugfs pollution due to early destruction of dev->debugfs_root in drm_dev_register() error path. --
commit d5c910dd66292cf0f42ddf2dd15a2566f8663a8a Author: Krzysztof Karas <[email protected]> drm: Unwind drm device registration upon error The drm device registration is done via drm_dev_register(). This function attempts to undo some of the initiatlization steps under err_unload and err_minors labels, but this process is incomplete - debugfs entries remain and the dev->registered flag is still set to true. > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index cb53baa70995f..2d964c9535f49 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -1144,6 +1144,8 @@ int drm_dev_register(struct drm_device *dev, unsigned > long flags) > drm_minor_unregister(dev, DRM_MINOR_ACCEL); > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > drm_minor_unregister(dev, DRM_MINOR_RENDER); > + drm_debugfs_dev_fini(dev); [Severity: Medium] Will this cause debugfs pollution if a driver retries registration after a failure? In drivers like amdgpu, there is a retry mechanism for registration: drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:amdgpu_pci_probe() { ... retry_init: ret = drm_dev_register(ddev, flags); if (ret == -EAGAIN && ++retry <= 3) { drm_info(adev_to_drm(adev), "retry init %d\n", retry); /* Don't request EX mode too frequently which is attacking */ msleep(5000); goto retry_init; } ... } Because dev->debugfs_root is initialized during drm_dev_init(), calling drm_debugfs_dev_fini() here in the error path of drm_dev_register() destroys the directory and permanently sets dev->debugfs_root to NULL. If the driver retries drm_dev_register() on the same device instance, drm_debugfs_dev_register() will pass the NULL dev->debugfs_root to debugfs_create_file(). This causes device-specific debugfs files (like 'clients', 'gem_names', 'framebuffer') to be created in the global debugfs root (/sys/kernel/debug/) instead of the device-specific directory, polluting the global namespace and causing naming collisions. Should dev->debugfs_root only be destroyed in drm_dev_release() to maintain object lifecycle symmetry? > + dev->registered = false; > out_unlock: > if (drm_dev_needs_global_mutex(dev)) > mutex_unlock(&drm_global_mutex); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
