[bug report] drm/omap: gem: Replace struct_mutex usage with omap_obj private lock

Dan Carpenter dan.carpenter at linaro.org
Tue Jul 30 22:01:35 UTC 2024


Hello Laurent Pinchart,

Commit 3cbd0c587b12 ("drm/omap: gem: Replace struct_mutex usage with
omap_obj private lock") from May 26, 2018 (linux-next), leads to the
following Smatch static checker warning:

	drivers/gpu/drm/omapdrm/omap_gem.c:1435 omap_gem_new_dmabuf()
	warn: 'omap_obj' was already freed. (line 1434)

drivers/gpu/drm/omapdrm/omap_gem.c
    1386 struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
    1387                                            struct sg_table *sgt)
    1388 {
    1389         struct omap_drm_private *priv = dev->dev_private;
    1390         struct omap_gem_object *omap_obj;
    1391         struct drm_gem_object *obj;
    1392         union omap_gem_size gsize;
    1393 
    1394         /* Without a DMM only physically contiguous buffers can be supported. */
    1395         if (!omap_gem_sgt_is_contiguous(sgt, size) && !priv->has_dmm)
    1396                 return ERR_PTR(-EINVAL);
    1397 
    1398         gsize.bytes = PAGE_ALIGN(size);
    1399         obj = omap_gem_new(dev, gsize, OMAP_BO_MEM_DMABUF | OMAP_BO_WC);
    1400         if (!obj)
    1401                 return ERR_PTR(-ENOMEM);
    1402 
    1403         omap_obj = to_omap_bo(obj);
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
This is omap_obj

    1404 
    1405         mutex_lock(&omap_obj->lock);
    1406 
    1407         omap_obj->sgt = sgt;
    1408 
    1409         if (omap_gem_sgt_is_contiguous(sgt, size)) {
    1410                 omap_obj->dma_addr = sg_dma_address(sgt->sgl);
    1411         } else {
    1412                 /* Create pages list from sgt */
    1413                 struct page **pages;
    1414                 unsigned int npages;
    1415                 unsigned int ret;
    1416 
    1417                 npages = DIV_ROUND_UP(size, PAGE_SIZE);
    1418                 pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL);
    1419                 if (!pages) {
    1420                         omap_gem_free_object(obj);
                                                      ^^^
It gets free inside this function

    1421                         obj = ERR_PTR(-ENOMEM);
    1422                         goto done;
    1423                 }
    1424 
    1425                 omap_obj->pages = pages;
    1426                 ret = drm_prime_sg_to_page_array(sgt, pages, npages);
    1427                 if (ret) {
    1428                         omap_gem_free_object(obj);
                                                      ^^^
Same

    1429                         obj = ERR_PTR(-ENOMEM);
    1430                         goto done;

So I think we can just return directly instead of unlocking.

    1431                 }
    1432         }
    1433 
    1434 done:
--> 1435         mutex_unlock(&omap_obj->lock);
    1436         return obj;
    1437 }

regards,
dan carpenter


More information about the dri-devel mailing list