Re: [PATCH] dma-fence: Make dma_fence_dedup_array() robust against 0-count input

[Christian König]

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@xxxxxxxxx>
>>
>> Reviewed-by: Christian König <christian.koenig@xxxxxxx>
>>
>> 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.