Re: [PATCH 01/12] dma-buf: Add reference counting to dma_resv

[Matthew Brost <matthew.brost@intel.com>]

On Sat, Jul 11, 2026 at 03:10:58PM +0200, Danilo Krummrich wrote:
> On Sat Jul 11, 2026 at 3:27 AM CEST, Matthew Brost wrote:
> > On Fri, Jul 10, 2026 at 08:52:41PM +0200, Christian König wrote:
> >> This provides clearer ownership semantics and makes the code more
> >> maintainable by removing the embedded allocation hack.
> >> 
> >
> > This looks a lot better to me. In particular, I agree with the last
> > sentence in the commit message.
> 
> I have to disagree with this, it is the opposite. As long as the struct

Daniilo is of course correct here, completely missed this when I looked.

> dma_resv::allocated fields and the corresponding semantics exists, this does
> result into less clear ownership semantics.
> 
> When the dma_resv is embedded in another object the reference count becomes
> meaningless. If the object embedding the dma_resv is freed it doesn't matter
> whether I have a reference count, it would a UAF regardless.
> 

Yes, I agree. The allocated field would need to be dropped to make this
viable, and we would disallow embedding a dma-resv object into other
objects (which I believe is the suggestion).

This doesn't look too painful, as I can only find two instances of
embedding in the kernel: drm_gem_object and i915_address_space and
handful of stack variables.

Matt

> It is misleading (and hence error prone) to have an API where one can obtain a
> reference count of an object where the underlying memory can be freed regardless
> of the obtained reference count.
> 
> A refernece count represents a shared ownership model, which is undermined if
> the underlying memory is not owned by the reference count.
> 
> That said, I don't mind the reference count, but we can't mix up exclusive
> ownership (embedding a structure) and shared ownership (reference count).
> 
> >> +static void dma_resv_release(struct kref *kref)
> >>  {
> >> -	/*
> >> -	 * This object should be dead and all references must have
> >> -	 * been released to it, so no need to be protected with rcu.
> >> -	 */
> >> +	struct dma_resv *obj = container_of(kref, struct dma_resv, refcount);
> >> +
> >>  	dma_resv_list_free(rcu_dereference_protected(obj->fences, true));
> >>  	ww_mutex_destroy(&obj->lock);
> >> +	if (obj->allocated)
> >> +		kfree(obj);
> >> +}