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]

Message-ID: <811a3af6-84f7-6f44-68c3-eba8be06e3f4@nvidia.com>
Date:   Wed, 24 Mar 2021 12:14:43 +0200
From:   Moshe Shemesh <moshe@...dia.com>
To:     Don Bollinger <don@...bollingers.org>,
        'Andrew Lunn' <andrew@...n.ch>
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 7:47 PM, Don Bollinger wrote:
>>>> 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.
>>> I like the clean approach, but...   How do you request low memory?
>> Duh!
>>
>> I got my conditions wrong. Too focused on 1/2 pages to think that two of
>> them makes one page!
>>
>> Lets try again:
>>
>> offset < 256
>> 0 < len < 128
> Actually 0 < len <= 128.  Length of 128 is not only legal, but very common.
> "Read the whole 1/2 page block".
Ack.
>> if (offset < 128)
>>     offset + len < 128
> Again, offset + len <= 128
>
>> else
>>     offset + len < 256
> offset + len <= 256
Ack.
>> Does that look better?
>>
>> Reading bytes from the lower 1/2 of page 0 should give the same data as
>> reading data from the lower 1/2 of page 42. So we can allow that, but
> don't
>> be too surprised when an SFP gets it wrong and gives you rubbish. I would
> The spec is clear that the lower half is the same for all pages.  If the SFP
> gives you rubbish you should throw the device in the rubbish.
>
>> suggest ethtool(1) never actually does read from the lower 1/2 of any page
>> other than 0.
> I agree, despite my previous comment.  While the spec is clear that should
> work, I believe virtually all such instances are bugs not yet discovered.


Agreed, so we will accept offset < 128 only on page zero.

> And, note that the legacy API provides no way to access lower memory from
> any page but 0.  There's just no syntax for it.  Not that we care about
> legacy :-).
>
>> And i agree about documentation. I would suggest a comment in
>> ethtool_netlink.h, and the RST documentation.


Ack, will comment there on limiting new KAPI only to half pages reading.

We may address reading cross pages by user space (implementing multiple 
calls to KAPI to get such data).

>>                   Andrew
> Don
>
>