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 <[email protected]> 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/[email protected]?part=10
