dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: qcom-adm: fix wrong calling convention for prep_slave_sg
@ 2022-09-16  4:12 Christian Marangi
  2022-09-29 16:44 ` Vinod Koul
  0 siblings, 1 reply; 2+ messages in thread
From: Christian Marangi @ 2022-09-16  4:12 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Vinod Koul,
	Arnd Bergmann, Mark Brown, Thomas Pedersen, Jonathan McDowell,
	linux-arm-msm, dmaengine, linux-kernel
  Cc: Christian Marangi, stable

The calling convention for pre_slave_sg is to return NULL on error and
provide an error log to the system. Qcom-adm instead provide error
pointer when an error occur. This indirectly cause kernel panic for
example for the nandc driver that checks only if the pointer returned by
device_prep_slave_sg is not NULL. Returning an error pointer makes nandc
think the device_prep_slave_sg function correctly completed and makes
the kernel panics later in the code.

While nandc is the one that makes the kernel crash, it was pointed out
that the real problem is qcom-adm not following calling convention for
that function.

To fix this, drop returning error pointer and return NULL with an error
log.

Fixes: 03de6b273805 ("dmaengine: qcom-adm: stop abusing slave_id config")
Fixes: 5c9f8c2dbdbe ("dmaengine: qcom: Add ADM driver")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Cc: stable@vger.kernel.org # v5.11+
---
 drivers/dma/qcom/qcom_adm.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/qcom/qcom_adm.c b/drivers/dma/qcom/qcom_adm.c
index facdacf8aede..cd3f12cf4721 100644
--- a/drivers/dma/qcom/qcom_adm.c
+++ b/drivers/dma/qcom/qcom_adm.c
@@ -379,13 +379,13 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
 		if (blk_size < 0) {
 			dev_err(adev->dev, "invalid burst value: %d\n",
 				burst);
-			return ERR_PTR(-EINVAL);
+			return NULL;
 		}
 
 		crci = achan->crci & 0xf;
 		if (!crci || achan->crci > 0x1f) {
 			dev_err(adev->dev, "invalid crci value\n");
-			return ERR_PTR(-EINVAL);
+			return NULL;
 		}
 	}
 
@@ -403,8 +403,10 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
 	}
 
 	async_desc = kzalloc(sizeof(*async_desc), GFP_NOWAIT);
-	if (!async_desc)
-		return ERR_PTR(-ENOMEM);
+	if (!async_desc) {
+		dev_err(adev->dev, "not enough memory for async_desc struct\n");
+		return NULL;
+	}
 
 	async_desc->mux = achan->mux ? ADM_CRCI_CTL_MUX_SEL : 0;
 	async_desc->crci = crci;
@@ -414,8 +416,10 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
 				sizeof(*cple) + 2 * ADM_DESC_ALIGN;
 
 	async_desc->cpl = kzalloc(async_desc->dma_len, GFP_NOWAIT);
-	if (!async_desc->cpl)
+	if (!async_desc->cpl) {
+		dev_err(adev->dev, "not enough memory for cpl struct\n");
 		goto free;
+	}
 
 	async_desc->adev = adev;
 
@@ -437,8 +441,10 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
 	async_desc->dma_addr = dma_map_single(adev->dev, async_desc->cpl,
 					      async_desc->dma_len,
 					      DMA_TO_DEVICE);
-	if (dma_mapping_error(adev->dev, async_desc->dma_addr))
+	if (dma_mapping_error(adev->dev, async_desc->dma_addr)) {
+		dev_err(adev->dev, "dma mapping error for cpl\n");
 		goto free;
+	}
 
 	cple_addr = async_desc->dma_addr + ((void *)cple - async_desc->cpl);
 
@@ -454,7 +460,7 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
 
 free:
 	kfree(async_desc);
-	return ERR_PTR(-ENOMEM);
+	return NULL;
 }
 
 /**
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] dmaengine: qcom-adm: fix wrong calling convention for prep_slave_sg
  2022-09-16  4:12 [PATCH] dmaengine: qcom-adm: fix wrong calling convention for prep_slave_sg Christian Marangi
@ 2022-09-29 16:44 ` Vinod Koul
  0 siblings, 0 replies; 2+ messages in thread
From: Vinod Koul @ 2022-09-29 16:44 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Arnd Bergmann,
	Mark Brown, Thomas Pedersen, Jonathan McDowell, linux-arm-msm,
	dmaengine, linux-kernel, stable

On 16-09-22, 06:12, Christian Marangi wrote:
> The calling convention for pre_slave_sg is to return NULL on error and
> provide an error log to the system. Qcom-adm instead provide error
> pointer when an error occur. This indirectly cause kernel panic for
> example for the nandc driver that checks only if the pointer returned by
> device_prep_slave_sg is not NULL. Returning an error pointer makes nandc
> think the device_prep_slave_sg function correctly completed and makes
> the kernel panics later in the code.
> 
> While nandc is the one that makes the kernel crash, it was pointed out
> that the real problem is qcom-adm not following calling convention for
> that function.
> 
> To fix this, drop returning error pointer and return NULL with an error
> log.

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-09-29 16:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-16  4:12 [PATCH] dmaengine: qcom-adm: fix wrong calling convention for prep_slave_sg Christian Marangi
2022-09-29 16:44 ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).