Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Pre-existing memory leak of dma_fence objects in amdgpu_userq_wait_return_fence_info() and incorrect commit message claim. -- commit 6bf96bb80013304553369c4d2d1884cc3193c908 Author: Wentao Liang <[email protected]> drm/virtio: fix dma_fence refcount leak on error in virtio_gpu_dma_fence_wait() This commit fixes a dma_fence reference count leak in the virtio DRM driver. It adds a dma_fence_put() before returning early from a dma_fence_unwrap_for_each() loop in virtio_gpu_dma_fence_wait(). Cc: [email protected] Fixes: eba57fb5498f ("drm/virtio: Wait for each dma-fence of in-fence array individually") > dma_fence_unwrap_for_each() internally calls dma_fence_unwrap_first() > which does cursor->chain = dma_fence_get(head), taking an extra > reference. On normal loop completion, dma_fence_unwrap_next() > releases this via dma_fence_chain_walk() -> dma_fence_put(). > > When virtio_gpu_do_fence_wait() fails and the function returns early > from inside the loop, the cursor->chain reference is never released. > This is the only caller in the entire kernel that does an early return > inside dma_fence_unwrap_for_each. [Severity: High] This isn't a bug introduced by this patch, but is this statement in the commit message accurate? Looking at amdgpu_userq_wait_return_fence_info() in drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c, there appears to be another instance of an early return from dma_fence_unwrap_for_each() without releasing the internal iterator reference: amdgpu_userq_wait_return_fence_info() { ... dma_fence_unwrap_for_each(f, &iter, fence) { r = amdgpu_userq_wait_add_fence(wait_info, fences, &num_fences, f); if (r) { dma_fence_put(fence); goto free_fences; } } ... } If amdgpu_userq_wait_add_fence() fails (for example, returning -ERESTARTSYS due to a signal), it exits early via the free_fences label. Does this path also leak the iter.chain reference similar to the virtio bug being fixed here? Since userspace might be able to trigger this intentionally (e.g., by providing a mismatching num_fences and sending a signal to interrupt the wait), should the commit message be updated, and the AMD bug addressed as well? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
