]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
drm/omap: Fix locking in omap_gem_new_dmabuf()
authorTomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tue, 6 Aug 2024 13:50:29 +0000 (16:50 +0300)
committerTomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Mon, 16 Sep 2024 04:12:40 +0000 (07:12 +0300)
omap_gem_new_dmabuf() creates the new gem object, and then takes and
holds the omap_obj->lock for the rest of the function. This has two
issues:

- omap_gem_free_object(), which is called in the error paths, also takes
  the same lock, leading to deadlock
- Even if the above wouldn't happen, in the error cases
  omap_gem_new_dmabuf() still unlocks omap_obj->lock, even after the
  omap_obj has already been freed.

Furthermore, I don't think there's any reason to take the lock at all,
as the object was just created and not yet shared with anyone else.

To fix all this, drop taking the lock.

Fixes: 3cbd0c587b12 ("drm/omap: gem: Replace struct_mutex usage with omap_obj private lock")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/511b99d7-aade-4f92-bd3e-63163a13d617@stanley.mountain/
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240806-omapdrm-misc-fixes-v1-3-15d31aea0831@ideasonboard.com
drivers/gpu/drm/omapdrm/omap_gem.c

index fdae677558f3ef7c1288f3b10b934050551f35ea..b9c67e4ca360545024d59dcaff0964e9451cb598 100644 (file)
@@ -1402,8 +1402,6 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
 
        omap_obj = to_omap_bo(obj);
 
-       mutex_lock(&omap_obj->lock);
-
        omap_obj->sgt = sgt;
 
        if (omap_gem_sgt_is_contiguous(sgt, size)) {
@@ -1418,21 +1416,17 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
                pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL);
                if (!pages) {
                        omap_gem_free_object(obj);
-                       obj = ERR_PTR(-ENOMEM);
-                       goto done;
+                       return ERR_PTR(-ENOMEM);
                }
 
                omap_obj->pages = pages;
                ret = drm_prime_sg_to_page_array(sgt, pages, npages);
                if (ret) {
                        omap_gem_free_object(obj);
-                       obj = ERR_PTR(-ENOMEM);
-                       goto done;
+                       return ERR_PTR(-ENOMEM);
                }
        }
 
-done:
-       mutex_unlock(&omap_obj->lock);
        return obj;
 }