netdev - Re: [RFC PATCH V4 net-next 1/5] ethtool: Allow network drivers to dump arbitrary EEPROM data

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]

Date:   Wed, 24 Mar 2021 12:03:32 +0200
From:   Moshe Shemesh <moshe@...dia.com>
To:     Andrew Lunn <andrew@...n.ch>, Don Bollinger <don@...bollingers.org>
CC:     "'David S. Miller'" <davem@...emloft.net>,
        'Jakub Kicinski' <kuba@...nel.org>,
        'Adrian Pop' <pop.adrian61@...il.com>,
        'Michal Kubecek' <mkubecek@...e.cz>, <netdev@...r.kernel.org>,
        'Vladyslav Tarasiuk' <vladyslavt@...dia.com>
Subject: Re: [RFC PATCH V4 net-next 1/5] ethtool: Allow network drivers to
 dump arbitrary EEPROM data


On 3/23/2021 2:27 AM, Andrew Lunn wrote:
>
>>> +#define ETH_MODULE_EEPROM_PAGE_LEN 256
>> Sorry to keep raising issues, but I think you want to make this constant
>> 128.
> Yes, i also think the KAPI should be limited to returning a maximum of
> a 1/2 page per call.
OK.
>>> +#define MODULE_EEPROM_MAX_OFFSET (257 *
>>> ETH_MODULE_EEPROM_PAGE_LEN)
>> The device actually has 257 addressable chunks of 128 bytes each.  With
>> ETH_MODULE_EEPROM_PAGE_LEN set to 256, your constant is 2X too big.
>>
>> Note also, SFP devices (but not QSFP or CMIS) actually have another 256
>> bytes available at 0x50, in addition to the full 257*128 at 0x51.  So SFP is
>> actually 259*128 or (256 + 257 * 128).
>>
>> Devices that don't support pages have much lower limits (256 bytes for
>> QSFP/CMIS and 512 for SFP).  Some SFP only support 256 bytes.  Most devices
>> will return nonsense for pages above 3.  So, this check is really only an
>> absolute limit.  The SFP driver that takes this request will probably check
>> against a more refined MAX length (eg modinfo->eeprom_len).
>>
>> I suggest setting this constant to 259 * (ETH_MODULE_EEPROM_PAGE_LEN / 2).
>> Let the driver refine it from there.
> I don't even see a need for this. The offset should be within one 1/2
> page, of one bank. So offset >= 0 and <= 127. Length is also > 0 and
> <- 127. And offset+length is <= 127.
>
> For the moment, please forget about backwards compatibility with the
> IOCTL interface. Lets get a new clean KAPI and a new clean internal
> API between the ethtool core and the drivers. Once we have that agreed
> on, we can work on the various compatibility shims we need to work
> between old and new APIs in various places.


OK, so following the comments here, I will ignore backward compatibility 
of having global offset and length.

That means I assume in this KAPI I am asked to get data from specific 
page. Should I also require user space to send page number to this KAPI 
(I mean make page number mandatory) ?

>        Andrew