[PATCH 0/3] spi: imx: Three fixes for the i.MX SPI driver

[PATCH 0/3] spi: imx: Three fixes for the i.MX SPI driver

[John Madieu]

Hi All,

This series independent fixes found in the i.MX SPI driver.

These are:

1/3 fixes a precedence bug in spi_imx_dma_max_wml_find() that makes
    the watermark-finding logic effectively dead code. The function
    currently always returns wml = 1 because of how the !-operator
    binds to the modulo expression.

2/3 fixes a missing return on the package-1 failure path in
    spi_imx_dma_data_prepare(). The error path frees the
    dma_data array and the package-0 buffers, then falls through
    to "return 0" - the caller proceeds with a freed pointer.

3/3 makes spi_imx_setupxfer() propagate the prepare_transfer()
    return value. Currently a -EINVAL from mx51_ecspi_prepare_transfer
    (e.g. on a word_delay overflow) is silently swallowed and the
    transfer proceeds with a partially-configured controller.

Regards,

John Madieu (3):
  spi: imx: Fix precedence bug in spi_imx_dma_max_wml_find()
  spi: imx: Fix UAF on package-1 prepare failure in
    spi_imx_dma_data_prepare()
  spi: imx: Propagate prepare_transfer() error from spi_imx_setupxfer()

 drivers/spi/spi-imx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 1/3] spi: imx: Fix precedence bug in spi_imx_dma_max_wml_find()

[John Madieu]

The watermark search in spi_imx_dma_max_wml_find() reads:

	if (!dma_data->dma_len % (i * bytes_per_word))
		break;

The unary ! binds tighter than %, so this parses as:

	if ((!dma_data->dma_len) % (i * bytes_per_word))
		break;

!dma_data->dma_len is 0 or 1, and `0 % x == 0` for any x; `1 % x` is
0 unless x == 1. The condition is therefore false in every case
except dma_len != 0 with i * bytes_per_word == 1, i.e. i == 1 and
bytes_per_word == 1.

The loop almost always falls through to its end, leaving i == 0,
which the post-loop fallback rewrites to 1:

	if (i == 0)
		i = 1;

So spi_imx->wml ends up at 1 for essentially every DMA transfer,
defeating the entire purpose of the function. The DMA engine then
requests service after every single FIFO word instead of using
multi-word bursts, hurting throughput on every DMA-capable variant.

Add the missing parentheses so the modulo is computed first, then
negated:

	if (!(dma_data->dma_len % (i * bytes_per_word)))
		break;

