| Message ID | b09f1996-3838-4fa2-9193-832b68262e43@moroto.mountain |
|---|---|
| State | Accepted |
| Commit | 00ae1491f970acc454be0df63f50942d94825860 |
| Headers | show |
| Series | [v2] dma-buf: fix an error pointer vs NULL bug | expand |
Am 06.07.23 um 14:37 schrieb Dan Carpenter: > Smatch detected potential error pointer dereference. > > drivers/gpu/drm/drm_syncobj.c:888 drm_syncobj_transfer_to_timeline() > error: 'fence' dereferencing possible ERR_PTR() > > The error pointer comes from dma_fence_allocate_private_stub(). One > caller expected error pointers and one expected NULL pointers. Change > it to return NULL and update the caller which expected error pointers, > drm_syncobj_assign_null_handle(), to check for NULL instead. > > Fixes: f781f661e8c9 ("dma-buf: keep the signaling time of merged fences v3") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Reviewed-by: Christian König <christian.koenig@amd.com> Should I push that one to drm-misc-fixes? Regards, Christian. > --- > v2: Fix it in dma_fence_allocate_private_stub() instead of > __dma_fence_unwrap_merge(). > > > drivers/dma-buf/dma-fence.c | 2 +- > drivers/gpu/drm/drm_syncobj.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index ad076f208760..8aa8f8cb7071 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -160,7 +160,7 @@ struct dma_fence *dma_fence_allocate_private_stub(ktime_t timestamp) > > fence = kzalloc(sizeof(*fence), GFP_KERNEL); > if (fence == NULL) > - return ERR_PTR(-ENOMEM); > + return NULL; > > dma_fence_init(fence, > &dma_fence_stub_ops, > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index 04589a35eb09..e592c5da70ce 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -355,8 +355,8 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) > { > struct dma_fence *fence = dma_fence_allocate_private_stub(ktime_get()); > > - if (IS_ERR(fence)) > - return PTR_ERR(fence); > + if (!fence) > + return -ENOMEM; > > drm_syncobj_replace_fence(syncobj, fence); > dma_fence_put(fence);
On Thu, 6 Jul 2023 at 18:24, Christian König <christian.koenig@amd.com> wrote: > > Am 06.07.23 um 14:37 schrieb Dan Carpenter: > > Smatch detected potential error pointer dereference. > > > > drivers/gpu/drm/drm_syncobj.c:888 drm_syncobj_transfer_to_timeline() > > error: 'fence' dereferencing possible ERR_PTR() > > > > The error pointer comes from dma_fence_allocate_private_stub(). One > > caller expected error pointers and one expected NULL pointers. Change > > it to return NULL and update the caller which expected error pointers, > > drm_syncobj_assign_null_handle(), to check for NULL instead. > > > > Fixes: f781f661e8c9 ("dma-buf: keep the signaling time of merged fences v3") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > Thanks for catching this! > Reviewed-by: Christian König <christian.koenig@amd.com> Reviewed-by: Sumit Semwal <sumit.semwal@linaro.org> > > Should I push that one to drm-misc-fixes? If you haven't pushed already, I can push it now. > > Regards, > Christian. Best, Sumit. > > > --- > > v2: Fix it in dma_fence_allocate_private_stub() instead of > > __dma_fence_unwrap_merge(). > > > > > > drivers/dma-buf/dma-fence.c | 2 +- > > drivers/gpu/drm/drm_syncobj.c | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > index ad076f208760..8aa8f8cb7071 100644 > > --- a/drivers/dma-buf/dma-fence.c > > +++ b/drivers/dma-buf/dma-fence.c > > @@ -160,7 +160,7 @@ struct dma_fence *dma_fence_allocate_private_stub(ktime_t timestamp) > > > > fence = kzalloc(sizeof(*fence), GFP_KERNEL); > > if (fence == NULL) > > - return ERR_PTR(-ENOMEM); > > + return NULL; > > > > dma_fence_init(fence, > > &dma_fence_stub_ops, > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > > index 04589a35eb09..e592c5da70ce 100644 > > --- a/drivers/gpu/drm/drm_syncobj.c > > +++ b/drivers/gpu/drm/drm_syncobj.c > > @@ -355,8 +355,8 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) > > { > > struct dma_fence *fence = dma_fence_allocate_private_stub(ktime_get()); > > > > - if (IS_ERR(fence)) > > - return PTR_ERR(fence); > > + if (!fence) > > + return -ENOMEM; > > > > drm_syncobj_replace_fence(syncobj, fence); > > dma_fence_put(fence); >
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index ad076f208760..8aa8f8cb7071 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -160,7 +160,7 @@ struct dma_fence *dma_fence_allocate_private_stub(ktime_t timestamp) fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) - return ERR_PTR(-ENOMEM); + return NULL; dma_fence_init(fence, &dma_fence_stub_ops, diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 04589a35eb09..e592c5da70ce 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -355,8 +355,8 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) { struct dma_fence *fence = dma_fence_allocate_private_stub(ktime_get()); - if (IS_ERR(fence)) - return PTR_ERR(fence); + if (!fence) + return -ENOMEM; drm_syncobj_replace_fence(syncobj, fence); dma_fence_put(fence);
Smatch detected potential error pointer dereference. drivers/gpu/drm/drm_syncobj.c:888 drm_syncobj_transfer_to_timeline() error: 'fence' dereferencing possible ERR_PTR() The error pointer comes from dma_fence_allocate_private_stub(). One caller expected error pointers and one expected NULL pointers. Change it to return NULL and update the caller which expected error pointers, drm_syncobj_assign_null_handle(), to check for NULL instead. Fixes: f781f661e8c9 ("dma-buf: keep the signaling time of merged fences v3") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- v2: Fix it in dma_fence_allocate_private_stub() instead of __dma_fence_unwrap_merge(). drivers/dma-buf/dma-fence.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)