DMA Engine development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] dmaengine: add helper macro dmaengine_prep_submit()
@ 2026-05-22 20:13 Frank.Li
  2026-05-22 20:13 ` [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single() Frank.Li
  2026-05-22 20:13 ` [PATCH v2 2/2] i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code Frank.Li
  0 siblings, 2 replies; 6+ messages in thread
From: Frank.Li @ 2026-05-22 20:13 UTC (permalink / raw)
  To: Vinod Koul, Dong Aisheng, Andi Shyti, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: dmaengine, linux-kernel, linux-i2c, imx, linux-arm-kernel,
	carlos.song, Frank Li

Add helper macro dmaengine_prep_submit() to combine prep and submit to
one call.

Pervious try to use cleanup
https://lore.kernel.org/dmaengine/20251002-dma_chan_free-v1-0-4dbf116c2b19@nxp.com/

It is not simple enough and easy missing retain_and_null_ptr() at success
path.

struct dma_async_tx_descriptor *rx_cmd_desc __free(dma_async_tx_descriptor) = NULL;
        ...
        cookie = dmaengine_submit(rx_cmd_desc);
        if (dma_submit_error(cookie))
                return dma_submit_error(cookie);
        ...
        retain_and_null_ptr(rx_cmd_desc);
}

So create help macro to combine prep and submit by one call.
patch 2.

 static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
 {
-       struct dma_async_tx_descriptor *rx_cmd_desc;
        struct lpi2c_imx_dma *dma = lpi2c_imx->dma;
        struct dma_chan *txchan = dma->chan_tx;
        dma_cookie_t cookie;
@@ -761,15 +760,10 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
                return -EINVAL;
        }

