Re: [PATCH][next] dma-buf: Fix NULL pointer dereference in dma_fence_enable_sw_signaling()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxxxxxx
Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
---
  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






[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux