netdev - RE: [RFC PATCH V2 net-next 5/5] ethtool: Add fallback to get_module

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: <001b01d7116a$cb64b7a0$622e26e0$@thebollingers.org>
Date:   Thu, 4 Mar 2021 18:53:58 -0800
From:   "Don Bollinger" <don@...bollingers.org>
To:     "'Moshe Shemesh'" <moshe@...dia.com>,
        "'David S. Miller'" <davem@...emloft.net>,
        "'Jakub Kicinski'" <kuba@...nel.org>,
        "'Andrew Lunn'" <andrew@...n.ch>,
        "'Adrian Pop'" <pop.adrian61@...il.com>,
        "'Michal Kubecek'" <mkubecek@...e.cz>, <netdev@...r.kernel.org>
Cc:     "'Vladyslav Tarasiuk'" <vladyslavt@...dia.com>,
        <don@...bollingers.org>
Subject: RE: [RFC PATCH V2 net-next 5/5] ethtool: Add fallback to get_module_eeprom from netlink command

> > --- a/net/ethtool/eeprom.c
> > +++ b/net/ethtool/eeprom.c
> > @@ -26,6 +26,88 @@ struct eeprom_data_reply_data {  #define
> > EEPROM_DATA_REPDATA(__reply_base) \
> >  	container_of(__reply_base, struct eeprom_data_reply_data, base)
> >
> > +static int fallback_set_params(struct eeprom_data_req_info *request,
> > +			       struct ethtool_modinfo *modinfo,
> > +			       struct ethtool_eeprom *eeprom) {
> 
> This is translating the new data structure into the old.  Hence, I assume
we
> have i2c_addr, page, bank, offset, len to work with, and we should use all
of
> them.  We shouldn't be applying the legacy data structure's rules to how
we
> interpret the *request data.  Therefore...
> 
> > +	u32 offset = request->offset;
> > +	u32 length = request->length;
> > +
> > +	if (request->page)
> > +		offset = 128 + request->page * 128 + offset;
> 
> This is tricky to map to old behavior.  The new data structure should give
> lower memory for offsets less than 128, and paged upper memory for
> offsets of 128 and higher.  There is no way to describe that request as
> {offset, length} in the old ethtool format with a fake linear memory.
> 
>         if (request->page) {
>                 if (offset < 128) && (offset + length > 128)
>                        return -EINVAL;

Actually, reflecting on Andrew's response, it occurs to me this does not
have to be an error.  The routine eeprom_data_fallback() (below) could
detect this case (a request crossing the 128 byte offset boundary) and
create two requests, one for lower memory and one for the paged 
upper memory.  That can't be done as a single request with the linear
memory model, but the two pieces can be read separately and glued
together.

Don