Fixes: faa8e404ad8e ("spi: imx: support dynamic burst length for ECSPI DMA mode")
Signed-off-by: John Madieu <john.madieu@gmail.com>
---
 drivers/spi/spi-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index e5c907c45b87..7ae8078c10ef 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1836,7 +1836,7 @@ static void spi_imx_dma_max_wml_find(struct spi_imx_data *spi_imx,
 	unsigned int i;
 
 	for (i = spi_imx->devtype_data->fifo_size / 2; i > 0; i--) {
-		if (!dma_data->dma_len % (i * bytes_per_word))
+		if (!(dma_data->dma_len % (i * bytes_per_word)))
 			break;
 	}
 	/* Use 1 as wml in case no available burst length got */
-- 
2.25.1

Re: [PATCH 1/3] spi: imx: Fix precedence bug in spi_imx_dma_max_wml_find()

[Frank Li]

On Fri, May 01, 2026 at 01:59:49PM +0000, John Madieu wrote:
> The watermark search in spi_imx_dma_max_wml_find() reads:
>
> 	if (!dma_data->dma_len % (i * bytes_per_word))
> 		break;
>
> The unary ! binds tighter than %, so this parses as:
>
> 	if ((!dma_data->dma_len) % (i * bytes_per_word))
> 		break;
>
> !dma_data->dma_len is 0 or 1, and `0 % x == 0` for any x; `1 % x` is
> 0 unless x == 1. The condition is therefore false in every case
> except dma_len != 0 with i * bytes_per_word == 1, i.e. i == 1 and
> bytes_per_word == 1.
>
> The loop almost always falls through to its end, leaving i == 0,
> which the post-loop fallback rewrites to 1:
>
> 	if (i == 0)
> 		i = 1;
>
> So spi_imx->wml ends up at 1 for essentially every DMA transfer,
> defeating the entire purpose of the function. The DMA engine then
> requests service after every single FIFO word instead of using
> multi-word bursts, hurting throughput on every DMA-capable variant.
>
> Add the missing parentheses so the modulo is computed first, then
> negated:
>
> 	if (!(dma_data->dma_len % (i * bytes_per_word)))
> 		break;
>
> Fixes: faa8e404ad8e ("spi: imx: support dynamic burst length for ECSPI DMA mode")
> Signed-off-by: John Madieu <john.madieu@gmail.com>
> ---

Thank you for find this.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/spi/spi-imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index e5c907c45b87..7ae8078c10ef 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1836,7 +1836,7 @@ static void spi_imx_dma_max_wml_find(struct spi_imx_data *spi_imx,
>  	unsigned int i;
>
>  	for (i = spi_imx->devtype_data->fifo_size / 2; i > 0; i--) {
> -		if (!dma_data->dma_len % (i * bytes_per_word))
> +		if (!(dma_data->dma_len % (i * bytes_per_word)))
>  			break;
>  	}
>  	/* Use 1 as wml in case no available burst length got */
> --
> 2.25.1
>

[PATCH 2/3] spi: imx: Fix UAF on package-1 prepare failure in spi_imx_dma_data_prepare()

[John Madieu]

When transfer->len exceeds MX51_ECSPI_CTRL_MAX_BURST and is not a
multiple of it, spi_imx_dma_data_prepare() splits the transfer into
two DMA packages. If preparing the second package fails:

	ret = spi_imx_dma_tx_data_handle(spi_imx, &spi_imx->dma_data[1],
					 transfer->tx_buf + spi_imx->dma_data[0].data_len,
					 false);
	if (ret) {
		kfree(spi_imx->dma_data[0].dma_tx_buf);
		kfree(spi_imx->dma_data[0].dma_rx_buf);
		kfree(spi_imx->dma_data);
	}
	}

	return 0;

the function frees the package-0 buffers and the dma_data array,
then falls through to `return 0`, telling the caller the prepare
succeeded. The caller then dereferences the freed dma_data array,
producing a use-after-free.

Return the error from the failure path so the caller takes its
existing failure branch.

Fixes: faa8e404ad8e ("spi: imx: support dynamic burst length for ECSPI DMA mode")
Signed-off-by: John Madieu <john.madieu@gmail.com>
---
 drivers/spi/spi-imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 7ae8078c10ef..4e3dbd01d619 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1709,6 +1709,7 @@ static int spi_imx_dma_data_prepare(struct spi_imx_data *spi_imx,
 			kfree(spi_imx->dma_data[0].dma_tx_buf);
 			kfree(spi_imx->dma_data[0].dma_rx_buf);
 			kfree(spi_imx->dma_data);
+			return ret;
 		}
 	}
 
-- 
2.25.1

Re: [PATCH 2/3] spi: imx: Fix UAF on package-1 prepare failure in spi_imx_dma_data_prepare()

[Frank Li]

On Fri, May 01, 2026 at 01:59:50PM +0000, John Madieu wrote:
> When transfer->len exceeds MX51_ECSPI_CTRL_MAX_BURST and is not a
> multiple of it, spi_imx_dma_data_prepare() splits the transfer into
> two DMA packages. If preparing the second package fails:
>
> 	ret = spi_imx_dma_tx_data_handle(spi_imx, &spi_imx->dma_data[1],
> 					 transfer->tx_buf + spi_imx->dma_data[0].data_len,
> 					 false);
> 	if (ret) {
> 		kfree(spi_imx->dma_data[0].dma_tx_buf);
> 		kfree(spi_imx->dma_data[0].dma_rx_buf);
> 		kfree(spi_imx->dma_data);
> 	}
> 	}

Nit: duplicated }

