On Wed, Oct 11, 2023 at 08:03:43AM -0600, Gustavo A. R. Silva wrote:
> Currently, a NULL pointer dereference will happen in function
> `dma_fence_enable_sw_signaling()` (at line 615), in case `chain`
> is not allocated in `mock_chain()` and this function returns
> `NULL` (at line 86). See below:
>
> drivers/dma-buf/st-dma-fence-chain.c:
> 86 chain = mock_chain(NULL, f, 1);
> 87 if (!chain)
> 88 err = -ENOMEM;
> 89
> 90 dma_fence_enable_sw_signaling(chain);
Instead of the larger patch, should line 88 here just do a "return
-ENOMEM" instead?
-Kees
>
> drivers/dma-buf/dma-fence.c:
> 611 void dma_fence_enable_sw_signaling(struct dma_fence *fence)
> 612 {
> 613 unsigned long flags;
> 614
> 615 spin_lock_irqsave(fence->lock, flags);
> ^^^^^^^^^^^
> |
> NULL pointer reference
> if fence == NULL
>
> 616 __dma_fence_enable_signaling(fence);
> 617 spin_unlock_irqrestore(fence->lock, flags);
> 618 }
>
> Fix this by adding a NULL check before dereferencing `fence` in
> `dma_fence_enable_sw_signaling()`. This will prevent any other NULL
> pointer dereference when the `fence` passed as an argument is `NULL`.
>
> Addresses-Coverity: ("Dereference after null check")
> Fixes: d62c43a953ce ("dma-buf: Enable signaling on fence for selftests")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
> ---
> drivers/dma-buf/dma-fence.c | 9 ++++++++-
> include/linux/dma-fence.h | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 8aa8f8cb7071..4d2f13560d0f 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -607,14 +607,21 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence)
> * This will request for sw signaling to be enabled, to make the fence
> * complete as soon as possible. This calls &dma_fence_ops.enable_signaling
> * internally.
> + *
> + * Returns 0 on success and a negative error value when @fence is NULL.
> */
> -void dma_fence_enable_sw_signaling(struct dma_fence *fence)
> +int dma_fence_enable_sw_signaling(struct dma_fence *fence)
> {
> unsigned long flags;
>
> + if (!fence)
> + return -EINVAL;
> +
> spin_lock_irqsave(fence->lock, flags);
> __dma_fence_enable_signaling(fence);
> spin_unlock_irqrestore(fence->lock, flags);
> +
> + return 0;
> }
> EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index ebe78bd3d121..1e4025e925e6 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -399,7 +399,7 @@ int dma_fence_add_callback(struct dma_fence *fence,
> dma_fence_func_t func);
> bool dma_fence_remove_callback(struct dma_fence *fence,
> struct dma_fence_cb *cb);
> -void dma_fence_enable_sw_signaling(struct dma_fence *fence);
> +int dma_fence_enable_sw_signaling(struct dma_fence *fence);
>
> /**
> * dma_fence_is_signaled_locked - Return an indication if the fence
> --
> 2.34.1
>
>
--
Kees Cook
