* [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
@ 2018-06-14 14:31 Dan Carpenter
2018-06-15 3:22 ` YoungJun Cho
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2018-06-14 14:31 UTC (permalink / raw)
To: yj44.cho; +Cc: dri-devel
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
2018-06-14 14:31 [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg Dan Carpenter
@ 2018-06-15 3:22 ` YoungJun Cho
2018-06-15 5:39 ` Oleksandr Andrushchenko
0 siblings, 1 reply; 5+ messages in thread
From: YoungJun Cho @ 2018-06-15 3:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2269 bytes --]
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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[-- Attachment #1.2: Type: text/html, Size: 3586 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
2018-06-15 3:22 ` YoungJun Cho
@ 2018-06-15 5:39 ` Oleksandr Andrushchenko
2018-06-15 9:08 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Oleksandr Andrushchenko @ 2018-06-15 5:39 UTC (permalink / raw)
To: YoungJun Cho, Dan Carpenter; +Cc: dri-devel
On 06/15/2018 06:22 AM, YoungJun Cho wrote:
> 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@oracle.com
> <mailto:dan.carpenter@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.
>
I believe it is safe to change the return value to error pointer:
the code below (which is the only caller of .gem_prime_get_sg_table
callback) does not check for NULL anyways
Dan, do you want me to send the patch for Xen or you prefer to post both
Xen and CMA helpers fix yourself?
>
> 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@lists.freedesktop.org
> <mailto:dri-devel@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
2018-06-15 5:39 ` Oleksandr Andrushchenko
@ 2018-06-15 9:08 ` Dan Carpenter
2018-06-15 9:17 ` Oleksandr Andrushchenko
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2018-06-15 9:08 UTC (permalink / raw)
To: Oleksandr Andrushchenko; +Cc: dri-devel
On Fri, Jun 15, 2018 at 08:39:17AM +0300, Oleksandr Andrushchenko wrote:
> On 06/15/2018 06:22 AM, YoungJun Cho wrote:
> > 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@oracle.com
> > <mailto:dan.carpenter@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.
> >
> I believe it is safe to change the return value to error pointer:
> the code below (which is the only caller of .gem_prime_get_sg_table
> callback) does not check for NULL anyways
>
> Dan, do you want me to send the patch for Xen or you prefer to post both
> Xen and CMA helpers fix yourself?
Can you do it and give me a Reported-by tag?
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
2018-06-15 9:08 ` Dan Carpenter
@ 2018-06-15 9:17 ` Oleksandr Andrushchenko
0 siblings, 0 replies; 5+ messages in thread
From: Oleksandr Andrushchenko @ 2018-06-15 9:17 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
On 06/15/2018 12:08 PM, Dan Carpenter wrote:
> On Fri, Jun 15, 2018 at 08:39:17AM +0300, Oleksandr Andrushchenko wrote:
>> On 06/15/2018 06:22 AM, YoungJun Cho wrote:
>>> 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@oracle.com
>>> <mailto:dan.carpenter@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.
>>>
>> I believe it is safe to change the return value to error pointer:
>> the code below (which is the only caller of .gem_prime_get_sg_table
>> callback) does not check for NULL anyways
>>
>> Dan, do you want me to send the patch for Xen or you prefer to post both
>> Xen and CMA helpers fix yourself?
> Can you do it and give me a Reported-by tag?
Sure
> regards,
> dan carpenter
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-15 9:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-14 14:31 [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg Dan Carpenter
2018-06-15 3:22 ` YoungJun Cho
2018-06-15 5:39 ` Oleksandr Andrushchenko
2018-06-15 9:08 ` Dan Carpenter
2018-06-15 9:17 ` Oleksandr Andrushchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.