Re: [PATCH] dma-buf/dma-fence: Mark two callbacks as deprecated

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dma-buf: UAF race condition trap in ops->release deprecation doc
- [High] dma-buf: strict memory layout constraint imposed by ops->release deprecation
- [Low] dma-buf: misleading wait callback documentation for consumers
--

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

dma-buf/dma-fence: Mark two callbacks as deprecated

This commit deprecates the dma_fence ops->release() and ops->wait()
callbacks to facilitate driver-unload support. It adds documentation to
guide users away from implementing these callbacks and suggests alternatives.

> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index dd07d128adc3b..59af0159cd258 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h

[ ... ]

> @@ -236,6 +238,10 @@ struct dma_fence_ops {
>  	 * Implementing this callback prevents the fence from detaching after
>  	 * signaling and so it is necessary for the module providing the
>  	 * dma_fence_ops to stay loaded as long as the dma_fence exists.
> +	 *
> +	 * Deprecated for the reason mentioned above. No new users must be
> +	 * implemented. Consumers of a fence can instead notify themselves by
> +	 * registering a callback on the fence.
>  	 */

[Severity: Low]
Is this advice to consumers accurate? Consumers typically call the frontend
dma_fence_wait() function.

If the producer drops the ops->wait callback, dma_fence_wait() automatically
falls back to dma_fence_default_wait(), which handles registering
dma_fence_default_wait_cb natively.

It seems consumers do not need to change their code or manually register
callbacks, but this documentation instructs them to reinvent the core's
default wait logic.

>  	signed long (*wait)(struct dma_fence *fence,
>  			    bool intr, signed long timeout);
> @@ -243,6 +249,8 @@ struct dma_fence_ops {
>  	/**
>  	 * @release:
>  	 *
> +	 * DEPRECATED!
> +	 *
>  	 * Called on destruction of fence to release additional resources.
>  	 * Can be called from irq context.  This callback is optional. If it is
>  	 * NULL, then dma_fence_free() is instead called as the default

[Severity: High]
Does forbidding new implementations of ops->release() impose an unstated
memory layout constraint?

If a driver omits ops->release(), dma_fence_release() automatically falls
back to dma_fence_free(), which calls kfree_rcu(fence, rcu).

The kfree_rcu() function requires the exact base pointer returned by the
allocator. Drivers commonly embed struct dma_fence inside a larger custom
structure.

If the fence is at a non-zero offset, passing its pointer to kfree_rcu()
will attempt to free an interior pointer, which can corrupt the memory
allocator.

Could this default behavior be problematic without warning developers about
this layout requirement?

> @@ -254,6 +262,12 @@ struct dma_fence_ops {
>  	 *
>  	 * If the callback is implemented the memory backing the dma_fence
>  	 * object must be freed RCU safe.
> +	 *
> +	 * Deprecated because it prevents the producer of a fence from
> +	 * unloading. No new users must be implemented. Parties with a
> +	 * hypothetical need for this callback can instead simply and directly
> +	 * perform their custom release operations one RCU grace period after
> +	 * they have signaled the fence.
>  	 */

[Severity: High]
Could this suggested cleanup pattern lead to a use-after-free?

Signaling a fence does not guarantee all kref references are dropped,
as consumers like sync_file or other drivers might still hold references.

If a producer frees the containing structure asynchronously after signaling
instead of waiting for the kref to reach zero, any consumer that subsequently
calls dma_fence_put() will trigger dma_fence_release() on freed memory.

Even if the structure is left to dma_fence_free(), tearing down associated
sub-resources before kref reaches zero creates race conditions with
consumers.

>  	void (*release)(struct dma_fence *fence);

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