Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [Critical] dma_fence_dedup_array() incorrectly returns 1 when called with an 
empty array (num_fences == 0), causing callers to read uninitialized memory.
--

commit 49edb87cad117674d9feb38e78cfaf85af7320cb
Author: Shahyan Soltani <[email protected]>

dma_buf: change unsigned int and int types into size_t

This commit changes the integer types of variables in dma_fence_dedup_array()
and __dma_fence_unwrap_merge() to size_t to resolve inconsistencies. It also
updates the return type of dma_fence_dedup_array() accordingly.

> diff --git a/drivers/dma-buf/dma-fence-unwrap.c 
> b/drivers/dma-buf/dma-fence-unwrap.c
> index 53bb40e70b277..65e87d263c3ac 100644
> --- a/drivers/dma-buf/dma-fence-unwrap.c
> +++ b/drivers/dma-buf/dma-fence-unwrap.c
> @@ -93,9 +93,9 @@ static int fence_cmp(const void *_a, const void *_b)
>   *
>   * Return: Number of unique fences remaining in the array.
>   */
> -int dma_fence_dedup_array(struct dma_fence **fences, int num_fences)
> +size_t dma_fence_dedup_array(struct dma_fence **fences, size_t num_fences)
>  {
> -     int i, j;
> +     size_t i, j;

[Severity: Critical]
This is a pre-existing issue, but does dma_fence_dedup_array() return the
wrong value when called with an empty array?

If num_fences is 0, the loop condition (i < num_fences) is false and skipped.
The function then unconditionally returns ++j, which evaluates to 1 instead
of 0:

drivers/dma-buf/dma-fence-unwrap.c:dma_fence_dedup_array() {
        ...
        j = 0;
        for (i = 1; i < num_fences; i++) {
                if (fences[i]->context == fences[j]->context)
                        dma_fence_put(fences[i]);
                else
                        fences[++j] = fences[i];
        }

        return ++j;
}

Could this lead to an uninitialized memory dereference in callers?

For example, in amdgpu_userq_wait_return_fence_info(), if 
wait_info->num_fences is 1 but zero handles are provided, it can pass a
1-element uninitialized fences array with num_fences = 0.

The caller relies on the return value of dma_fence_dedup_array() to know how
many fences to process:

drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c:amdgpu_userq_wait_return_fence_info()
 {
        ...
        num_fences = dma_fence_dedup_array(fences, num_fences);
        ...
        for (i = 0, cnt = 0; i < num_fences; i++) {
                userq_fence = to_amdgpu_userq_fence(fences[i]);
                ...
}

If dma_fence_dedup_array() returns 1 when passed 0, the caller will iterate
and dereference the uninitialized kmalloc memory at fences[0], which could
trigger a kernel panic.

[ ... ]

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to