Re: [PATCH v8 2/2] Input: Add support for Wacom W9000-series penabled touchscreens

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver uses stack-allocated buffers for I2C transfers, which violates the DMA API and can cause panics or memory corruption.
--

commit 5c60cba7d031762dd1ed0e3769ce031bf93c9b48
Author: Hendrik Noack <hendrik-noack@gmx.de>

Input: Add support for Wacom W9000-series penabled touchscreens

This commit adds a new input driver for the Wacom W9000-series penabled
touchscreens, supporting passive Wacom pens over an I2C interface.

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

[ ... ]

> +static int wacom_w9000_read(struct i2c_client *client, u8 command, u8 len, u8 *data)
> +{
> +	int error, res;
> +	struct i2c_msg msg[] = {
> +		{
> +			.addr = client->addr,
> +			.flags = 0,
> +			.buf = &command,
> +			.len = sizeof(command),

[Severity: High]
Is it safe to pass a stack-allocated variable (&command) as an I2C transfer
buffer? 

If the underlying I2C adapter uses DMA, transferring from a stack address
can lead to kernel panics or silent memory corruption. This happens because
stack memory is often virtually mapped (when CONFIG_VMAP_STACK is enabled)
and is not guaranteed to be safely cacheline aligned.

Would it be better to allocate these buffers dynamically or embed a 
DMA-safe buffer inside the wacom_w9000_data structure?

> +		}, {
> +			.addr = client->addr,
> +			.flags = I2C_M_RD,
> +			.buf = data,
> +			.len = len,
> +		}
> +	};
> +
> +	res = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> +	if (res != ARRAY_SIZE(msg)) {
> +		error = res < 0 ? res : -EIO;
> +		dev_err(&client->dev, "%s: i2c transfer failed: %d (%d)\n", __func__, error, res);
> +		return error;
> +	}
> +
> +	return 0;
> +}
> +
> +static int wacom_w9000_query(struct wacom_w9000_data *wacom_data)
> +{
> +	struct i2c_client *client = wacom_data->client;
> +	struct device *dev = &wacom_data->client->dev;
> +	int error;
> +	int retry = 0;
> +	u8 data[CMD_QUERY_NUM_MAX];

[Severity: High]
Similarly, does passing this stack-allocated array down to wacom_w9000_read()
violate the DMA API requirements for the same reasons?

> +
> +	for (; retry < CMD_QUERY_RETRIES; retry++) {
> +		error = wacom_w9000_read(client, CMD_QUERY, wacom_data->variant->cmd_query_num,
> +					 data);

[ ... ]

> +static void wacom_w9000_coord(struct wacom_w9000_data *wacom_data)
> +{
> +	struct i2c_client *client = wacom_data->client;
> +	struct device *dev = &wacom_data->client->dev;
> +	int error;
> +	u8 data[MSG_COORD_NUM_MAX];
> +	bool touch, rubber, side_button;
> +	u16 x, y, pressure;
> +	u8 distance = 0;
> +
> +	error = i2c_master_recv(client, data, wacom_data->variant->msg_coord_num);

[Severity: High]
Can this call to i2c_master_recv() corrupt memory by DMAing into the
stack-allocated data array? Using a dedicated buffer allocated with kzalloc
or similar might prevent this issue on the coordinate data path.

> +	if (error != wacom_data->variant->msg_coord_num) {
> +		if (error >= 0)
> +			error = -EIO;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528074818.12151-1-hendrik-noack@gmx.de?part=2