dma_fence_is_signaled() can return true *while* the fence is still
signaling. This is fragile and has been involved in at least two bugs:
commit c8a5d5ea3ba6 ("nouveau: fix client work fence deletion race")
commit 7c703a7d3f2b ("drm/amdgpu: Fix one use-after-free of VM")
The signaled-bit is set under lock-protection but read without it.
Adding the lock has been rejected on the grounds of the lock-acquisition
shooting down cache lines of parties that merely want to check whether
the fence is already signaled.
Make sure the bit can only become visible as 1 once the callbacks have
ran. To achieve this, read the bit with test_bit_acquire() and set it
with set_bit_release().
Signed-off-by: Philipp Stanner <[email protected]>
---
drivers/dma-buf/dma-fence.c | 11 +++++++++--
include/linux/dma-fence.h | 6 +++++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index bd58688b81a7..07d499fa35ac 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -359,8 +359,7 @@ void dma_fence_signal_timestamp_locked(struct dma_fence
*fence,
dma_fence_assert_held(fence);
- if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
- &fence->flags)))
+ if (unlikely(dma_fence_test_signaled_flag(fence)))
return;
trace_dma_fence_signaled(fence);
@@ -384,6 +383,14 @@ void dma_fence_signal_timestamp_locked(struct dma_fence
*fence,
INIT_LIST_HEAD(&cur->node);
cur->func(fence, cur);
}
+
+ /*
+ * This orders the signaled-state with the test_bit_acquire() in
+ * dma_fence_test_signaled_flag(). This ensures that functions like
+ * dma_fence_is_signaled() can only ever return true once signaling is
+ * completed (i.e., after the callbacks above ran).
+ */
+ set_bit_release(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
}
EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index dd07d128adc3..61522027af41 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -496,7 +496,11 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence
*fence);
static inline bool
dma_fence_test_signaled_flag(struct dma_fence *fence)
{
- return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
+ /*
+ * The counterpart of this ordered operation can be found in
+ * dma_fence_signal_timestamp_locked().
+ */
+ return test_bit_acquire(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
}
/**
--
2.56.0
