Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH] i2c: mxs: fix DMA channel leak on probe error
@ 2026-08-14 13:40 Ruoyu Wang
  2026-08-14 13:49 ` sashiko-bot
  2026-08-14 14:35 ` Frank Li
  0 siblings, 2 replies; 3+ messages in thread
From: Ruoyu Wang @ 2026-08-14 13:40 UTC (permalink / raw)
  To: Andi Shyti, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Marek Vasut, Wolfram Sang
  Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Ruoyu Wang

dma_request_chan() grants the driver exclusive use of the channel until
dma_release_channel() is called. If controller reset or I2C adapter
registration fails after the request, mxs_i2c_probe() returns without
releasing it. The failed probe does not invoke mxs_i2c_remove(), so the
channel remains allocated and can prevent a later probe from acquiring
it.

Release the DMA channel on both post-request error paths. Keep the
adapter-registration unwind ordered after the existing controller reset.

This issue was found by a static analysis checker and confirmed by
manual source review.

Fixes: 62885f59a261 ("MXS: Implement DMA support into mxs-i2c")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/i2c/busses/i2c-mxs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 4e07babea9c3f..a5c8b44ec8142 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -849,8 +849,10 @@ static int mxs_i2c_probe(struct platform_device *pdev)
 
 	/* Do reset to enforce correct startup after pinmuxing */
 	err = mxs_i2c_reset(i2c);
-	if (err)
+	if (err) {
+		dma_release_channel(i2c->dmach);
 		return err;
+	}
 
 	adap = &i2c->adapter;
 	strscpy(adap->name, "MXS I2C adapter", sizeof(adap->name));
@@ -865,6 +867,7 @@ static int mxs_i2c_probe(struct platform_device *pdev)
 	if (err) {
 		writel(MXS_I2C_CTRL0_SFTRST,
 				i2c->regs + MXS_I2C_CTRL0_SET);
+		dma_release_channel(i2c->dmach);
 		return err;
 	}
 
-- 
2.51.0


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

* Re: [PATCH] i2c: mxs: fix DMA channel leak on probe error
  2026-08-14 13:40 [PATCH] i2c: mxs: fix DMA channel leak on probe error Ruoyu Wang
@ 2026-08-14 13:49 ` sashiko-bot
  2026-08-14 14:35 ` Frank Li
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-14 13:49 UTC (permalink / raw)
  To: Ruoyu Wang; +Cc: Frank.Li, imx

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

Pre-existing issues:
- [High] The driver maps client-provided `msg->buf` directly for DMA without using `i2c_get_dma_safe_msg_buf()`, leading to potential memory corruption.
- [High] The driver ignores the return value of `dma_map_sg()`, risking DMA engine faults and memory corruption if mapping fails.
- [High] The `timeout` error path in `mxs_i2c_xfer_msg()` unmaps DMA buffers without terminating the active DMA channel, leading to potential use-after-unmap and double-unmap bugs.
--

--- Patch [1]: [PATCH] i2c: mxs: fix DMA channel leak on probe error ---
commit b8eb950d1bfe51582e811624063eaccb540a465b
Author: Ruoyu Wang <ruoyuw560@gmail.com>

i2c: mxs: fix DMA channel leak on probe error

This commit fixes a DMA channel leak when mxs_i2c_probe() fails during
controller reset or adapter registration. It ensures dma_release_channel()
is called on these error paths.

> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
[ ... ]
> @@ -197,6 +197,7 @@ static void mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap,
>  
>  		/* Queue the DMA data transfer. */
>  		sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1);
>  		dma_map_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);

[Severity: High]
This isn't a bug introduced by this patch, but does this code ignore the
return value of dma_map_sg()?

If mapping fails under memory pressure or IOMMU fragmentation, dma_map_sg()
will return 0, and the unmapped scatterlist will be queued to the DMA engine,
which could lead to DMA hardware accessing invalid memory.

This pattern also occurs later in mxs_i2c_dma_setup_xfer() for both the read
and write transfer mapping paths.

