From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/appletbdrm: Allocate request/response buffers in begin_fb_access Date: Sat, 16 May 2026 15:34:45 +1000 Message-ID: In-Reply-To: <20260511122421.114014-6-tzimmermann@suse.de> References: <20260511122421.114014-1-tzimmermann@suse.de> <20260511122421.114014-6-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Correctness issue -- resource leak in error path**: In `appletbdrm_primary_plane_helper_begin_fb_access()`: ```c ret = drm_gem_begin_shadow_fb_access(plane, new_plane_state); if (ret) return ret; // ... later ... appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL); if (!appletbdrm_state->request) return -ENOMEM; // LEAK: shadow fb access not ended appletbdrm_state->response = kzalloc_obj(*appletbdrm_state->response); if (!appletbdrm_state->response) return -ENOMEM; // LEAK: shadow fb access not ended, request not freed ``` If `kvzalloc` or `kzalloc_obj` fails, `drm_gem_end_shadow_fb_access()` is never called, and allocated memory is not freed. The `end_fb_access` helper (`drm_gem_end_shadow_fb_access`) is only called on the success path by the framework. The `appletbdrm_primary_plane_destroy_state()` will eventually free the allocations via `kvfree`/`kfree` (which handle NULL), but the shadow fb vmap may leak depending on how the framework handles begin_fb_access failures. **Note**: This bug may have existed in the original code's atomic_check path too (in a different form), so it may be a pre-existing issue that this patch isn't making worse. But since the code is being restructured, it would be good to fix the error paths here with proper `goto err_*` cleanup. The broader design change (moving buffer allocation from atomic_check to begin_fb_access) is correct and well-motivated: damage info isn't fully evaluated during atomic_check, so sizing allocations based on damage clips there could produce incorrectly-sized buffers. --- Generated by Claude Code Patch Reviewer