All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Baineng Shou <shoubaineng@gmail.com>,
	Sumit Semwal <sumit.semwal@linaro.org>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>,
	Philipp Stanner <phasta@kernel.org>,
	Akash Goel <akash.goel@arm.com>,
	linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] dma-fence: Make dma_fence_dedup_array() robust against 0-count input
Date: Mon, 29 Jun 2026 10:45:23 +0200	[thread overview]
Message-ID: <becd29b5-9e40-4104-b6c9-3d91e2ddddff@amd.com> (raw)
In-Reply-To: <20260629031346.3875683-1-shoubaineng@gmail.com>

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
> 


  parent reply	other threads:[~2026-06-29  8:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29  3:13 [PATCH] dma-fence: Make dma_fence_dedup_array() robust against 0-count input Baineng Shou
2026-06-29  3:23 ` sashiko-bot
2026-06-29  6:59 ` Philipp Stanner
2026-06-29  8:45 ` Christian König [this message]
2026-06-29  8:49   ` Philipp Stanner
2026-06-29  8:52     ` Christian König
2026-06-29  9:06       ` Philipp Stanner
2026-06-29  9:52         ` Christian König
2026-06-29 10:16           ` Philipp Stanner
2026-06-29 11:51             ` Christian König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=becd29b5-9e40-4104-b6c9-3d91e2ddddff@amd.com \
    --to=christian.koenig@amd.com \
    --cc=akash.goel@arm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=phasta@kernel.org \
    --cc=shoubaineng@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tursulin@ursulin.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.