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

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

[Baineng Shou]

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.

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).

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>
---
 drivers/dma-buf/dma-fence-unwrap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
index 53bb40e70b27..364cbf79ad73 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -97,6 +97,9 @@ int dma_fence_dedup_array(struct dma_fence **fences, int num_fences)
 {
 	int i, j;
 
+	if (!num_fences)
+		return 0;
+
 	sort(fences, num_fences, sizeof(*fences), fence_cmp, NULL);
 
 	/*
-- 
2.34.1

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

[sashiko-bot]

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 <shoubaineng@gmail.com>

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/20260629031346.3875683-1-shoubaineng@gmail.com?part=1

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

[Philipp Stanner]

On Mon, 2026-06-29 at 11:13 +0800, 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.
> 
> 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).
> 
> 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>
> ---
>  drivers/dma-buf/dma-fence-unwrap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
> index 53bb40e70b27..364cbf79ad73 100644
> --- a/drivers/dma-buf/dma-fence-unwrap.c
> +++ b/drivers/dma-buf/dma-fence-unwrap.c
> @@ -97,6 +97,9 @@ int dma_fence_dedup_array(struct dma_fence **fences, int num_fences)
>  {
>  	int i, j;
>  
> +	if (!num_fences)
> +		return 0;

Hm, since you're adding this, what about num_fences = -5 ?


Let me use this opportunity to also highlight that we should, in C, in
general get away from using int for everything.

__dma_fence_unwrap_merge() directly below uses unsigned int, as you
would expect. dma_fence_dedup_array() has almost no users. Probably we
should change that API to unsigned int, too?


Regards
P.

> +
>  	sort(fences, num_fences, sizeof(*fences), fence_cmp, NULL);
>  
>  	/*

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

[Christian König]

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.

Thanks,
Christian.


> ---
>  drivers/dma-buf/dma-fence-unwrap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c
> index 53bb40e70b27..364cbf79ad73 100644
> --- a/drivers/dma-buf/dma-fence-unwrap.c
> +++ b/drivers/dma-buf/dma-fence-unwrap.c
> @@ -97,6 +97,9 @@ int dma_fence_dedup_array(struct dma_fence **fences, int num_fences)
>  {
>         int i, j;
> 
> +       if (!num_fences)
> +               return 0;
> +
>         sort(fences, num_fences, sizeof(*fences), fence_cmp, NULL);
> 
>         /*
> --
> 2.34.1
> 

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

[Philipp Stanner]

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?

What about my comments?


P.

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@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.

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

[Philipp Stanner]

On Mon, 2026-06-29 at 10:52 +0200, Christian König wrote:
> On 6/29/26 10:49, Philipp Stanner wrote:
> > > 
> > > 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.

But if we keep it an integer for now, and if that check is added, and
it most certainly should also catch negative integers, shouldn't it?

P.

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

[Christian König]

On 6/29/26 11:06, Philipp Stanner wrote:
> On Mon, 2026-06-29 at 10:52 +0200, Christian König wrote:
>> On 6/29/26 10:49, Philipp Stanner wrote:
>>>>
>>>> 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.
> 
> But if we keep it an integer for now, and if that check is added, and
> it most certainly should also catch negative integers, shouldn't it?

Maybe with a WARN_ON(), but not as regular code path.

The thing is I think we need to backport this fix to stable kernels, but switching from signed to unsigned is only a minor cleanup when no real users are currently affected.

Christian.

> 
> P.

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

[Philipp Stanner]

On Mon, 2026-06-29 at 11:52 +0200, Christian König wrote:
> On 6/29/26 11:06, Philipp Stanner wrote:
> > On Mon, 2026-06-29 at 10:52 +0200, Christian König wrote:
> > > On 6/29/26 10:49, Philipp Stanner wrote:
> > > > > 
> > > > > 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.
> > 
> > But if we keep it an integer for now, and if that check is added,
> > and
> > it most certainly should also catch negative integers, shouldn't
> > it?
> 
> Maybe with a WARN_ON(), but not as regular code path.
> 
> The thing is I think we need to backport this fix to stable kernels,
> but switching from signed to unsigned is only a minor cleanup when no
> real users are currently affected.

Fine by me.

P.

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

[Christian König]

On 6/29/26 12:16, Philipp Stanner wrote:
> On Mon, 2026-06-29 at 11:52 +0200, Christian König wrote:
>> On 6/29/26 11:06, Philipp Stanner wrote:
>>> On Mon, 2026-06-29 at 10:52 +0200, Christian König wrote:
>>>> On 6/29/26 10:49, Philipp Stanner wrote:
>>>>>>
>>>>>> 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.
>>>
>>> But if we keep it an integer for now, and if that check is added,
>>> and
>>> it most certainly should also catch negative integers, shouldn't
>>> it?
>>
>> Maybe with a WARN_ON(), but not as regular code path.
>>
>> The thing is I think we need to backport this fix to stable kernels,
>> but switching from signed to unsigned is only a minor cleanup when no
>> real users are currently affected.
> 
> Fine by me.

Thanks, I updated the commit message a bit and added Fixes and CC stable tags and pushed the result to drm-misc-fixes.

@Baineng it would be cool if you could write a test cases for this as well. E.g. just ~10 line in drivers/dma-buf/st-dma-fence-unwrap.c.

Thanks in advance,
Christian.

> 
> P.