linux-kernel - Re: [PATCH] net: sfp: add SMBus I2C block support

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: <29295127-e54f-4954-8a63-03289c113a1f@bootlin.com>
Date: Mon, 5 Jan 2026 09:29:41 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Jakub Kicinski <kuba@...nel.org>, Jonas Jelonek <jelonek.jonas@...il.com>
Cc: Russell King <linux@...linux.org.uk>, Andrew Lunn <andrew@...n.ch>,
 Heiner Kallweit <hkallweit1@...il.com>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Bjørn Mork <bjorn@...k.no>
Subject: Re: [PATCH] net: sfp: add SMBus I2C block support

Hi folks,

On 04/01/2026 17:05, Jakub Kicinski wrote:
> On Sun, 28 Dec 2025 21:33:31 +0000 Jonas Jelonek wrote:
>> +static int sfp_smbus_block_write(struct sfp *sfp, bool a2, u8 dev_addr,
>> +				 void *buf, size_t len)
>> +{
>> +	size_t block_size = sfp->i2c_block_size;
>> +	union i2c_smbus_data smbus_data;
>> +	u8 bus_addr = a2 ? 0x51 : 0x50;
>> +	u8 *data = buf;
>> +	u8 this_len;
>> +	int ret;
>> +
>> +	while (len) {
>> +		this_len = min(len, block_size);
>> +
>> +		smbus_data.block[0] = this_len;
>> +		memcpy(&smbus_data.block[1], data, this_len);
>> +		ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
>> +				     I2C_SMBUS_WRITE, dev_addr,
>> +				     I2C_SMBUS_I2C_BLOCK_DATA, &smbus_data);
>> +		if (ret)
>> +			return ret;
>> +
>> +		len -= this_len;
>> +		data += this_len;
>> +		dev_addr += this_len;
>> +	}
>> +
>> +	return 0;
>> +}
> 
> AI code review says:
> 
>  Should this return the number of bytes written instead of 0?
> 
>  The existing sfp_i2c_write() returns the byte count on success, and several
>  callers depend on this return value:
> 
>  sfp_cotsworks_fixup_check() checks:
>     err = sfp_write(sfp, false, SFP_PHYS_ID, &id->base, 3);
>     if (err != 3) { ... error path ... }
> 
>  sfp_sm_mod_hpower() via sfp_modify_u8() checks:
>     if (err != sizeof(u8)) { ... error path ... }
> 
>  With this function returning 0 on success, these checks will always fail,
>  causing high-power SFP modules to fail initialization with "failed to enable
>  high power" errors, and Cotsworks module EEPROM fixups to fail with "Failed
>  to rewrite module EEPROM" errors.
> 
> Either way, you'll need to repost, net-next was closed when you posted.

Looks like I made the same mistake in sfp_smbus_byte_write(). I'll send
a fix for that/

Maxime