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