-       rx_cmd_desc = dmaengine_prep_slave_single(txchan, dma->dma_tx_addr,
-                                                 dma->rx_cmd_buf_len, DMA_MEM_TO_DEV,
-                                                 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-       if (!rx_cmd_desc) {
-               dev_err(&lpi2c_imx->adapter.dev, "DMA prep slave sg failed, use pio\n");
-               goto desc_prepare_err_exit;
-       }
-
-       cookie = dmaengine_submit(rx_cmd_desc);
+       cookie = dmaengine_prep_submit(txchan, NULL, NULL, slave_single,
+                                      dma->dma_tx_addr,
+                                      dma->rx_cmd_buf_len, DMA_MEM_TO_DEV,
+                                      DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
        if (dma_submit_error(cookie)) {
                dev_err(&lpi2c_imx->adapter.dev, "submitting DMA failed, use pio\n");
                goto submit_err_exit;
@@ -779,15 +773,9 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)

        return 0;

-desc_prepare_err_exit:
-       dma_unmap_single(txchan->device->dev, dma->dma_tx_addr,
-                        dma->rx_cmd_buf_len, DMA_TO_DEVICE);
-       return -EINVAL;
-
 submit_err_exit:
        dma_unmap_single(txchan->device->dev, dma->dma_tx_addr,
                         dma->rx_cmd_buf_len, DMA_TO_DEVICE);
-       dmaengine_desc_free(rx_cmd_desc);
        return -EINVAL;
 }

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v2:
- use API dmaengine_prep_submit_slave_single()
- Link to v1: https://lore.kernel.org/r/20260130-dma_prep_submit-v1-0-2198f9e848fa@nxp.com

---
Frank Li (2):
      dmaengine: Add helper dmaengine_prep_submit_slave_single()
      i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code

 drivers/dma/dmaengine.c            | 28 ++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-imx-lpi2c.c | 21 +++++----------------
 include/linux/dmaengine.h          | 17 +++++++++++++++++
 3 files changed, 50 insertions(+), 16 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20260130-dma_prep_submit-cbeac742de48

Best regards,
--  
Frank Li <Frank.Li@nxp.com>


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

* [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single()
  2026-05-22 20:13 [PATCH v2 0/2] dmaengine: add helper macro dmaengine_prep_submit() Frank.Li
@ 2026-05-22 20:13 ` Frank.Li
  2026-05-22 20:45   ` sashiko-bot
  2026-05-22 20:13 ` [PATCH v2 2/2] i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code Frank.Li
  1 sibling, 1 reply; 6+ messages in thread
From: Frank.Li @ 2026-05-22 20:13 UTC (permalink / raw)
  To: Vinod Koul, Dong Aisheng, Andi Shyti, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: dmaengine, linux-kernel, linux-i2c, imx, linux-arm-kernel,
	carlos.song, Frank Li

From: Frank Li <Frank.Li@nxp.com>

Previously, DMA users had to call dmaengine_prep_slave_single() followed by
dmaengine_submit(). Many DMA consumers missed call dmaengine_desc_free()
when dmaengine_submit() returned an error.

Introduce dmaengine_prep_submit_slave_single() to combine preparation and
submission into a single step and ensure the descriptor is freed on
submission failure.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v2
- use api function dmaengine_prep_submit_slave_single()
---
 drivers/dma/dmaengine.c   | 28 ++++++++++++++++++++++++++++
 include/linux/dmaengine.h | 17 +++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index ca13cd39330ba..1e25be78a22a5 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1619,6 +1619,34 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx)
 }
 EXPORT_SYMBOL_GPL(dma_run_dependencies);
 
+#define dmaengine_prep_submit(chan, cb, cb_param, func, ...)	\
+({	struct dma_async_tx_descriptor *tx =			\
+		dmaengine_prep_##func(chan, __VA_ARGS__);	\
+		dma_cookie_t cookie = -ENOMEM;			\
+								\
+	if (tx) {						\
+		tx->callback = cb;				\
+		tx->callback_param = cb_param;			\
+		cookie = dmaengine_submit(tx);			\
+								\
+		if (dma_submit_error(cookie))			\
+			dmaengine_desc_free(tx);		\
+	}							\
+	cookie;							\
+})
+
+dma_cookie_t
+dmaengine_prep_submit_slave_single(struct dma_chan *chan,
+				   dma_async_tx_callback cb, void *cb_param,
+				   dma_addr_t buf, size_t len,
+				   enum dma_transfer_direction dir,
+				   unsigned long flags)
+{
+	return dmaengine_prep_submit(chan, cb, cb_param, slave_single,
+				     buf, len, dir, flags);
+}
+EXPORT_SYMBOL_GPL(dmaengine_prep_submit_slave_single);
+
 static int __init dma_bus_init(void)
 {
 	int err = dmaengine_init_unmap_pool();
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 99efe2b9b4ea9..0f789fac7e91a 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -990,6 +990,13 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
 						  dir, flags, NULL);
 }
 
+dma_cookie_t
+dmaengine_prep_submit_slave_single(struct dma_chan *chan,
+				   dma_async_tx_callback cb, void *cb_param,
+				   dma_addr_t buf, size_t len,
+				   enum dma_transfer_direction dir,
+				   unsigned long flags);
+
 /**
  * dmaengine_prep_peripheral_dma_vec() - Prepare a DMA scatter-gather descriptor
  * @chan: The channel to be used for this descriptor
@@ -1575,6 +1582,16 @@ static inline int dma_get_slave_caps(struct dma_chan *chan,
 {
 	return -ENXIO;
 }
+
+static inline dma_cookie_t
+dmaengine_prep_submit_slave_single(struct dma_chan *chan,
+				   dma_async_tx_callback cb, void *cb_param;
+				   dma_addr_t buf, size_t len,
+				   enum dma_transfer_direction dir,
+				   unsigned long flags);
+{
+	return -ENODEV;
+}
 #endif
 
 static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)

-- 
2.43.0


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

* [PATCH v2 2/2] i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code
  2026-05-22 20:13 [PATCH v2 0/2] dmaengine: add helper macro dmaengine_prep_submit() Frank.Li
  2026-05-22 20:13 ` [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single() Frank.Li
@ 2026-05-22 20:13 ` Frank.Li
  2026-05-22 21:15   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Frank.Li @ 2026-05-22 20:13 UTC (permalink / raw)
  To: Vinod Koul, Dong Aisheng, Andi Shyti, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: dmaengine, linux-kernel, linux-i2c, imx, linux-arm-kernel,
	carlos.song, Frank Li

From: Frank Li <Frank.Li@nxp.com>

Use dmaengine_prep_submit() to simple code. No functional change.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 2a0962a0b4417..c90f72eec8498 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -720,7 +720,6 @@ static void lpi2c_dma_callback(void *data)
 
 static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
 {
-	struct dma_async_tx_descriptor *rx_cmd_desc;
 	struct lpi2c_imx_dma *dma = lpi2c_imx->dma;
 	struct dma_chan *txchan = dma->chan_tx;
 	dma_cookie_t cookie;
@@ -733,15 +732,11 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
 		return -EINVAL;
 	}
 
-	rx_cmd_desc = dmaengine_prep_slave_single(txchan, dma->dma_tx_addr,
-						  dma->rx_cmd_buf_len, DMA_MEM_TO_DEV,
-						  DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-	if (!rx_cmd_desc) {
-		dev_err(&lpi2c_imx->adapter.dev, "DMA prep slave sg failed, use pio\n");
-		goto desc_prepare_err_exit;
-	}
-
-	cookie = dmaengine_submit(rx_cmd_desc);
+	cookie = dmaengine_prep_submit_slave_single(txchan, NULL, NULL,
+						    dma->dma_tx_addr,
+						    dma->rx_cmd_buf_len,
+						    DMA_MEM_TO_DEV,
+						    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (dma_submit_error(cookie)) {
 		dev_err(&lpi2c_imx->adapter.dev, "submitting DMA failed, use pio\n");
 		goto submit_err_exit;
@@ -751,15 +746,9 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
 
 	return 0;
 
-desc_prepare_err_exit:
-	dma_unmap_single(txchan->device->dev, dma->dma_tx_addr,
-			 dma->rx_cmd_buf_len, DMA_TO_DEVICE);
-	return -EINVAL;
-
 submit_err_exit:
 	dma_unmap_single(txchan->device->dev, dma->dma_tx_addr,
 			 dma->rx_cmd_buf_len, DMA_TO_DEVICE);
-	dmaengine_desc_free(rx_cmd_desc);
 	return -EINVAL;
 }
 

-- 
2.43.0


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

* Re: [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single()
  2026-05-22 20:13 ` [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single() Frank.Li
@ 2026-05-22 20:45   ` sashiko-bot
  2026-05-22 21:36     ` Frank Li
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-05-22 20:45 UTC (permalink / raw)
  To: Frank.Li; +Cc: vkoul, Frank.Li, dmaengine, imx

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] The macro accesses the descriptor after calling `dmaengine_submit()`, violating the DMA Engine API contract and causing a Use-After-Free.
- [Medium] Calling `dmaengine_desc_free()` fails to prevent memory leaks for standard, non-reusable DMA descriptors, defeating the primary goal of the patch.
- [Low] The fallback implementation of `dmaengine_prep_submit_slave_single` contains severe syntax errors that break compilation when `CONFIG_DMA_ENGINE` is disabled.
--

commit c731d16a7ec5a6fed71b17234ae1bc2184ad8818
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: Add helper dmaengine_prep_submit_slave_single()

This commit introduces a helper macro and function to combine the preparation
and submission of a DMA slave single transfer into a single step. It also
aims to automatically free the descriptor if the submission fails.

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index ca13cd39330ba..1e25be78a22a5 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
[ ... ]
> @@ -1619,6 +1619,34 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx)
>  }
>  EXPORT_SYMBOL_GPL(dma_run_dependencies);
>  
> +#define dmaengine_prep_submit(chan, cb, cb_param, func, ...)	\
> +({	struct dma_async_tx_descriptor *tx =			\
> +		dmaengine_prep_##func(chan, __VA_ARGS__);	\
> +		dma_cookie_t cookie = -ENOMEM;			\
> +								\
> +	if (tx) {						\
> +		tx->callback = cb;				\
> +		tx->callback_param = cb_param;			\
> +		cookie = dmaengine_submit(tx);			\
> +								\
> +		if (dma_submit_error(cookie))			\
> +			dmaengine_desc_free(tx);		\

[Severity: Critical]
Does this access the descriptor after ownership has been transferred?

The DMA Engine API contract specifies that once dmaengine_submit is called,
the client must consider the descriptor pointer invalid. If submission fails,
some DMA providers (like the idxd driver in idxd_dma_tx_submit) internally
free the descriptor before returning an error.

Calling dmaengine_desc_free here would read tx->flags on an already-freed
pointer on the error path, resulting in a use-after-free.

[Severity: Medium]
Will this actually free standard, non-reusable descriptors?

The dmaengine_desc_free function is explicitly designed for reusable
descriptors. It internally calls dmaengine_desc_test_reuse, which checks
the DMA_CTRL_REUSE flag and returns -EPERM immediately if it is not set. 

Since standard descriptors created via dmaengine_prep_slave_single don't 
have this flag set by default, it seems this call acts as a no-op and might
not resolve the resource leak mentioned in the commit message.

> +	}							\
> +	cookie;							\
> +})
> +
[ ... ]
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 99efe2b9b4ea9..0f789fac7e91a 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
[ ... ]
> @@ -1575,6 +1582,16 @@ static inline int dma_get_slave_caps(struct dma_chan *chan,
>  {
>  	return -ENXIO;
>  }
> +
> +static inline dma_cookie_t
> +dmaengine_prep_submit_slave_single(struct dma_chan *chan,
> +				   dma_async_tx_callback cb, void *cb_param;
> +				   dma_addr_t buf, size_t len,
> +				   enum dma_transfer_direction dir,
> +				   unsigned long flags);
> +{

[Severity: Low]
Are there syntax errors in this fallback implementation?

The parameter list uses a semicolon instead of a comma after cb_param, and
there is a stray semicolon immediately preceding the function body. This will
cause a build failure when CONFIG_DMA_ENGINE is disabled.

> +	return -ENODEV;
> +}
>  #endif
>  
>  static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-dma_prep_submit-v2-0-7a87a5a29525@nxp.com?part=1

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

* Re: [PATCH v2 2/2] i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code
  2026-05-22 20:13 ` [PATCH v2 2/2] i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code Frank.Li
@ 2026-05-22 21:15   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-22 21:15 UTC (permalink / raw)
  To: Frank.Li; +Cc: vkoul, Frank.Li, imx, dmaengine

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Incomplete implementation of the refactoring pattern intended by the commit.
- [Low] The fallback declaration for `dmaengine_prep_submit_slave_single` contains semicolons instead of commas, breaking the build when CONFIG_DMA_ENGINE is disabled.

Pre-existing issues:
- [Critical] A critical double free vulnerability can occur on `dma->rx_cmd_buf` in the error handling path of `lpi2c_imx_dma_xfer()`.
- [High] DMA channels requested during probe are never released on device removal or on a subsequent probe failure.
--

commit c9c1ee53a58bc638a612c603e4f9002874839ea5
Author: Frank Li <Frank.Li@nxp.com>

i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code
    
Use dmaengine_prep_submit() to simple code. No functional change.

> Use dmaengine_prep_submit() to simple code. No functional change.

[Severity: Medium]
Is the refactoring incomplete here? The commit message states it simplifies the
code by using dmaengine_prep_submit(), but it only updates
lpi2c_dma_rx_cmd_submit().

An identical DMA preparation sequence exists just below it in lpi2c_dma_submit(),
which continues to manually call dmaengine_prep_slave_single() and
dmaengine_submit(). Should lpi2c_dma_submit() be updated as well for
consistency?

> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 2a0962a0b4417..c90f72eec8498 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -733,15 +732,11 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
>  		return -EINVAL;
>  	}
>  
> -	rx_cmd_desc = dmaengine_prep_slave_single(txchan, dma->dma_tx_addr,
> -						  dma->rx_cmd_buf_len, DMA_MEM_TO_DEV,
> -						  DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> -	if (!rx_cmd_desc) {
> -		dev_err(&lpi2c_imx->adapter.dev, "DMA prep slave sg failed, use pio\n");
> -		goto desc_prepare_err_exit;
> -	}
> -
> -	cookie = dmaengine_submit(rx_cmd_desc);
> +	cookie = dmaengine_prep_submit_slave_single(txchan, NULL, NULL,
> +						    dma->dma_tx_addr,
> +						    dma->rx_cmd_buf_len,
> +						    DMA_MEM_TO_DEV,
> +						    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);

[Severity: Low]
Will this break the build when CONFIG_DMA_ENGINE is disabled?

In include/linux/dmaengine.h, the !CONFIG_DMA_ENGINE stub for
dmaengine_prep_submit_slave_single() contains semicolons instead of commas:

static inline dma_cookie_t
dmaengine_prep_submit_slave_single(struct dma_chan *chan,
				   dma_async_tx_callback cb, void *cb_param;
				   dma_addr_t buf, size_t len,
				   enum dma_transfer_direction dir,
				   unsigned long flags);
{
	return -ENODEV;
}

This syntax error will cause a compilation failure for any driver calling it
when CONFIG_DMA_ENGINE is not set.

>  	if (dma_submit_error(cookie)) {
>  		dev_err(&lpi2c_imx->adapter.dev, "submitting DMA failed, use pio\n");
>  		goto submit_err_exit;

[ ... ]

[Severity: Critical]
This isn't a bug introduced by this patch, but could this result in a double
free on dma->rx_cmd_buf in the error handling path of lpi2c_imx_dma_xfer()?

When a read transfer (I2C_M_RD) finishes, dma->rx_cmd_buf is freed at the
disable_dma label:

	if (dma->dma_msg_flag & I2C_M_RD)
		kfree(dma->rx_cmd_buf);

However, the pointer is not set to NULL. If a subsequent read transfer fails
early (e.g., if lpi2c_dma_config() or lpi2c_dma_submit() fails), the code jumps
to disable_dma before lpi2c_imx_alloc_rx_cmd_buf() can allocate a new buffer.

Since dma->dma_msg_flag & I2C_M_RD is true, wouldn't kfree() be called a second
time on the dangling pointer from the previous transfer?

[Severity: High]
This is a pre-existing issue, but are the DMA channels requested during probe
leaking on device removal or probe failure?

In lpi2c_dma_init(), the "tx" and "rx" DMA channels are acquired using the
non-managed dma_request_chan() API. However, the driver's remove function
lpi2c_imx_remove() does not call dma_release_channel() to free them.

Furthermore, if i2c_add_adapter() fails during lpi2c_imx_probe(), the error path
(rpm_disable) fails to release the DMA channels that were successfully acquired
moments earlier. Should these paths be updated to release the channels?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-dma_prep_submit-v2-0-7a87a5a29525@nxp.com?part=2

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

* Re: [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single()
  2026-05-22 20:45   ` sashiko-bot
@ 2026-05-22 21:36     ` Frank Li
  0 siblings, 0 replies; 6+ messages in thread
From: Frank Li @ 2026-05-22 21:36 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Frank.Li, vkoul, Frank.Li, dmaengine, imx

On Fri, May 22, 2026 at 08:45:38PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] The macro accesses the descriptor after calling `dmaengine_submit()`, violating the DMA Engine API contract and causing a Use-After-Free.
> - [Medium] Calling `dmaengine_desc_free()` fails to prevent memory leaks for standard, non-reusable DMA descriptors, defeating the primary goal of the patch.
> - [Low] The fallback implementation of `dmaengine_prep_submit_slave_single` contains severe syntax errors that break compilation when `CONFIG_DMA_ENGINE` is disabled.
> --
>
> commit c731d16a7ec5a6fed71b17234ae1bc2184ad8818
> Author: Frank Li <Frank.Li@nxp.com>
>
> dmaengine: Add helper dmaengine_prep_submit_slave_single()
>
> This commit introduces a helper macro and function to combine the preparation
> and submission of a DMA slave single transfer into a single step. It also
> aims to automatically free the descriptor if the submission fails.
>
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index ca13cd39330ba..1e25be78a22a5 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> [ ... ]
> > @@ -1619,6 +1619,34 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx)
> >  }
> >  EXPORT_SYMBOL_GPL(dma_run_dependencies);
> >
> > +#define dmaengine_prep_submit(chan, cb, cb_param, func, ...)	\
> > +({	struct dma_async_tx_descriptor *tx =			\
> > +		dmaengine_prep_##func(chan, __VA_ARGS__);	\
> > +		dma_cookie_t cookie = -ENOMEM;			\
> > +								\
> > +	if (tx) {						\
> > +		tx->callback = cb;				\
> > +		tx->callback_param = cb_param;			\
> > +		cookie = dmaengine_submit(tx);			\
> > +								\
> > +		if (dma_submit_error(cookie))			\
> > +			dmaengine_desc_free(tx);		\
>
> [Severity: Critical]
> Does this access the descriptor after ownership has been transferred?
>
> The DMA Engine API contract specifies that once dmaengine_submit is called,
> the client must consider the descriptor pointer invalid. If submission fails,
> some DMA providers (like the idxd driver in idxd_dma_tx_submit) internally
> free the descriptor before returning an error.

Not sure if it is defined beheavor or just idxd implement as it. If it is
defined beheavor, suggest update Document.

From API document:

 "Once a descriptor has been obtained, the callback information can be
  added and the descriptor must then be submitted. Some DMA engine
  drivers may hold a spinlock between a successful preparation and
  submission so it is important that these two operations are closely
  paired."

prep_submit() API should be provide to align document. but not sure
if dmaengine_desc_free() call for reusable descriptors, Or providor should
consider this like idxd_dma_tx_submit()?

Frank
>
> Calling dmaengine_desc_free here would read tx->flags on an already-freed
> pointer on the error path, resulting in a use-after-free.
>
> [Severity: Medium]
> Will this actually free standard, non-reusable descriptors?
>
> The dmaengine_desc_free function is explicitly designed for reusable
> descriptors. It internally calls dmaengine_desc_test_reuse, which checks
> the DMA_CTRL_REUSE flag and returns -EPERM immediately if it is not set.
>
> Since standard descriptors created via dmaengine_prep_slave_single don't
> have this flag set by default, it seems this call acts as a no-op and might
> not resolve the resource leak mentioned in the commit message.
>
> > +	}							\
> > +	cookie;							\
> > +})
> > +
> [ ... ]
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 99efe2b9b4ea9..0f789fac7e91a 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> [ ... ]
> > @@ -1575,6 +1582,16 @@ static inline int dma_get_slave_caps(struct dma_chan *chan,
> >  {
> >  	return -ENXIO;
> >  }
> > +
> > +static inline dma_cookie_t
> > +dmaengine_prep_submit_slave_single(struct dma_chan *chan,
> > +				   dma_async_tx_callback cb, void *cb_param;
> > +				   dma_addr_t buf, size_t len,
> > +				   enum dma_transfer_direction dir,
> > +				   unsigned long flags);
> > +{
>
> [Severity: Low]
> Are there syntax errors in this fallback implementation?
>
> The parameter list uses a semicolon instead of a comma after cb_param, and
> there is a stray semicolon immediately preceding the function body. This will
> cause a build failure when CONFIG_DMA_ENGINE is disabled.
>
> > +	return -ENODEV;
> > +}
> >  #endif
> >
> >  static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260522-dma_prep_submit-v2-0-7a87a5a29525@nxp.com?part=1

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

end of thread, other threads:[~2026-05-22 21:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22 20:13 [PATCH v2 0/2] dmaengine: add helper macro dmaengine_prep_submit() Frank.Li
2026-05-22 20:13 ` [PATCH v2 1/2] dmaengine: Add helper dmaengine_prep_submit_slave_single() Frank.Li
2026-05-22 20:45   ` sashiko-bot
2026-05-22 21:36     ` Frank Li
2026-05-22 20:13 ` [PATCH v2 2/2] i2c: imx-lpi2c: use dmaengine_prep_submit() to simple code Frank.Li
2026-05-22 21:15   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox