Re: [PATCH v7 10/17] drm/msm/gem: Remove useless locking in GEM import

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent access to imported GEM objects via debugfs causes a lock imbalance, leading to system crash or permanent resource lock.
--

--- Patch [10]: [PATCH v7 10/17] drm/msm/gem: Remove useless locking in GEM import ---
commit 1dfbce02f7920b37d011abac3bc5352c1cadab22
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm/gem: Remove useless locking in GEM import
    
The locking has changed a few times over the years, and this extra
locking was the mistake of evolution.  Harmless but useless.

> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 74c3728f45bd0..cc45392691e50 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
[ ... ]
> @@ -1310,22 +1310,17 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
[ ... ]
>  	drm_gem_lru_move_tail(&priv->lru.pinned, obj);
>  
>  	mutex_lock(&priv->obj_lock);

[Severity: High]
This is a pre-existing issue, but can publishing the object to the global
list here lead to a lock imbalance if debugfs is accessed concurrently?

Once the object is added to the priv->objects list, it becomes visible to
concurrent debugfs iteration before the import process has finished.

According to drm_prime.c, drm_gem_prime_import_dev() overwrites obj->resv
after msm_gem_import() has already published it:

	obj->import_attach = attach;
	obj->resv = dma_buf->resv;

If msm_gem_describe() in debugfs iterates the list concurrently during this
window, it locks the original obj->resv via msm_gem_trylock(obj). If the
resv pointer is then overwritten mid-flight, the subsequent
msm_gem_unlock(obj) call will unlock a completely different lock:

	if (!msm_gem_trylock(obj))
		return;
	...
	dma_resv_describe(robj, m);
	msm_gem_unlock(obj);

Could this lead to a kernel oops due to a bad unlock balance, or leave the
initial reservation object permanently locked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729155609.20190-1-robin.clark@oss.qualcomm.com?part=10