Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] The commit message incorrectly claims that all in-tree callers pre-filter the `num_fences == 0` case. In reality, `amdgpu_userq_wait_return_fence_info()` can and does pass 0, meaning this patch actually fixes a critical pre-existing use-of-uninitialized-memory vulnerability. -- commit a22db32a014bd90abd28aebf26a5fc39ddb4a3cc Author: Baineng Shou <[email protected]> dma-fence: Make dma_fence_dedup_array() robust against 0-count input This commit updates dma_fence_dedup_array() to handle an input of 0 fences by returning 0 early. It also explains how the previous implementation incorrectly returned 1, which could lead to dereferencing uninitialized fence slots in arrays. > All in-tree callers currently filter num_fences == 0 before invoking > this helper (__dma_fence_unwrap_merge() bails out via the > `if (count == 0 || count == 1)` fast path; amdgpu_userq_wait_*() > cannot reach the dedup call with a zero local count because the > amdgpu_userq_wait_add_fence() helper guarantees num_fences stays in > [0, wait_info->num_fences], and wait_info->num_fences > 0 is enforced > at the ioctl entry). [Severity: Critical] This is a pre-existing issue, but the commit message claims that all in-tree callers pre-filter the num_fences == 0 case. Is this description accurate? Looking at amdgpu_userq_wait_return_fence_info() in drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c, if a user calls the ioctl with wait_info->num_fences > 0 but with no actual handles, the local num_fences variable will remain 0. Prior to this patch, dma_fence_dedup_array(fences, 0) returned 1: drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c:amdgpu_userq_wait_return_fence_info() { ... num_fences = dma_fence_dedup_array(fences, num_fences); ... } This causes the subsequent loop to read fences[0], which contains uninitialized kernel heap data from kmalloc_array(), and pass it to dma_fence_wait(): drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c:amdgpu_userq_wait_return_fence_info() { ... for (i = 0, cnt = 0; i < num_fences; i++) { ... r = dma_fence_wait(fences[i], true); ... } Does this mean this patch actually fixes a pre-existing use-of-uninitialized-memory regression that could lead to arbitrary code execution? If so, should this commit message be updated to reflect it is a security fix and include a Cc: stable tag for backporting? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
