Re: [PATCH v2 08/14] spi: stm32-ospi: Open-code message transfer walk

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] A valid SPI message containing a transfer with length > 0 but both `tx_buf` and `rx_buf` set to NULL will cause a kernel crash when the driver dereferences the NULL buffer during FIFO writes.
- [High] Early error returns in `stm32_ospi_transfer_one_message` fail to finalize the SPI message, resulting in a permanent hang for callers.
--

commit 77e013496da647c31add0f5085a07d31c7c250b2
Author: Kaitao Cheng <chengkaitao@kylinos.cn>

spi: stm32-ospi: Open-code message transfer walk

This commit modifies stm32_ospi_transfer_one_message() to open-code the
list iteration over message transfers. It prepares the loop to observe
cursor updates explicitly, anticipating a later list iterator update.

> diff --git a/drivers/spi/spi-stm32-ospi.c b/drivers/spi/spi-stm32-ospi.c
> index 3757f6ba8fc61..fe92bb06251be 100644
> --- a/drivers/spi/spi-stm32-ospi.c
> +++ b/drivers/spi/spi-stm32-ospi.c
> @@ -674,7 +674,9 @@ static int stm32_ospi_transfer_one_message(struct spi_controller *ctrl,

[Severity: High]
This is a pre-existing issue, but do the early error returns in
stm32_ospi_transfer_one_message() fail to finalize the SPI message?

If cs_gpiod is null or pm_runtime_resume_and_get() fails, the function
returns early:

	if (!cs_gpiod)
		return -EOPNOTSUPP;

	ret = pm_runtime_resume_and_get(ospi->dev);
	if (ret < 0)
		return ret;

Since these paths bypass spi_finalize_current_message(), does this leave
the SPI core waiting indefinitely and cause a deadlock for callers?

[Severity: High]
This is also a pre-existing issue, but can this driver crash if an SPI
message contains a dummy transfer with both tx_buf and rx_buf set to NULL?

The SPI core allows dummy transfers with both buffers set to NULL. If a
transfer reaches stm32_ospi_transfer_one_message() without the dummy_data
flag set, the driver defaults to a write operation and assigns the NULL
tx_buf to the operation:

		} else {
			ospi->fmode = CR_FMODE_INDW;
			op.data.buswidth = transfer->tx_nbits;
			op.data.dir = SPI_MEM_DATA_OUT;
			op.data.buf.out = transfer->tx_buf;
		}

This eventually leads to stm32_ospi_send() and stm32_ospi_xfer(). Because
DMA mapping of a NULL pointer fails, it falls back to PIO mode by calling
stm32_ospi_poll().

Inside stm32_ospi_poll(), the stm32_ospi_write_fifo() callback is invoked,
which dereferences the NULL pointer directly:

	switch (len) {
	case sizeof(u32):
		writel_relaxed(*((u32 *)val), addr);
		break;

Could this trigger a kernel panic and allow a local denial of service?

>  
>  	gpiod_set_value_cansleep(cs_gpiod, true);
>  
> -	list_for_each_entry(transfer, &msg->transfers, transfer_list) {
> +	for (transfer = list_first_entry(&msg->transfers, typeof(*transfer), transfer_list);
> +	     !list_entry_is_head(transfer, &msg->transfers, transfer_list);
> +	     transfer = list_next_entry(transfer, transfer_list)) {
>  		u8 dummy_bytes = 0;
>  
>  		memset(&op, 0, sizeof(op));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=8