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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2rlnnjixgd65u6gbqxfuhzu5humehvjth7iysj23xvuv5fi2ft@i5su6kfrqnt5>
Date: Fri, 21 Mar 2025 13:28:03 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Akhil R <akhilrajeev@...dia.com>
Cc: ldewangan@...dia.com, digetx@...il.com, andi.shyti@...nel.org, 
	jonathanh@...dia.com, wsa@...nel.org, linux-i2c@...r.kernel.org, 
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] i2c: tegra: check msg length in SMBUS block read

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@...dia.com>
> ---
> 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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)