[PATCH 1/7] dma-buf: add WARN_ON() illegal dma-fence signaling

Philipp Stanner pstanner at redhat.com
Wed Sep 11 09:44:15 UTC 2024


On Wed, 2024-09-11 at 10:58 +0200, Christian König wrote:
> Calling the signaling a NULL fence is obviously a coding error in a
> driver. Those functions unfortunately just returned silently without
> raising a warning.

Good catch

> 
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/dma-buf/dma-fence.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-
> fence.c
> index 0393a9bba3a8..325a263ac798 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -412,7 +412,7 @@ int dma_fence_signal_timestamp(struct dma_fence
> *fence, ktime_t timestamp)
>  	unsigned long flags;
>  	int ret;
>  
> -	if (!fence)
> +	if (WARN_ON(!fence))
>  		return -EINVAL;

While one can do that, as far as I can see there are only a hand full
of users of that function anyways.

Couldn't one (additionally) add the error check of
dma_fenc_signal_timestapm() to those? Like in
dma_fenc_allocate_private_stub().

It seems some of them are void functions, though. Hm.
There is also the attribute __must_check that could be considered now
or in the future for such functions.

Regards,
P.


>  
>  	spin_lock_irqsave(fence->lock, flags);
> @@ -464,7 +464,7 @@ int dma_fence_signal(struct dma_fence *fence)
>  	int ret;
>  	bool tmp;
>  
> -	if (!fence)
> +	if (WARN_ON(!fence))
>  		return -EINVAL;
>  
>  	tmp = dma_fence_begin_signalling();



More information about the dri-devel mailing list