Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Memory leak of drm_client_buffer and its virtual memory mapping if register_framebuffer() fails. -- commit 99c42165eff4a5954aaf41ad682bb169178e6b09 Author: shechenglong <[email protected]> drm/client: fix restore of partially initialized client This commit fixes a NULL pointer dereference that occurs when closing a DRM file descriptor for a partially initialized fbdev client. It skips the restore process in drm_fbdev_client_restore() if no valid fbdev framebuffer is found. > diff --git a/drivers/gpu/drm/clients/drm_fbdev_client.c > b/drivers/gpu/drm/clients/drm_fbdev_client.c > index 91d196a397cf2..1c16bc1084c40 100644 > --- a/drivers/gpu/drm/clients/drm_fbdev_client.c > +++ b/drivers/gpu/drm/clients/drm_fbdev_client.c > @@ -42,6 +42,14 @@ static int drm_fbdev_client_restore(struct drm_client_dev > *client, bool force) > { > struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); > > + /* > + * The client is registered before the initial fbdev probe. > + * If probing failed, the client remains registered but there > + * is no valid fbdev framebuffer to restore. > + */ > + if (!fb_helper->info || !fb_helper->fb) > + return 0; > + [Severity: High] This is a pre-existing issue, but since the commit message mentions mirroring the logic from drm_fbdev_client_unregister(), does the current cleanup path leak the drm_client_buffer when probing fails? In drivers/gpu/drm/drm_fb_helper.c:__drm_fb_helper_initial_config_and_unlock(), drm_fb_helper_single_fb_probe() allocates fb_helper->buffer (which creates the framebuffer and maps memory). If the subsequent call to register_framebuffer() fails, it returns without cleaning up the buffer: drivers/gpu/drm/drm_fb_helper.c:__drm_fb_helper_initial_config_and_unlock() { ... ret = register_framebuffer(info); if (ret < 0) return ret; ... } The caller handles this error by calling drm_fb_helper_fini(), which frees fb_helper->info via framebuffer_release() and sets fb_helper->info to NULL. Importantly, framebuffer_release() only frees the struct without invoking the fb_destroy callback. Later, when the client is unregistered, drm_fbdev_client_unregister() takes the else branch because fb_helper->info is NULL: drivers/gpu/drm/clients/drm_fbdev_client.c:drm_fbdev_client_unregister() { if (fb_helper->info) { ... } else { /* * Partially initialized client, no framebuffer device yet */ drm_client_release(&fb_helper->client); } } This skips drm_fb_helper_unregister_info() and the associated fb_destroy() callback, permanently leaking fb_helper->buffer, its backing GEM object, and its virtual memory mapping. This leak can be triggered by resource exhaustion, limit reached (FB_MAX), or other failures during register_framebuffer(). Could the error handling path be updated to ensure the buffer is properly destroyed when this occurs? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
