Re: [PATCH 3/3] Input: mms114 - fix Y-resolution configuration

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unsafe DMA buffer allocation on the stack for I2C transfers.
--

commit b2ece0d05aab20ebd4014035e04b87c4d9127c9f
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Input: mms114 - fix Y-resolution configuration

This commit fixes a copy-paste error in mms114_setup_regs() where
props->max_x was being used instead of props->max_y to configure the
lower bits of the Y resolution.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 84afdadb3bccc..27911a9f4e9eb 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -408,7 +408,7 @@ static int mms114_setup_regs(struct mms114_data *data)
>  	if (error < 0)
>  		return error;
>  
> -	val = props->max_x & 0xff;
> +	val = props->max_y & 0xff;
>  	error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);

[Severity: High]
This is a pre-existing issue, but since mms114_write_reg() is called here,
is it safe for the driver to use stack-allocated buffers for I2C transfers?

Looking at mms114_write_reg():

static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
{
        struct i2c_client *client = data->client;
        u8 buf[2];
        ...
        buf[0] = reg;
        buf[1] = val;

        error = i2c_master_send(client, buf, 2);

If the underlying I2C adapter uses DMA for transfers and CONFIG_VMAP_STACK is
enabled, passing a stack-allocated buffer to i2c_master_send() can cause memory
corruption or kernel panics because stack memory is not DMA-safe.

A similar pattern appears in __mms114_read_reg() and mms114_interrupt():

static int __mms114_read_reg(struct mms114_data *data, unsigned int reg, ...)
{
        ...
        u8 buf = reg;
        ...
        xfer[0].buf = &buf;
        ...
        error = i2c_transfer(client->adapter, xfer, 2);

and in mms114_interrupt(), a large array is allocated on the stack and passed
down through __mms114_read_reg():

        struct mms114_touch touch[MMS114_MAX_TOUCH];
        ...
        error = __mms114_read_reg(data, MMS114_INFORMATION,
                                  MMS114_MAX_TOUCH * sizeof(struct mms114_touch),
                                  (u8 *)touch);

Could these stack-allocated buffers lead to instability on DMA-capable I2C
controllers? Should these buffers be moved to the mms114_data struct so they
are dynamically allocated on the heap?

>  	if (error < 0)
>  		return error;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704060115.353049-1-dmitry.torokhov@gmail.com?part=3