mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] spi: amd: fix len==0 underflow and FIFO overflow
@ 2026-10-14 12:39 Weibin Liu
  2026-10-15 22:25 ` Mark Brown
  2026-10-16 11:27 ` M, Krishnamoorthi
  0 siblings, 2 replies; 4+ messages in thread
From: Weibin Liu @ 2026-10-14 12:39 UTC (permalink / raw)
  To: linux-spi, broonie, Raju.Rangoju, krishnamoorthi.m
  Cc: linux-kernel, sanju.mehta, stable, Guanglei Zhu, Weibin Liu

amd_spi_fifo_xfer() unconditionally decrements xfer->len when a tx_buf
is present. The SPI core does not reject zero-length transfers, so a
transfer submitted through the SPI_IOC_MESSAGE ioctl with len == 0 and
a tx_buf makes the u32 len wrap to 0xFFFFFFFF, and the following
per-byte loop writes far past the FIFO window.

The driver also never checks the cumulative FIFO offset against
AMD_SPI_FIFO_SIZE. The core splits each transfer down to the
controller's max transfer size but does not bound the total message
length, so a message of several transfers whose total exceeds the FIFO
overflows it with out-of-bounds MMIO writes (tx) and reads (rx).

Skip zero-length transfers and reject any transfer that would run past
the end of the FIFO window.

Tested with a user-space replica of the fifo_xfer length arithmetic
(ASAN build): a zero-length transfer with tx_buf makes len-- wrap to
0xffffffff, and skipping zero-length transfers avoids the wrap. No AMD
SPI hardware was available, so the FIFO bound check was verified by
inspection against AMD_SPI_FIFO_SIZE.

