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: <6615e8fc-9476-3d91-eabc-aae91a5d7898@nvidia.com>
Date:   Wed, 24 Mar 2021 12:05:57 +0200
From:   Moshe Shemesh <moshe@...dia.com>
To:     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>,
        "Don Bollinger" <don@...bollingers.org>, <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:54 AM, Andrew Lunn wrote:
>
>> +static int eeprom_prepare_data(const struct ethnl_req_info *req_base,
>> +                            struct ethnl_reply_data *reply_base,
>> +                            struct genl_info *info)
>> +{
>> +     struct eeprom_reply_data *reply = MODULE_EEPROM_REPDATA(reply_base);
>> +     struct eeprom_req_info *request = MODULE_EEPROM_REQINFO(req_base);
>> +     struct ethtool_module_eeprom page_data = {0};
>> +     struct net_device *dev = reply_base->dev;
>> +     int ret;
>> +
>> +     if (!dev->ethtool_ops->get_module_eeprom_by_page)
>> +             return -EOPNOTSUPP;
>> +
>> +     /* Allow dumps either of low or high page without crossing half page boundary */
>> +     if ((request->offset < ETH_MODULE_EEPROM_PAGE_LEN / 2 &&
>> +          request->offset + request->length > ETH_MODULE_EEPROM_PAGE_LEN / 2) ||
>> +         request->offset + request->length > ETH_MODULE_EEPROM_PAGE_LEN)
>> +             return -EINVAL;
> Please keep all the parameter validation together, in
> eeprom_parse_request(). At some point, we may extend
> eeprom_parse_request() to make use of extack, to indicate which
> parameter is invalid. Just getting an -EINVAL can be hard to debug,
> where as NL_SET_ERR_MSG_ATTR() can help the user.
Sure, we can add that.
>        Andrew