>
> 	return 0;
>
> the function frees the package-0 buffers and the dma_data array,
> then falls through to `return 0`, telling the caller the prepare
> succeeded. The caller then dereferences the freed dma_data array,
> producing a use-after-free.
>
> Return the error from the failure path so the caller takes its
> existing failure branch.
>
> Fixes: faa8e404ad8e ("spi: imx: support dynamic burst length for ECSPI DMA mode")
> Signed-off-by: John Madieu <john.madieu@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/spi/spi-imx.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 7ae8078c10ef..4e3dbd01d619 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1709,6 +1709,7 @@ static int spi_imx_dma_data_prepare(struct spi_imx_data *spi_imx,
>  			kfree(spi_imx->dma_data[0].dma_tx_buf);
>  			kfree(spi_imx->dma_data[0].dma_rx_buf);
>  			kfree(spi_imx->dma_data);
> +			return ret;
>  		}
>  	}
>
> --
> 2.25.1
>

[PATCH 3/3] spi: imx: Propagate prepare_transfer() error from spi_imx_setupxfer()

[John Madieu]

spi_imx_setupxfer() calls the per-variant prepare_transfer()
callback and returns 0 unconditionally:

	spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);

	return 0;

mx51_ecspi_prepare_transfer() can return -EINVAL when the requested
word_delay does not fit in MX51_ECSPI_PERIOD_MASK. The error is
detected after a partial set of register writes (CTRL: BL, clkdiv,
SMC), so the controller is left in a partially-configured state and
the transfer is then submitted as if setup succeeded.

Propagate the return value. The other variants' prepare_transfer
callbacks all return 0, so this is a no-op for them.

Signed-off-by: John Madieu <john.madieu@gmail.com>
---
 drivers/spi/spi-imx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 4e3dbd01d619..480d1e8b281f 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1382,9 +1382,7 @@ static int spi_imx_setupxfer(struct spi_device *spi,
 		spi_imx->target_burst = t->len;
 	}
 
-	spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
-
-	return 0;
+	return spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
 }
 
 static void spi_imx_sdma_exit(struct spi_imx_data *spi_imx)
-- 
2.25.1

Re: [PATCH 3/3] spi: imx: Propagate prepare_transfer() error from spi_imx_setupxfer()

[Frank Li]

On Fri, May 01, 2026 at 01:59:51PM +0000, John Madieu wrote:
> spi_imx_setupxfer() calls the per-variant prepare_transfer()
> callback and returns 0 unconditionally:
>
> 	spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
>
> 	return 0;
>
> mx51_ecspi_prepare_transfer() can return -EINVAL when the requested
> word_delay does not fit in MX51_ECSPI_PERIOD_MASK. The error is
> detected after a partial set of register writes (CTRL: BL, clkdiv,
> SMC), so the controller is left in a partially-configured state and
> the transfer is then submitted as if setup succeeded.
>
> Propagate the return value. The other variants' prepare_transfer
> callbacks all return 0, so this is a no-op for them.
>
> Signed-off-by: John Madieu <john.madieu@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/spi/spi-imx.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 4e3dbd01d619..480d1e8b281f 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1382,9 +1382,7 @@ static int spi_imx_setupxfer(struct spi_device *spi,
>  		spi_imx->target_burst = t->len;
>  	}
>
> -	spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
> -
> -	return 0;
> +	return spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
>  }
>
>  static void spi_imx_sdma_exit(struct spi_imx_data *spi_imx)
> --
> 2.25.1
>

Re: [PATCH 0/3] spi: imx: Three fixes for the i.MX SPI driver

[Mark Brown]

On Fri, 01 May 2026 13:59:48 +0000, John Madieu wrote:
> spi: imx: Three fixes for the i.MX SPI driver
> 
> Hi All,
> 
> This series independent fixes found in the i.MX SPI driver.
> 
> These are:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.1

Thanks!

[1/3] spi: imx: Fix precedence bug in spi_imx_dma_max_wml_find()
      https://git.kernel.org/broonie/spi/c/24e0fd8b8520
[2/3] spi: imx: Fix UAF on package-1 prepare failure in spi_imx_dma_data_prepare()
      https://git.kernel.org/broonie/spi/c/f5b554825504
[3/3] spi: imx: Propagate prepare_transfer() error from spi_imx_setupxfer()
      https://git.kernel.org/broonie/spi/c/894e04b71162

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark