On Tue, 2026-06-16 at 10:28 +0200, Philipp Stanner wrote: > > […]
> Add abstractions for dma_fence in Rust. > > Signed-off-by: Philipp Stanner <[email protected]> RFC for one thing I'm worried about below > --- > rust/bindings/bindings_helper.h | 1 + > rust/helpers/dma_fence.c | 48 ++ > rust/helpers/helpers.c | 1 + > rust/kernel/dma_buf/dma_fence.rs | 884 +++++++++++++++++++++++++++++++ > rust/kernel/dma_buf/mod.rs | 14 + > rust/kernel/lib.rs | 1 + > 6 files changed, 949 insertions(+) > create mode 100644 rust/helpers/dma_fence.c > create mode 100644 rust/kernel/dma_buf/dma_fence.rs > create mode 100644 rust/kernel/dma_buf/mod.rs > > > […] > + > +/// The receiving counterpart of a [`DriverFence`], designed to register > callbacks > +/// on, check the signalled state etc. A [`Fence`] cannot be signalled. > +/// A [`Fence`] is always refcounted. > +pub struct Fence { > + /// The actual dma_fence passed to C. > + inner: Opaque<bindings::dma_fence>, > +} > + > > […] > + > +#[repr(C)] // Necessary to guarantee that `inner` always comes first so that > we can cast. > +#[pin_data] > +struct DriverFenceData<T: Send + Sync + FenceCtxOps> { > + /// Callback head for dropping this in a deferred manner through RCU. > + rcu_head: bindings::callback_head, > + #[pin] > + /// The inner fence. > + inner: Fence, > + /// Pointer to access the FenceCtx. Useful for obtaining name parameters. > + fctx: Arc<FenceCtx<T>>, > + /// The API user's data. This must either not need drop, or must delay > its > + /// drop by a grace period. It is essential that the data only performs > + /// operations legal in atomic context in its [`Drop`] implementation. > + #[pin] > + data: T::FenceDataType, > +} > + > > > […] > +pub struct DriverFence<T: Send + Sync + FenceCtxOps> { > + /// The actual content of the fence. Lives in a raw pointer so that its > + /// memory can be managed independently. Valid until both the > [`DriverFence`] > + /// and all associated [`Fence`]s have disappeared. > + data: NonNull<DriverFenceData<T>>, > +} > + > + > […] > } > + > +impl<T: Send + Sync + FenceCtxOps> Drop for DriverFence<T> { > + fn drop(&mut self) { > + let fence = self.as_raw(); > + let mut fence_flags: usize = 0; > + let flag_ptr = &raw mut fence_flags; > + > + // SAFETY: `fence` is valid until the `call_rcu()` below. `flag_ptr` > is > + // merely a pointer to an integer, which lives as long as this > function. > + unsafe { bindings::dma_fence_lock_irqsave(fence, flag_ptr) }; > + > + // Use dma_fence_test_signaled_flag() instead of > dma_fence_is_signaled_locked() > + // because the C backend wants to get rid of the latter. > + > + // SAFETY: `fence` is valid until the `call_rcu()` below. > + let signaled: bool = unsafe { > bindings::dma_fence_test_signaled_flag(fence) }; > + if warn_on!(!signaled) { > + // SAFETY: `fence` is valid until the `call_rcu()` below. The > fence > + // must not have been signaled yet, which we check directly > above. > + unsafe { bindings::dma_fence_set_error(fence, > ECANCELED.to_errno()) }; > + // SAFETY: `fence` is valid until the `call_rcu()` below. The > lock must > + // be held, which we acquired above. > + unsafe { bindings::dma_fence_signal_locked(fence) }; > + } > + // SAFETY: `fence` is valid until the `call_rcu()` below. `flag_ptr` > is > + // merely a pointer to an integer, which lives as long as this > function. > + // The lock must be held, which we acquired above. > + unsafe { bindings::dma_fence_unlock_irqrestore(fence, flag_ptr) }; > + > + // SAFETY: Valid because `self` is valid. > + let rcu_head_ptr = unsafe { &raw mut (*self.data.as_ptr()).rcu_head > }; > + > + // `DriverFenceData` but could be accessed through some dma_fence > callbacks > + // right now. Access is being revoked in principle above by > signalling > + // the fence, but since the C backend does not guarantee perfect full > + // synchronization, we have to wait for one grace period to ensure > that > + // all accessors of `DriverFenceData` (through the dma_fence_ops > accessible > + // through a `Fence`) are gone. > + > + // SAFETY: `call_rcu()` is always safe to be called. `rcu_head_ptr` > was created > + // validly above. The module must perform a `synchronize_rcu()` or > + // `rcu_barrier()` call to guard against module unload. > + unsafe { bindings::call_rcu(rcu_head_ptr, > Some(drop_driver_fence_data::<T>)) }; > + } > +} > + > +// TODO: > +// The entire call_rcu() mechanism in the drop above and the code below > would be > +// unnecessary if C's dma_fence_signal() could be reworked in a way that > after it > +// ran, the caller knows that no fence_ops callbacks can be running anymore. > +// In other words, if the dma_fence backend would use its spinlock for full > +// synchronization. > +// > +// Then we could move the drop_in_place() and dma_fence_put() upwards into > the > +// drop() implementation and call it a day. > + > +/// Finally really drop this `DriverFence<T>` > +/// > +/// # Safety > +/// > +/// `head` references the `rcu_head` field of an `DriverFenceData<T>`. All > +/// accessors to that `DriverFenceData<T>` must be gone by now. This must be > +/// ensured by signalling the associated `DriverFence<T>` and then waiting > +/// for a grace period until calling this function here. > +unsafe extern "C" fn drop_driver_fence_data<T: Send + Sync + FenceCtxOps>( > + head: *mut bindings::callback_head, > +) { > + // SAFETY: Caller provides a pointer to the `rcu_head` field of a > `DriverFenceData<C>`. > + let fence_data = unsafe { container_of!(head, DriverFenceData<T>, > rcu_head) }; > + > + // SAFETY: `fence_data` was created validly above. All the fence's data > will > + // only drop below, but the raw pointer to the raw C `dma_fence` remains > + // valid because the reference count is only decremented at the end of > the > + // function. > + let fence = unsafe { (*fence_data).inner.inner.get() }; > + > + // SAFETY: A grace period has passed. All accessors to that > `DriverFenceData` > + // are gone now. We have exclusive access and can now drop it. > + unsafe { drop_in_place(fence_data) }; Here we would drop DriverFenceData. We do it like that (RCU delay) now because of two-fold reasoning: a) dropping too soon could deallocate the FenceCtx, causing UAF. b) drop_in_place() invalidates the fctx pointer, so the Rust compiler could hypothetically write some random data for CPU pipelining etc. reasons into the pointer, despite it still being used from the backend functions. So what I'm wondering about is whether we also solved that problem for DriverFenceData.inner.inner, i.e., the C dma_fence which resides in that memory, which can be in use by an arbitrary number of refcounting Fence's Does Opaque guarantee that the backing memory within DriverFenceData will never be interfered with in an undefined behavior way? The documentation does not explicitly seem to state that. P.
