Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Pre-existing error path vulnerability in `msm_gem_import` causes a double-free of the exporter's `sgt`, NULL pointer dereference, and wild pointer dereferences during cleanup. -- --- Patch [9]: [PATCH v6 09/16] drm/msm/gem: Remove useless locking in GEM import --- commit 19246e79feb40c4e0066b56b3385fea83fcc3d75 Author: Rob Clark <[email protected]> drm/msm/gem: Remove useless locking in GEM import This patch removes unnecessary locking during the GEM object import process in msm_gem_import(). > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index 2cb3ab04f1250..a358fb5ca6d73 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -1302,23 +1302,18 @@ struct drm_gem_object *msm_gem_import(struct > drm_device *dev, > npages = size / PAGE_SIZE; > > msm_obj = to_msm_bo(obj); > - msm_gem_lock(obj); > msm_obj->sgt = sgt; > msm_obj->pages = kvmalloc_objs(struct page *, npages); > if (!msm_obj->pages) { > - msm_gem_unlock(obj); > ret = -ENOMEM; > goto fail; > } > > ret = drm_prime_sg_to_page_array(sgt, msm_obj->pages, npages); > if (ret) { > - msm_gem_unlock(obj); > goto fail; [Severity: High] This is a pre-existing issue, but does this error path lead to a NULL pointer dereference and a double-free? When msm_gem_import() jumps to fail, it calls drm_gem_object_put(obj). At this point, obj->import_attach has not yet been set (it is assigned later by the caller drm_gem_prime_import_dev() on success). During cleanup, msm_gem_free_object() checks drm_gem_is_imported(obj). Since import_attach is NULL, this evaluates to false, causing the cleanup to fall through to the native path: msm_gem_free_object() -> put_pages(obj) In put_pages(), the code incorrectly frees the exporter-owned sgt (leading to a double-free) and then calls drm_gem_put_pages(). Finally, drm_gem_put_pages() unconditionally accesses file_inode(obj->filp)->i_mapping. Because obj->filp is NULL for imported objects, this causes a NULL pointer dereference. > } > > - msm_gem_unlock(obj); > - > drm_gem_lru_move_tail(&priv->lru.pinned, obj); > > mutex_lock(&priv->obj_lock); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
