[bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg

[YoungJun Cho]

Dear Dan,

Your mail flashes back to my memory 5 years ago.
Back then, I had cleaned up the exynos driver.

And the replacement IS_ERR was one of items.

IMHO it is still better to modify those two functions,
drm_gem_cma_prime_get_sg_table and xen_drm_front_gem_get_sg_table.

Thank you.
Best regards YJ


On Thu, 14 Jun 2018, 23:42 Dan Carpenter, <dan.carpenter at oracle.com> wrote:

> Hello YoungJun Cho,
>
> The patch 7e3d88f9cce3: "drm/prime: replace NULL with error value in
> drm_prime_pages_to_sg" from Jun 24, 2013, leads to the following
> static checker warning:
>
>         drivers/gpu/drm/drm_prime.c:317 drm_gem_map_dma_buf()
>         warn: 'sgt' can also be NULL
>
> drivers/gpu/drm/drm_prime.c
>    307          /*
>    308           * two mappings with different directions for the same
> attachment are
>    309           * not allowed
>    310           */
>    311          if (WARN_ON(prime_attach->dir != DMA_NONE))
>    312                  return ERR_PTR(-EBUSY);
>    313
>    314          sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
>
> The problematic functions here are drm_gem_cma_prime_get_sg_table() and
> xen_drm_front_gem_get_sg_table().  My preference would be to update
> those functions to return error pointers, but I'm not familiar with the
> code or able to test it.
>
> But this static checker test seems pretty good so I'm likely to publish
> it soon and then this sort of bug will normally be caught quickly.
>
>    315
>    316          if (!IS_ERR(sgt)) {
>    317                  if (!dma_map_sg_attrs(attach->dev, sgt->sgl,
> sgt->nents, dir,
>    318                                        DMA_ATTR_SKIP_CPU_SYNC)) {
>    319                          sg_free_table(sgt);
>    320                          kfree(sgt);
>    321                          sgt = ERR_PTR(-ENOMEM);
>    322                  } else {
>    323                          prime_attach->sgt = sgt;
>    324                          prime_attach->dir = dir;
>    325                  }
>    326          }
>    327
>    328          return sgt;
>
> regards,
> dan carpenter
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180615/52b3d34b/attachment.html>