Re: [PATCH 1/5] spi: spi-sun4i: sun4i_spi_transfer_one(): fix transfers for SUN4I_FIFO_DEPTH

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] If the hardware reports a TX FIFO count greater than SUN4I_FIFO_DEPTH - 1 (e.g., a full FIFO of 64), sun4i_spi_fill_fifo encounters an integer underflow and enters a nearly infinite loop.
--

commit bfe90ee7eb547aed612b50ceef43a3538623eb4d
Author: Marc Kleine-Budde <mkl@pengutronix.de>

spi: spi-sun4i: sun4i_spi_transfer_one(): fix transfers for SUN4I_FIFO_DEPTH

This commit fixes SPI transfers with exactly 64 bytes by enabling the
TX-FIFO empty interrupt when the transfer length is greater than
SUN4I_FIFO_DEPTH - 1. Previously, the interrupt was only enabled for
transfers strictly larger than the 64-byte FIFO, causing timeouts since
only 63 bytes are initially filled.

> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index d5c16392cd4d7..2e23244539053 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -322,7 +322,7 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>  	sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC |
>  					 SUN4I_INT_CTL_RF_F34);
>  	/* Only enable Tx FIFO interrupt if we really need it */
> -	if (tx_len > SUN4I_FIFO_DEPTH)
> +	if (tx_len > SUN4I_FIFO_DEPTH - 1)
>  		sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34);

[Severity: High]
This isn't a bug introduced by this patch, but does the hardware reporting a
TX FIFO count greater than SUN4I_FIFO_DEPTH - 1 cause an integer underflow?

If sun4i_spi_get_tx_fifo_count(sspi) returns a value >= 64 (which is
structurally possible since SUN4I_FIFO_DEPTH is 64 and the register mask is
0x7f), the unsigned subtraction in sun4i_spi_fill_fifo() underflows:

drivers/spi/spi-sun4i.c:sun4i_spi_fill_fifo() {
	cnt = SUN4I_FIFO_DEPTH - 1 - sun4i_spi_get_tx_fifo_count(sspi);

	len = min_t(int, cnt, sspi->len);

	while (len--) {
		...
}

This underflow produces 0xFFFFFFFF, which min_t(int, cnt, sspi->len) casts
to -1.

This negative length could cause the while (len--) loop to evaluate -1 as
true, turning it into an infinite loop that executes ~4 billion times and
overruns sspi->tx_buf.

This problem appears to be made more likely by the subsequent commit "spi:
spi-sun4i: sun4i_spi_fill_fifo(): always apply A10 FIFO fill level
workaround" which lowered the underflow threshold from > 64 to >= 64.

Can this out-of-bounds memory access occur if the hardware FIFO is completely
full (64 bytes) or reports spurious values?

>  
>  	/* Start the transfer */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-spi-sun4i-fixes-v1-0-19985ef75673@pengutronix.de?part=1