lkml.org 
[lkml]   [2023]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH][next] dma-buf: Fix NULL pointer dereference in dma_fence_enable_sw_signaling()
From


On 10/11/23 10:03, Kees Cook wrote:
> On Wed, Oct 11, 2023 at 08:03:43AM -0600, Gustavo A. R. Silva wrote:
>> Currently, a NULL pointer dereference will happen in function
>> `dma_fence_enable_sw_signaling()` (at line 615), in case `chain`
>> is not allocated in `mock_chain()` and this function returns
>> `NULL` (at line 86). See below:
>>
>> drivers/dma-buf/st-dma-fence-chain.c:
>> 86 chain = mock_chain(NULL, f, 1);
>> 87 if (!chain)
>> 88 err = -ENOMEM;
>> 89
>> 90 dma_fence_enable_sw_signaling(chain);
>
> Instead of the larger patch, should line 88 here just do a "return
> -ENOMEM" instead?

Nope. I would have to add a `goto` to skip `dma_fence_enable_sw_signaling(chain)`.

I originally thought of that, but as other _signaling() functions have
sanity-checks inside, I decided to go with that solution.

This bug has been there since Sep 2022. So, adding a sanity check inside that
function should prevent any other issue of this same kind to enter the codebase
and stay there for years.

--
Gustavo

>
> -Kees
>
>>
>> drivers/dma-buf/dma-fence.c:
>> 611 void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>> 612 {
>> 613 unsigned long flags;
>> 614
>> 615 spin_lock_irqsave(fence->lock, flags);
>> ^^^^^^^^^^^
>> |
>> NULL pointer reference
>> if fence == NULL
>>
>> 616 __dma_fence_enable_signaling(fence);
>> 617 spin_unlock_irqrestore(fence->lock, flags);
>> 618 }
>>
>> Fix this by adding a NULL check before dereferencing `fence` in
>> `dma_fence_enable_sw_signaling()`. This will prevent any other NULL
>> pointer dereference when the `fence` passed as an argument is `NULL`.
>>
>> Addresses-Coverity: ("Dereference after null check")
>> Fixes: d62c43a953ce ("dma-buf: Enable signaling on fence for selftests")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>> drivers/dma-buf/dma-fence.c | 9 ++++++++-
>> include/linux/dma-fence.h | 2 +-
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 8aa8f8cb7071..4d2f13560d0f 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -607,14 +607,21 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence)
>> * This will request for sw signaling to be enabled, to make the fence
>> * complete as soon as possible. This calls &dma_fence_ops.enable_signaling
>> * internally.
>> + *
>> + * Returns 0 on success and a negative error value when @fence is NULL.
>> */
>> -void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>> +int dma_fence_enable_sw_signaling(struct dma_fence *fence)
>> {
>> unsigned long flags;
>>
>> + if (!fence)
>> + return -EINVAL;
>> +
>> spin_lock_irqsave(fence->lock, flags);
>> __dma_fence_enable_signaling(fence);
>> spin_unlock_irqrestore(fence->lock, flags);
>> +
>> + return 0;
>> }
>> EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>>
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index ebe78bd3d121..1e4025e925e6 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -399,7 +399,7 @@ int dma_fence_add_callback(struct dma_fence *fence,
>> dma_fence_func_t func);
>> bool dma_fence_remove_callback(struct dma_fence *fence,
>> struct dma_fence_cb *cb);
>> -void dma_fence_enable_sw_signaling(struct dma_fence *fence);
>> +int dma_fence_enable_sw_signaling(struct dma_fence *fence);
>>
>> /**
>> * dma_fence_is_signaled_locked - Return an indication if the fence
>> --
>> 2.34.1
>>
>>
>

\
 
 \ /
  Last update: 2023-10-11 18:14    [W:0.086 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site