On 6/29/26 10:49, Philipp Stanner wrote:
On Mon, 2026-06-29 at 10:45 +0200, Christian König wrote:
On 6/29/26 05:13, Baineng Shou wrote:
dma_fence_dedup_array() returns 1 when called with num_fences == 0: the for-loop body never executes, j stays at 0, and the final `return ++j` yields 1. This contradicts both the kernel-doc ("Return: Number of unique fences remaining in the array") and the natural expectation that 0 input gives 0 output.
Good catch.
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).
That's not correct, wait_info->num_fences is just the maximum amount of fences we return.
It is perfectly possible that amdgpu never finds any fences to add to the array.
However, dma_fence_dedup_array() is EXPORT_SYMBOL_GPL, so any future caller that forgets to pre-filter the zero case will get a misleading return value of 1. Depending on how that caller uses the result, it could dereference an uninitialized fence slot in the array, since the caller's array may have been allocated but not yet populated.
Make the contract match the documentation by returning 0 early. This also skips an unnecessary sort() call on an empty array.
Signed-off-by: Baineng Shou shoubaineng@gmail.com
Reviewed-by: Christian König christian.koenig@amd.com
I will add a CC stable before pushing to drm-misc-fixes.
No offense intended or taken, but don't the DRM rules say that things do not get merged while there are outstanding concerns or significant points in review feedback?
I haven't seen that before writing the response.
I usually go over my mails till the end and wait a couple of hours before pushing anything.
What about my comments?
Looks valid to me as well, but I think that is a separate issue.
If I'm not completely mistaken we should use size_t instead of int for array sizes all around the place in those functions.
Regards, Christian.
P.