[ ... ]
> @@ -227,6 +227,7 @@ static void mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap,
>  
>  		/* Queue the DMA data transfer. */
>  		sg_init_one(&i2c->sg_io[1], msg->buf, msg->len);
>  		dma_map_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);

[Severity: High]
This is a pre-existing issue, but is it safe to map the client-provided
msg->buf directly for DMA here?

The I2C subsystem allows clients to pass stack or vmalloc buffers for
standard transfers. Without allocating a bounce buffer via
i2c_get_dma_safe_msg_buf(), this could result in direct DMA into stack
memory and cause memory corruption.

This direct mapping also occurs in the write transfer path below.

[ ... ]
> @@ -629,6 +634,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
>  timeout:
>  	dev_dbg(i2c->dev, "Timeout!\n");
>  	mxs_i2c_dma_finish(i2c);

[Severity: High]
This is a pre-existing issue, but does the timeout handler fail to terminate
the pending DMA transaction?

Since dmaengine_terminate_sync() is not called after a timeout, the active
DMA engine might continue accessing the unmapped memory.

If it eventually finishes, mxs_i2c_dma_irq_callback() could run and call
mxs_i2c_dma_finish() a second time on the same buffers.

>  	ret = mxs_i2c_reset(i2c);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814134033.1386874-1-ruoyuw560@gmail.com?part=1

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

* Re: [PATCH] i2c: mxs: fix DMA channel leak on probe error
  2026-08-14 13:40 [PATCH] i2c: mxs: fix DMA channel leak on probe error Ruoyu Wang
  2026-08-14 13:49 ` sashiko-bot
@ 2026-08-14 14:35 ` Frank Li
  1 sibling, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-08-14 14:35 UTC (permalink / raw)
  To: Ruoyu Wang
  Cc: Andi Shyti, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Marek Vasut, Wolfram Sang, linux-i2c, imx,
	linux-arm-kernel, linux-kernel

On Fri, Aug 14, 2026 at 09:40:33PM +0800, Ruoyu Wang wrote:
> [You don't often get email from ruoyuw560@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> dma_request_chan() grants the driver exclusive use of the channel until
> dma_release_channel() is called. If controller reset or I2C adapter
> registration fails after the request, mxs_i2c_probe() returns without
> releasing it. The failed probe does not invoke mxs_i2c_remove(), so the
> channel remains allocated and can prevent a later probe from acquiring
> it.
>
> Release the DMA channel on both post-request error paths. Keep the
> adapter-registration unwind ordered after the existing controller reset.
>
> This issue was found by a static analysis checker and confirmed by
> manual source review.
>
> Fixes: 62885f59a261 ("MXS: Implement DMA support into mxs-i2c")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/i2c/busses/i2c-mxs.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 4e07babea9c3f..a5c8b44ec8142 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -849,8 +849,10 @@ static int mxs_i2c_probe(struct platform_device *pdev)
>
>         /* Do reset to enforce correct startup after pinmuxing */
>         err = mxs_i2c_reset(i2c);
> -       if (err)
> +       if (err) {
> +               dma_release_channel(i2c->dmach);

Please use devm_dma_request_chan() to fix this problem

Frank
>                 return err;
> +       }
>
>         adap = &i2c->adapter;
>         strscpy(adap->name, "MXS I2C adapter", sizeof(adap->name));
> @@ -865,6 +867,7 @@ static int mxs_i2c_probe(struct platform_device *pdev)
>         if (err) {
>                 writel(MXS_I2C_CTRL0_SFTRST,
>                                 i2c->regs + MXS_I2C_CTRL0_SET);
> +               dma_release_channel(i2c->dmach);
>                 return err;
>         }
>
> --
> 2.51.0
>
>

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

end of thread, other threads:[~2026-08-14 14:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 13:40 [PATCH] i2c: mxs: fix DMA channel leak on probe error Ruoyu Wang
2026-08-14 13:49 ` sashiko-bot
2026-08-14 14:35 ` Frank Li

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