Fixes: bbb336f39efc ("spi: spi-amd: Add AMD SPI controller driver support")
Cc: stable@vger.kernel.org # 5.8+
Reviewed-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
---
 drivers/spi/spi-amd.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 71a6e5c475b0..ddf89e5353c5 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -322,11 +322,22 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 
 	list_for_each_entry(xfer, &message->transfers,
 			    transfer_list) {
+		/* Skip zero-length transfers to avoid len underflow below */
+		if (!xfer->len)
+			continue;
+
 		if (xfer->speed_hz)
 			amd_set_spi_freq(amd_spi, xfer->speed_hz);
 		else
 			amd_set_spi_freq(amd_spi, spi->max_speed_hz);
 
+		/* Reject transfers that would overflow the FIFO window */
+		if (fifo_pos + xfer->len >
+		    AMD_SPI_FIFO_BASE + AMD_SPI_FIFO_SIZE) {
+			message->status = -EMSGSIZE;
+			goto fin_msg;
+		}
+
 		if (xfer->tx_buf) {
 			buf = (u8 *)xfer->tx_buf;
 			if (!tx_len) {
@@ -369,6 +380,12 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 
 		list_for_each_entry(xfer, &message->transfers, transfer_list)
 			if (xfer->rx_buf) {
+				/* Reject transfers that would overflow the FIFO window */
+				if (fifo_pos + xfer->len >
+				    AMD_SPI_FIFO_BASE + AMD_SPI_FIFO_SIZE) {
+					message->status = -EMSGSIZE;
+					goto fin_msg;
+				}
 				buf = (u8 *)xfer->rx_buf;
 				/* Read data from FIFO to receive buffer */
 				for (i = 0; i < xfer->len; i++)

---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20261014-spi-amd-fifo-fix-37ac4ef05bf6

Best regards,
-- 
Weibin Liu <liuwb@xiaopeng.com>


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

* Re: [PATCH] spi: amd: fix len==0 underflow and FIFO overflow
  2026-10-14 12:39 [PATCH] spi: amd: fix len==0 underflow and FIFO overflow Weibin Liu
@ 2026-10-15 22:25 ` Mark Brown
  2026-10-16 12:16   ` M, Krishnamoorthi
  2026-10-16 11:27 ` M, Krishnamoorthi
  1 sibling, 1 reply; 4+ messages in thread
From: Mark Brown @ 2026-10-15 22:25 UTC (permalink / raw)
  To: linux-spi, Raju.Rangoju, krishnamoorthi.m, Weibin Liu
  Cc: linux-kernel, sanju.mehta, stable, Guanglei Zhu

On Wed, 14 Oct 2026 20:39:13 +0800, Weibin Liu wrote:
> spi: amd: fix len==0 underflow and FIFO overflow

Applied to

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

Thanks!

[1/1] spi: amd: fix len==0 underflow and FIFO overflow
      https://git.kernel.org/broonie/spi/c/e2f4edfc061e

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


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

* Re: [PATCH] spi: amd: fix len==0 underflow and FIFO overflow
  2026-10-14 12:39 [PATCH] spi: amd: fix len==0 underflow and FIFO overflow Weibin Liu
  2026-10-15 22:25 ` Mark Brown
@ 2026-10-16 11:27 ` M, Krishnamoorthi
  1 sibling, 0 replies; 4+ messages in thread
From: M, Krishnamoorthi @ 2026-10-16 11:27 UTC (permalink / raw)
  To: Weibin Liu, linux-spi, broonie, Raju.Rangoju
  Cc: linux-kernel, sanju.mehta, stable, Guanglei Zhu

Hi,


On 10/14/2026 6:09 PM, Weibin Liu wrote:
> [You don't often get email from liuwb@xiaopeng.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> amd_spi_fifo_xfer() unconditionally decrements xfer->len when a tx_buf
> is present. The SPI core does not reject zero-length transfers, so a
> transfer submitted through the SPI_IOC_MESSAGE ioctl with len == 0 and
> a tx_buf makes the u32 len wrap to 0xFFFFFFFF, and the following
> per-byte loop writes far past the FIFO window.
> 
> The driver also never checks the cumulative FIFO offset against
> AMD_SPI_FIFO_SIZE. The core splits each transfer down to the
> controller's max transfer size but does not bound the total message
> length, so a message of several transfers whose total exceeds the FIFO
> overflows it with out-of-bounds MMIO writes (tx) and reads (rx).
> 
> Skip zero-length transfers and reject any transfer that would run past
> the end of the FIFO window.
> 
> Tested with a user-space replica of the fifo_xfer length arithmetic
> (ASAN build): a zero-length transfer with tx_buf makes len-- wrap to
> 0xffffffff, and skipping zero-length transfers avoids the wrap. No AMD
> SPI hardware was available, so the FIFO bound check was verified by
> inspection against AMD_SPI_FIFO_SIZE.
> 
> Fixes: bbb336f39efc ("spi: spi-amd: Add AMD SPI controller driver support")
> Cc: stable@vger.kernel.org # 5.8+
> Reviewed-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
> ---
>   drivers/spi/spi-amd.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
> index 71a6e5c475b0..ddf89e5353c5 100644
> --- a/drivers/spi/spi-amd.c
> +++ b/drivers/spi/spi-amd.c
> @@ -322,11 +322,22 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
> 
>          list_for_each_entry(xfer, &message->transfers,
>                              transfer_list) {
> +               /* Skip zero-length transfers to avoid len underflow below */
> +               if (!xfer->len)
> +                       continue;
> +
>                  if (xfer->speed_hz)
>                          amd_set_spi_freq(amd_spi, xfer->speed_hz);
>                  else
>                          amd_set_spi_freq(amd_spi, spi->max_speed_hz);
> 
> +               /* Reject transfers that would overflow the FIFO window */
> +               if (fifo_pos + xfer->len >
> +                   AMD_SPI_FIFO_BASE + AMD_SPI_FIFO_SIZE) {
> +                       message->status = -EMSGSIZE;
> +                       goto fin_msg;
> +               }
> +
>                  if (xfer->tx_buf) {
>                          buf = (u8 *)xfer->tx_buf;
>                          if (!tx_len) {
> @@ -369,6 +380,12 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
> 
>                  list_for_each_entry(xfer, &message->transfers, transfer_list)
>                          if (xfer->rx_buf) {
> +                               /* Reject transfers that would overflow the FIFO window */
> +                               if (fifo_pos + xfer->len >
> +                                   AMD_SPI_FIFO_BASE + AMD_SPI_FIFO_SIZE) {
> +                                       message->status = -EMSGSIZE;
> +                                       goto fin_msg;
> +                               }
>                                  buf = (u8 *)xfer->rx_buf;
>                                  /* Read data from FIFO to receive buffer */
>                                  for (i = 0; i < xfer->len; i++)
> 
> ---

Thanks for the fix. The zero-length skip and the overflow bound are both 
correct, but the FIFO check can be done more simply and, more 
importantly, before the command is issued.

fifo_pos only advances in the tx branch, so this form can't track the 
cumulative rx length in the first loop. That's why you need the second 
copy in the drain loop - but by then amd_spi_execute_opcode() has 
already run, so an oversized message still drives one SPI transaction 
before being rejected.

Please use tx_len + rx_len against AMD_SPI_FIFO_SIZE and check it in the 
first loop only, so the whole message is validated before execute and 
the drain-loop check can be dropped:

if (xfer->tx_buf) {
     ...
     tx_len += xfer->len;
     if (tx_len + rx_len > AMD_SPI_FIFO_SIZE) {
         message->status = -EMSGSIZE;
         goto fin_msg;
     }
     ...
}
...
/* Store no. of bytes to be received from FIFO */
if (xfer->rx_buf) {
     rx_len += xfer->len;
     if (tx_len + rx_len > AMD_SPI_FIFO_SIZE) {
         message->status = -EMSGSIZE;
         goto fin_msg;
     }
}


Thanks,
Krishna
> base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
> change-id: 20261014-spi-amd-fifo-fix-37ac4ef05bf6
> 
> Best regards,
> --
> Weibin Liu <liuwb@xiaopeng.com>
> 


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

* Re: [PATCH] spi: amd: fix len==0 underflow and FIFO overflow
  2026-10-15 22:25 ` Mark Brown
@ 2026-10-16 12:16   ` M, Krishnamoorthi
  0 siblings, 0 replies; 4+ messages in thread
From: M, Krishnamoorthi @ 2026-10-16 12:16 UTC (permalink / raw)
  To: Mark Brown, linux-spi, Raju.Rangoju, Weibin Liu
  Cc: linux-kernel, sanju.mehta, stable, Guanglei Zhu

Hi Mark,

On 10/16/2026 3:55 AM, Mark Brown wrote:
> On Wed, 14 Oct 2026 20:39:13 +0800, Weibin Liu wrote:
>> spi: amd: fix len==0 underflow and FIFO overflow
> 
> Applied to
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.4
> 
> Thanks!
> 

I sent review comments in parallel without seeing the apply mail - 
they're not a blocker, just a minor improvement.

The applied change is already memory-safe; the rx FIFO bound is just 
checked after amd_spi_execute_opcode(), so an oversized message drives 
one transaction before being rejected. Since the patch is applied, the 
change can be taken in a follow-up patch.


Thanks,
Krishna

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

end of thread, other threads:[~2026-10-16 12:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-14 12:39 [PATCH] spi: amd: fix len==0 underflow and FIFO overflow Weibin Liu
2026-10-15 22:25 ` Mark Brown
2026-10-16 12:16   ` M, Krishnamoorthi
2026-10-16 11:27 ` M, Krishnamoorthi

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

all inboxes | Powered by JetHome®