Re: [RFC PATCH v6 3/5] rust: DmaFence: Don't drop on signal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma_fence: counter underflow in DriverFence::signal leads to panic or UAF
--

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

rust: DmaFence: Don't drop on signal

This commit changes the DriverFence::signal method to take a shared reference
instead of consuming the object. This allows multiple or concurrent calls to
signal() on the same fence object, avoiding unnecessary drops in atomic
contexts.

> diff --git a/rust/kernel/dma_buf/dma_fence.rs b/rust/kernel/dma_buf/dma_fence.rs
> index 0028124e6aa64..b67121c7abd41 100644
> --- a/rust/kernel/dma_buf/dma_fence.rs
> +++ b/rust/kernel/dma_buf/dma_fence.rs
> @@ -806,23 +806,23 @@ 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
[ ... ]
>              // 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: High]
Since signal() now takes a shared reference and can be called multiple times,
will this unconditional decrement of nr_of_unsignaled_fences cause an
underflow?

When called repeatedly for the same fence, this decrement executes outside
the if !signaled block and will occur on every call.

This could cause the counter to artificially reach 0 and bypass the panic
check in FenceContext::drop(), potentially allowing the context to be freed
while unsignaled fences still exist.

Could this result in a use-after-free when those fences are later queried by
the C backend?

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