Re: [PATCH v2] i2c: tegra: check msg length in SMBUS block read

[Thierry Reding]

On Thu, Mar 20, 2025 at 06:51:44PM +0530, Akhil R wrote:
> For SMBUS block read, do not continue to read if the message length
> passed from the device is '0' or greater than the maximum allowed bytes.
> 
> Signed-off-by: Akhil R <akhilrajeev@xxxxxxxxxx>
> ---
> v1->v2: Add check for the maximum data as well.
> 
>  drivers/i2c/busses/i2c-tegra.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 87976e99e6d0..049b4d154c23 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1395,6 +1395,11 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
>  			ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], MSG_END_CONTINUE);
>  			if (ret)
>  				break;
> +
> +			/* Validate message length before proceeding */
> +			if (msgs[i].buf[0] == 0 || msgs[i].buf[0] > I2C_SMBUS_BLOCK_MAX)

I wonder if this can ever happen. Looking at the implementation of the
i2c_smbus_{read,write}_i2c_block_data() functions, they already cap the
length at I2C_SMBUS_BLOCK_MAX.

I suppose some user could be explicitly sending off messages with bad
lengths, but wouldn't it be better to return an error in that case
instead of just aborting silently?

Thierry

> +				break;
> +
>  			/* Set the msg length from first byte */
>  			msgs[i].len += msgs[i].buf[0];
>  			dev_dbg(i2c_dev->dev, "reading %d bytes\n", msgs[i].len);
> -- 
> 2.43.2
>

Attachment: signature.asc
Description: PGP signature