netdev - RE: [net-next 07/15] ixgbe: fix SFF data dumps of SFP+ modules

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: <87618083B2453E4A8714035B62D679924FDEDF40@FMSMSX105.amr.corp.intel.com>
Date:	Tue, 6 Aug 2013 00:56:10 +0000
From:	"Tantilov, Emil S" <emil.s.tantilov@...el.com>
To:	Ben Hutchings <bhutchings@...arflare.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [net-next 07/15] ixgbe: fix SFF data dumps of SFP+ modules

>-----Original Message-----
>From: netdev-owner@...r.kernel.org [mailto:netdev-
>owner@...r.kernel.org] On Behalf Of Ben Hutchings
>Sent: Friday, August 02, 2013 2:09 AM
>To: Kirsher, Jeffrey T
>Cc: davem@...emloft.net; Tantilov, Emil S;
>netdev@...r.kernel.org; gospo@...hat.com;
>sassmann@...hat.com
>Subject: Re: [net-next 07/15] ixgbe: fix SFF data dumps of
>SFP+ modules
>
>On Mon, 2013-07-29 at 05:52 -0700, Jeff Kirsher wrote:
>> From: Emil Tantilov <emil.s.tantilov@...el.com>
>>
>> This patch fixes several issues with the previous
>implementation of the
>> SFF data dump of SFP+ modules:
>>
>> - removed the __IXGBE_READ_I2C flag - I2C access locking
>is handled in the
>>   HW specific routines
>>
>> - fixed the read loop to read data from ee->offset to ee-
>>len
>>
>> - the reads fail if __IXGBE_IN_SFP_INIT is set in the
>process - this is
>>   needed because on some HW I2C operations can take long
>time and disrupt
>>   the SFP and link detection process
>>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
>> Reported-by: Ben Hutchings <bhutchings@...arflare.com>
>> Tested-by: Phil Schmitt <phillip.j.schmitt@...el.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
>[...]
>> @@ -2969,48 +2955,25 @@ static int
>ixgbe_get_module_eeprom(struct net_device *dev,
>>  	int i = 0;
>>  	int ret_val = 0;
>>
>> -	/* ixgbe_get_module_info is called before this function
>in all
>> -	 * cases, so we do not need any checks we already do
>above,
>> -	 * and can trust ee->len to be a known value.
>> -	 */
>> +	if (ee->len == 0)
>> +		return -EINVAL;
>>
>> -	while (test_bit(__IXGBE_IN_SFP_INIT, &adapter->state))
>> -		msleep(100);
>> -	set_bit(__IXGBE_READ_I2C, &adapter->state);
>> +	for (i = ee->offset; i < ee->len; i++) {
>
>                               i < ee->offset + ee->len
>
>> +		/* I2C reads can take long time */
>> +		if (test_bit(__IXGBE_IN_SFP_INIT, &adapter-
>>state))
>> +			return -EBUSY;
>>
>> -	/* Read the first block, SFF-8079 */
>> -	for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i++) {
>> -		status = hw->phy.ops.read_i2c_eeprom(hw, i,
>&databyte);
>> -		if (status != 0) {
>> -			/* Error occured while reading module */
>> +		if (i < ETH_MODULE_SFF_8079_LEN)
>> +			status  = hw->phy.ops.read_i2c_eeprom(hw, i,
>&databyte);
>> +		else
>> +			status = hw->phy.ops.read_i2c_sff8472(hw, i,
>&databyte);
>[...]
>                                                     i -
>ETH_MODULE_SFF_8079_LEN ?
>
>But this works anyway because the address is truncated to
>u8.

Actually as is the loop will only work when offset is 0, so it looks like it should be:

for (i = ee->offset; i < ee->offset + ee->len; i++) {


Unless you had something else in mind.

I will submit a fix for this.

Thanks,
Emil

>
>Ben.