Re: [PATCH 10/10] dma-buf: Inline dma_resv_init and remove allocated flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Synchronous freeing of shared dma_resv objects causes Use-After-Free for concurrent lockless RCU readers.
- [High] Drivers like i915 leak dynamically allocated dma_resv objects when destroying GEM objects because they bypass standard DRM cleanup routines.
--

commit d696682993c0c4cb40a8a895d00e6e7b158307dd
Author: Christian König <ckoenig.leichtzumerken@gmail.com>

dma-buf: Inline dma_resv_init and remove allocated flag

This commit migrates all dma_resv object initializations directly into
dma_resv_alloc() and removes the dma_resv_init() function. It also makes
dma_resv objects strictly dynamically allocated, removing the allocated
flag and simplifying the cleanup in dma_resv_release() to an unconditional
kfree().

> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 1251b18e1b540..b553ad9c87445 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
[ ... ]
> -/**
> - * dma_resv_init - initialize a reservation object
> - * @obj: the reservation object
> - */
> -void dma_resv_init(struct dma_resv *obj)
> -{
> -	kref_init(&obj->refcount);
> -	obj->allocated = false;
> -	ww_mutex_init(&obj->lock, &reservation_ww_class);
> -
> -	RCU_INIT_POINTER(obj->fences, NULL);
> -}
> -EXPORT_SYMBOL(dma_resv_init);

[Severity: High]
Does replacing the static initialization with dynamic allocation via
dma_resv_alloc() cause memory leaks in drivers that bypass standard DRM
cleanup routines?

Drivers like i915 use custom cleanup logic. For instance,
i915_gem_object_alloc() calls drm_gem_private_object_init(), which
dynamically allocates obj->resv via dma_resv_alloc(). When the object is
freed, i915 uses __i915_gem_free_object(), which bypasses
drm_gem_object_release() and only calls __i915_gem_object_fini().

Since dma_resv_fini() was removed from __i915_gem_object_fini() and not
replaced with a call to dma_resv_put(obj->base.resv), the reference is
never dropped. Could this permanently leak the dma_resv allocation on every
GEM object destruction?

[ ... ]
> @@ -159,8 +146,7 @@ static void dma_resv_release(struct kref *kref)
>  
>  	dma_resv_list_free(rcu_dereference_protected(obj->fences, true));
>  	ww_mutex_destroy(&obj->lock);
> -	if (obj->allocated)
> -		kfree(obj);
> +	kfree(obj);
>  }

[Severity: Critical]
Does this synchronous free cause a use-after-free for concurrent lockless
RCU readers?

In the i915 driver, GEM objects are destroyed using call_rcu() (via
__i915_gem_free_object_rcu). Lockless readers like i915_gem_busy_ioctl()
look up objects under rcu_read_lock() and access obj->base.resv.

If an object shares its resv instance (e.g., via obj->shares_resv_from with
an i915_address_space), __i915_gem_free_object() drops the lock reference
synchronously via i915_vm_resv_put(). If that drops the last reference,
dma_resv_release() synchronously frees the memory here.

However, the GEM object itself remains valid during the RCU grace period.
Can concurrent RCU readers dereference the already freed dma_resv pointer
when calling dma_resv_iter_begin(&cursor, obj->base.resv, ...) in
i915_gem_busy_ioctl()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827124910.2245-1-christian.koenig@amd.com?part=10