On 16/10/2025 09:56, Tvrtko Ursulin wrote:
On 13/10/2025 14:48, Christian König wrote:
When neither a release nor a wait operation is specified it is possible
to let the dma_fence live on independent of the module who issued it.
This makes it possible to unload drivers and only wait for all their
fences to signal.
Have you looked at whether the requirement to not have the release and
wait callbacks will exclude some drivers from being able to benefit from
this?
I had a browse and this seems to be the situation:
Custom .wait:
- radeon, qxl, nouveau, i915
Those would therefore still be vulnerable to the unbind->unload
sequence. Actually not sure about qxl, but other three are PCI so in
theory at least. I915 at least supports unbind and unload.
Custom .release:
- vgem, nouveau, lima, pvr, i915, usb-gadget, industrialio, etnaviv, xe
Out of those there do not actually need a custom release and could
probably be weaned off it:
- usb-gadget, industrialio, etnaviv, xe
(Xe would lose a debug assert and some would have their kfrees replaced
with kfree_rcu. Plus build time asserts added the struct dma-fence
remains first in the respective driver structs. It sounds feasible.)
That would leave us with .release in:
- vgem, nouveau, lima, pvr, i915
Combined list of custom .wait + .release:
- radeon, qxl, nouveau, i915, lima, pvr, vgem
From those the ones which support unbind and module unload would remain
potentially vulnerable to use after free.
It doesn't sound great to only solve it partially but maybe it is a
reasonable next step. Where could we go from there to solve it for everyone?
Regards,
Tvrtko
Signed-off-by: Christian König <[email protected]>
---
drivers/dma-buf/dma-fence.c | 16 ++++++++++++----
include/linux/dma-fence.h | 4 ++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 982f2b2a62c0..39f73edf3a33 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -374,6 +374,14 @@ int dma_fence_signal_timestamp_locked(struct
dma_fence *fence,
&fence->flags)))
return -EINVAL;
+ /*
+ * When neither a release nor a wait operation is specified set
the ops
+ * pointer to NULL to allow the fence structure to become
independent
+ * who originally issued it.
+ */
+ if (!fence->ops->release && !fence->ops->wait)
+ RCU_INIT_POINTER(fence->ops, NULL);
+
/* Stash the cb_list before replacing it with the timestamp */
list_replace(&fence->cb_list, &cb_list);
@@ -513,7 +521,7 @@ dma_fence_wait_timeout(struct dma_fence *fence,
bool intr, signed long timeout)
rcu_read_lock();
ops = rcu_dereference(fence->ops);
trace_dma_fence_wait_start(fence);
- if (ops->wait) {
+ if (ops && ops->wait) {
/*
* Implementing the wait ops is deprecated and not supported
for
* issuer independent fences, so it is ok to use the ops
outside
@@ -578,7 +586,7 @@ void dma_fence_release(struct kref *kref)
}
ops = rcu_dereference(fence->ops);
- if (ops->release)
+ if (ops && ops->release)
ops->release(fence);
else
dma_fence_free(fence);
@@ -614,7 +622,7 @@ static bool __dma_fence_enable_signaling(struct
dma_fence *fence)
rcu_read_lock();
ops = rcu_dereference(fence->ops);
- if (!was_set && ops->enable_signaling) {
+ if (!was_set && ops && ops->enable_signaling) {
trace_dma_fence_enable_signal(fence);
if (!ops->enable_signaling(fence)) {
@@ -1000,7 +1008,7 @@ void dma_fence_set_deadline(struct dma_fence
*fence, ktime_t deadline)
rcu_read_lock();
ops = rcu_dereference(fence->ops);
- if (ops->set_deadline && !dma_fence_is_signaled(fence))
+ if (ops && ops->set_deadline && !dma_fence_is_signaled(fence))
ops->set_deadline(fence, deadline);
rcu_read_unlock();
}
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 38421a0c7c5b..e1ba1d53de88 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -425,7 +425,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
rcu_read_lock();
ops = rcu_dereference(fence->ops);
- if (ops->signaled && ops->signaled(fence)) {
+ if (ops && ops->signaled && ops->signaled(fence)) {
rcu_read_unlock();
dma_fence_signal_locked(fence);
return true;
@@ -461,7 +461,7 @@ dma_fence_is_signaled(struct dma_fence *fence)
rcu_read_lock();
ops = rcu_dereference(fence->ops);
- if (ops->signaled && ops->signaled(fence)) {
+ if (ops && ops->signaled && ops->signaled(fence)) {
rcu_read_unlock();
dma_fence_signal(fence);
return true;
