| Message ID | 694691bf-f591-4286-a615-df91d2ebab93@moroto.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dma-buf: fix an error pointer vs NULL bug | expand |
Am 06.07.23 um 07:52 schrieb Dan Carpenter: > The __dma_fence_unwrap_merge() function is supposed to return NULL on > error. But the dma_fence_allocate_private_stub() returns error pointers > so check for that and covert the error pointers to NULL returns. > Otherwise, the callers do not expect error pointers and it leads to an > Oops. Oh, good catch. But I think we should probably change dma_fence_allocate_private_stub() instead, that this function returns an ERR_PTR doesn't seem to make to much sense. Christian. > > Fixes: f781f661e8c9 ("dma-buf: keep the signaling time of merged fences v3") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/dma-buf/dma-fence-unwrap.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c > index c625bb2b5d56..d183eda0db89 100644 > --- a/drivers/dma-buf/dma-fence-unwrap.c > +++ b/drivers/dma-buf/dma-fence-unwrap.c > @@ -94,8 +94,12 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, > * If we couldn't find a pending fence just return a private signaled > * fence with the timestamp of the last signaled one. > */ > - if (count == 0) > - return dma_fence_allocate_private_stub(timestamp); > + if (count == 0) { > + tmp = dma_fence_allocate_private_stub(timestamp); > + if (IS_ERR(tmp)) > + return NULL; > + return tmp; > + } > > array = kmalloc_array(count, sizeof(*array), GFP_KERNEL); > if (!array) > @@ -176,6 +180,8 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, > > return_tmp: > kfree(array); > + if (IS_ERR(tmp)) > + return NULL; > return tmp; > } > EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge);
On Thu, Jul 06, 2023 at 08:21:35AM +0200, Christian König wrote: > Am 06.07.23 um 07:52 schrieb Dan Carpenter: > > The __dma_fence_unwrap_merge() function is supposed to return NULL on > > error. But the dma_fence_allocate_private_stub() returns error pointers > > so check for that and covert the error pointers to NULL returns. > > Otherwise, the callers do not expect error pointers and it leads to an > > Oops. > > Oh, good catch. > > But I think we should probably change dma_fence_allocate_private_stub() > instead, that this function returns an ERR_PTR doesn't seem to make to much > sense. Sure, I've sent v2. regards, dan carpenter
diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c index c625bb2b5d56..d183eda0db89 100644 --- a/drivers/dma-buf/dma-fence-unwrap.c +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -94,8 +94,12 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, * If we couldn't find a pending fence just return a private signaled * fence with the timestamp of the last signaled one. */ - if (count == 0) - return dma_fence_allocate_private_stub(timestamp); + if (count == 0) { + tmp = dma_fence_allocate_private_stub(timestamp); + if (IS_ERR(tmp)) + return NULL; + return tmp; + } array = kmalloc_array(count, sizeof(*array), GFP_KERNEL); if (!array) @@ -176,6 +180,8 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, return_tmp: kfree(array); + if (IS_ERR(tmp)) + return NULL; return tmp; } EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge);
The __dma_fence_unwrap_merge() function is supposed to return NULL on error. But the dma_fence_allocate_private_stub() returns error pointers so check for that and covert the error pointers to NULL returns. Otherwise, the callers do not expect error pointers and it leads to an Oops. Fixes: f781f661e8c9 ("dma-buf: keep the signaling time of merged fences v3") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/dma-buf/dma-fence-unwrap.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)