linux-kernel - Re: [PATCH] i2c: busses: Fix out-of-bounds bug in mchp_corei2c_smbus

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] [day] [month] [year] [list]

Message-ID: <20250619-jubilant-nag-c50bdebdac0c@wendy>
Date: Thu, 19 Jun 2025 08:33:10 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: Alex Guo <alexguo1023@...il.com>
CC: <daire.mcnamara@...rochip.com>, <andi.shyti@...nel.org>,
	<linux-riscv@...ts.infradead.org>, <linux-i2c@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c: busses: Fix out-of-bounds bug in
 mchp_corei2c_smbus_xfer

On Sun, Jun 15, 2025 at 07:49:19PM -0400, Alex Guo wrote:
> The data->block[0] variable comes from user. Without proper check,
> the variable may be very large to cause an out-of-bounds bug.
> 
> Fix this bug by checking the value of data->block[0] first.
> 
> Similar commit:
> 1. commit 39244cc7548 ("i2c: ismt: Fix an out-of-bounds bug in
> ismt_access()")
> 2. commit 92fbb6d1296 ("i2c: xgene-slimpro: Fix out-of-bounds
> bug in xgene_slimpro_i2c_xfer()")
> 
> Signed-off-by: Alex Guo <alexguo1023@...il.com>
> ---
>  drivers/i2c/busses/i2c-microchip-corei2c.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-microchip-corei2c.c b/drivers/i2c/busses/i2c-microchip-corei2c.c
> index 492bf4c34722..a79d4d327f20 100644
> --- a/drivers/i2c/busses/i2c-microchip-corei2c.c
> +++ b/drivers/i2c/busses/i2c-microchip-corei2c.c
> @@ -492,6 +492,8 @@ static int mchp_corei2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned
>  		if (read_write == I2C_SMBUS_WRITE) {
>  			int data_len;
>  
> +			if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX)
> +				return -EINVAL;

Seems reasonable, but I'd like to see a blank line here after the
return. Maybe Andi can do it on application?

Acked-by: Conor Dooley <conor.dooley@...rochip.com>

>  			data_len = data->block[0];
>  			msgs[CORE_I2C_SMBUS_MSG_WR].len = data_len + 2;
>  			for (int i = 0; i <= data_len; i++)
> -- 
> 2.34.1
> 

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