Re: [PATCH 3/8] rust: DmaFence: Don't drop on signal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] rust: dma_fence: missing RCU grace period in
  DriverFence::drop() [1]
--

commit ee6d42825838039a4b13a90afb76009d7c94efbd
Author: Philipp Stanner <phasta@kernel.org>

rust: DmaFence: Don't drop on signal

This commit changes DriverFence::signal() to take &self instead of consuming
self, and decrements nr_of_unsignaled_fences only when the fence had not yet
been signaled.

> diff --git a/rust/kernel/dma_buf/dma_fence.rs b/rust/kernel/dma_buf/dma_fence.rs
> index 897308f60dd3c..c04b68fe67fe3 100644
> --- a/rust/kernel/dma_buf/dma_fence.rs
> +++ b/rust/kernel/dma_buf/dma_fence.rs
> @@ -820,7 +820,7 @@ pub fn as_fence(&self) -> &Fence {
>      }
>  
>      /// Signal the fence. This will invoke all registered callbacks.
> -    pub fn signal(self, res: Result) {
> +    pub fn signal(&self, res: Result) {
>          let fence = self.as_fence().lock();
>  
>          // SAFETY: `fence` is valid because `self` is valid. The lock must be
> @@ -831,14 +831,15 @@ pub fn signal(self, res: Result) {
>                  // must not have been signaled yet, which we check directly above.
>                  unsafe { bindings::dma_fence_set_error(fence.as_raw(), err.to_errno()) };
>              }
> +
> +            // SAFETY: `self.data` is valid because `self` is valid.
> +            let fctx = unsafe { self.data.as_ref().fctx };
> +            let _ = fctx.nr_of_unsignaled_fences.fetch_sub(1, Relaxed);
> +
>              // SAFETY: `fence` is valid because `self` is valid. The lock must
>              // be held, which we acquired above.
>              unsafe { bindings::dma_fence_signal_locked(fence.as_raw()) };
>          }
> -
> -        // SAFETY: `self.data` is valid because `self` is valid.
> -        let fctx = unsafe { self.data.as_ref().fctx };
> -        let _ = fctx.nr_of_unsignaled_fences.fetch_sub(1, Relaxed);
>      }
>  }

[Severity: Critical]
This problem wasn't introduced by this patch, but can FenceContext::drop()
free the context before an RCU grace period has elapsed [1] when
T::FenceDataType does not require drop glue?

FenceContext::drop() calls rcu_barrier() to wait for in-flight RCU callbacks
and concurrent dma_fence_ops readers before freeing the FenceContext.
However, in DriverFence::drop():

rust/kernel/dma_buf/dma_fence.rs:DriverFence::drop() {
    ...
        if !core::mem::needs_drop::<T::FenceDataType>() {
            ...
            unsafe { bindings::dma_fence_put(self.as_raw()) };
            return;
        }
    ...
        unsafe { bindings::call_rcu(rcu_head_ptr, Some(drop_driver_fence_data::<T>)) };
}

When needs_drop is false and an external consumer still holds a reference to
the underlying dma_fence, dma_fence_put() does not schedule the RCU release of
the fence and DriverFence::drop() returns without calling call_rcu().
Because rcu_barrier() only waits for already-queued RCU callbacks rather than
a grace period, FenceContext::drop() can return immediately while a concurrent
RCU reader in dma_fence_driver_name() or dma_fence_timeline_name() is still
running:

CPU0                                  CPU1
dma_fence_driver_name()
  rcu_read_lock()
  !dma_fence_test_signaled_flag()
                                      DriverFence::signal()
                                        nr_of_unsignaled_fences.fetch_sub(1)
                                        dma_fence_signal_locked()
                                      DriverFence::drop()
                                        // !needs_drop::<T::FenceDataType>()
                                        dma_fence_put() // refcount > 0
                                      FenceContext::drop()
                                        rcu_barrier() // no callback queued
                                      // FenceContext is freed
  FenceContext::get_driver_name()
    fctx.driver_name.as_char_ptr()    <-- use-after-free

[1] https://sashiko.dev/#/bug/linux-40274ecc-9f61-4d35-b6b3-1ceb72f48227

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106140752.520386-2-phasta@kernel.org?part=3