Re: [PATCH 4/7] Input: s6sy761 - add SPI support for S6SY791

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] input: s6sy761: DMA API violation in i2c_transfer with stack buffers
--

commit 4918bc0af27755ebb84f3e96213a8de871d7f898
Author: Lukas Timmermann <foss@timmermann.space>

Input: s6sy761 - add SPI support for S6SY791

This commit adds SPI support for the S6SY791 touchscreen controller.
It implements the SPI framing and read delays based on the downstream driver.
The new functionality operates over full-duplex SPI with padded transfers.

> diff --git a/drivers/input/touchscreen/s6sy761_spi.c b/drivers/input/touchscreen/s6sy761_spi.c
> new file mode 100644
> index 0000000000000..c6627e61c2856
> --- /dev/null
> +++ b/drivers/input/touchscreen/s6sy761_spi.c

[ ... ]

> +static int s6sy761_spi_read(struct device *dev, u8 *xfer_buf,
> +			    u8 cmd, size_t length, void *values)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct spi_transfer reply = { };

[ ... ]

> +	ret = spi_sync_transfer(spi, &reply, 1);
> +	if (ret)
> +		return ret;
> +
> +	if (xfer_buf[0] != S6SY761_SPI_SYNC_CODE ||
> +	    xfer_buf[5] != cmd ||
> +	    xfer_buf[resp_len - 1] !=
> +			s6sy761_spi_checksum(xfer_buf, resp_len - 1))
> +		return -EIO;
> +
> +	memcpy(values, xfer_buf + S6SY761_SPI_READ_HEADER_SIZE, length);
> +
> +	return 0;
> +}

[Severity: High]
While the SPI implementation safely copies the response into the values buffer
using memcpy, does the I2C implementation violate DMA safety requirements
by passing this buffer directly into i2c_transfer?

Looking at drivers/input/touchscreen/s6sy761_core.c:s6sy761_power_on(),
it allocates a small buffer on the stack and passes it down to the bus read
routine:

    u8 buffer[S6SY761_EVENT_SIZE];
    ...
    ret = s6sy761_read(sdata, S6SY761_READ_ONE_EVENT, S6SY761_EVENT_SIZE, buffer);

This calls s6sy761_i2c_read() in drivers/input/touchscreen/s6sy761_i2c.c,
which assigns both the caller's stack pointer (values) and its own local
stack variable (&cmd) directly to struct i2c_msg buffers:

	struct i2c_msg msgs[2] = {
		{
			.addr	= client->addr,
			.len	= 1,
			.buf	= &cmd,
		},
		{
			.addr	= client->addr,
			.flags	= I2C_M_RD,
			.len	= length,
			.buf	= values,
		},
	};
	int ret;

	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));

It then calls i2c_transfer() which may use DMA. Stack memory is not DMA-safe
(especially with CONFIG_VMAP_STACK), leading to potential memory corruption or
kernel panics when DMA operates on stack addresses. Is it possible to use the
dynamically allocated xfer_buf for I2C transfers instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-lat3st-google-raven-touch-upstream-v1-0-2a9f55159b9e@timmermann.space?part=4