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 <[email protected]>
> ---
> 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
>
