Re: [RFC PATCH 5/6] dma-buf: dma-fence: Close race in dma_fence_is_signaled() by default

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Oct 29, 2026 at 10:55:42AM +0100, Philipp Stanner wrote:
> dma_fence_is_signaled() contains a lockless check of the signaled-bit,
> which is set under lock protection. This is a race condition which makes
> the API fragile and has caused at least two bugs already, addressed in:
> 
> 	commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
> 	commit 7c703a7d3f2b ("drm/amdgpu: Fix one use-after-free of VM")
> 
> It has been argued by some that this race cannot be closed for the
> entire kernel through locking because this would lead to a cache line
> invalidation for all parties reading that fence. While it remains
> unclear why these parties don't simply let themselves be notified about
> the fence being signaled instead of checking this repeatedly manually,
> there is a way to preserve this behavior in any case:
> 
> This patch adds a new flag for dma_fence_init(),
> DMA_FENCE_INIT_FLAG_RACY_SIGNALED_CHECK. If this flag is not set, the
> spinlock is taken and dma_fence_is_signaled() becomes perfectly
> synchronized: it can only return true once signaling of the fence is
> completed.
> 
> Disable the racy check for everyone by default. Should some parties
> really experience severe performance degredations because of this, they
> can trivially re-enable the race condition for their driver by setting
> the flag in dma_fence_init().
> 

My 2 cents - anyone arguing about cachelines probably has broken code to
begin with. I'd say don't give anyone an option to opt into a racey
check, if this regressing some peoples code the answer is likely
structurally fix your broken code.

Matt

> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/dma-buf/dma-fence.c |  2 ++
>  include/linux/dma-fence.h   | 22 ++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index f7c084926ae6..1ae8c7aade39 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -1119,6 +1119,8 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>  	/* Mapp the init_flags to the fence-internal flags. */
>  	if (test_bit(DMA_FENCE_INIT_FLAG_SEQNO64, &init_flags))
>  		set_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence_flag_bits);
> +	if (test_bit(DMA_FENCE_INIT_FLAG_RACY_SIGNALED_CHECK, &init_flags))
> +		set_bit(DMA_FENCE_FLAG_RACY_SIGNALED_CHECK_BIT, &fence_flag_bits);
>  
>  	__dma_fence_init(fence, ops, lock, context, seqno, fence_flag_bits);
>  }
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 52e7ddfd2d06..ee6e8de3ebc6 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -115,6 +115,7 @@ enum dma_fence_flag_bits {
>  	DMA_FENCE_FLAG_SIGNALED_BIT,
>  	DMA_FENCE_FLAG_TIMESTAMP_BIT,
>  	DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> +	DMA_FENCE_FLAG_RACY_SIGNALED_CHECK_BIT,
>  
>  	DMA_FENCE_FLAG_USER_BITS, /* must always be last member */
>  };
> @@ -124,9 +125,12 @@ enum dma_fence_flag_bits {
>   * These flags can be passed in dma_fence_init().
>   *
>   * @DMA_FENCE_INIT_FLAG_SEQNO64: Use 64-bit sequence numbers.
> + * @DMA_FENCE_INIT_FLAG_RACY_SIGNALED_CHECK:
> + * Legacy flag. Enables a race condition in dma_fence_is_signaled().
>   */
>  enum dma_fence_init_flags {
>  	DMA_FENCE_INIT_FLAG_SEQNO64,
> +	DMA_FENCE_INIT_FLAG_RACY_SIGNALED_CHECK,
>  
>  	/* Must always be last member. */
>  	DMA_FENCE_INIT_FLAG_NR_OF_FLAGs,
> @@ -568,9 +572,27 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>  static inline bool
>  dma_fence_is_signaled(struct dma_fence *fence)
>  {
> +	const bool racy_check = test_bit(DMA_FENCE_FLAG_RACY_SIGNALED_CHECK_BIT,
> +					 &fence->flags);
>  	const struct dma_fence_ops *ops;
> +	bool signaled = false;
> +	unsigned long flags;
>  
> +	/*
> +	 * For legacy reasons, there is a race window here against parties
> +	 * signaling the fence. They might still be doing that right now.
> +	 *
> +	 * For most users, this window shall be closed, but there are parties
> +	 * for whom the cache-line invalidation by taking the lock might be so
> +	 * significant that they prefer to accept the race at their own risk.
> +	 */
> +	if (!racy_check)
> +		dma_fence_lock_irqsave(fence, flags);
>  	if (dma_fence_test_signaled_flag(fence))
> +		signaled = true;
> +	if (!racy_check)
> +		dma_fence_unlock_irqrestore(fence, flags);
> +	if (signaled)
>  		return true;
>  
>  	rcu_read_lock();
> -- 
> 2.56.0
>