| 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
| ||
|
Message-ID: <aa1237d1-315b-8233-72a8-95e7afd033ee@nvidia.com>
Date: Mon, 8 Mar 2021 11:04:52 +0200
From: Moshe Shemesh <moshe@...dia.com>
To: Don Bollinger <don@...bollingers.org>,
"'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>
Subject: Re: [RFC PATCH V2 net-next 5/5] ethtool: Add fallback to
get_module_eeprom from netlink command
On 3/5/2021 2:50 AM, Don Bollinger wrote:
>
> On Thu, Mar 04, 2021 at 10:57AM-0800, Moshe Shemesh wrote:
>> From: Vladyslav Tarasiuk <vladyslavt@...dia.com>
>>
>> In case netlink get_module_eeprom_data_by_page() callback is not
>> implemented by the driver, try to call old get_module_info() and
>> get_module_eeprom() pair. Recalculate parameters to
>> get_module_eeprom() offset and len using page number and their sizes.
>> Return error if this can't be done.
>>
>> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@...dia.com>
>> ---
>> net/ethtool/eeprom.c | 84
>> +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ethtool/eeprom.c b/net/ethtool/eeprom.c index
>> 2618a55b9a40..72c7714a9d37 100644
>> --- 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;
> if (offset > 127) offset = request->page * 128 + offset;
Yes, if we got page, that's the new API.
>> +
>> + if (!length)
>> + length = modinfo->eeprom_len;
>> +
>> + if (offset >= modinfo->eeprom_len)
>> + return -EINVAL;
>> +
>> + if (modinfo->eeprom_len < offset + length)
>> + length = modinfo->eeprom_len - offset;
>> +
>> + eeprom->cmd = ETHTOOL_GMODULEEEPROM;
>> + eeprom->len = length;
>> + eeprom->offset = offset;
>> +
>> + switch (modinfo->type) {
>> + case ETH_MODULE_SFF_8079:
>> + if (request->page > 1)
>> + return -EINVAL;
>> + break;
>> + case ETH_MODULE_SFF_8472:
>> + if (request->page > 3)
> Not sure this is needed, there can be pages higher than 3.
>
>> + return -EINVAL;
> I *think* the linear memory on SFP puts 0x50 in the first
> 256 bytes, 0x51 after that, including pages after that. So,
> the old fashioned linear memory offset needs to be adjusted
> for accesses to 0x51. Thus add:
>
> if (request->i2c_address == 0x51)
> offset += 256;
Will check that. In the old KAPI the i2c address is not a parameter, so
it depends on driver implementation.
>> + break;
>> + case ETH_MODULE_SFF_8436:
>> + case ETH_MODULE_SFF_8636:
> Not sure this is needed, there can be pages higher than 3.
>
>> + if (request->page > 3)
>> + return -EINVAL;
>> + break;
>> + }
>> + return 0;
>> +}
>> +
>> +static int eeprom_data_fallback(struct eeprom_data_req_info *request,
>> + struct eeprom_data_reply_data *reply,
>> + struct genl_info *info)
>> +{
>> + struct net_device *dev = reply->base.dev;
>> + struct ethtool_modinfo modinfo = {0};
>> + struct ethtool_eeprom eeprom = {0};
>> + u8 *data;
>> + int err;
>> +
>> + if ((!dev->ethtool_ops->get_module_info &&
>> + !dev->ethtool_ops->get_module_eeprom) ||
>> + request->bank || request->i2c_address) {
> We don't need to reject if there is an i2c_address. Indeed, we need that
> to determine the correct offset for the legacy linear memory offset.
Will check that. As Andrew said, there might be usage of other i2c
addresses with old KAPI.
> Note my comment on an earlier patch in this series, I would have rejected
> any request that didn't have either 0x50 or 0x51 here.
>
>> + return -EOPNOTSUPP;
>> + }
>> + modinfo.cmd = ETHTOOL_GMODULEINFO;
>> + err = dev->ethtool_ops->get_module_info(dev, &modinfo);
>> + if (err < 0)
>> + return err;
>> +
>> + err = fallback_set_params(request, &modinfo, &eeprom);
>> + if (err < 0)
>> + return err;
>> +
>> + data = kmalloc(eeprom.len, GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> + err = dev->ethtool_ops->get_module_eeprom(dev, &eeprom,
>> data);
>> + if (err < 0)
>> + goto err_out;
>> +
>> + reply->data = data;
>> + reply->length = eeprom.len;
>> +
>> + return 0;
>> +
>> +err_out:
>> + kfree(data);
>> + return err;
>> +}
>> +
>> static int eeprom_data_prepare_data(const struct ethnl_req_info
>> *req_base,
>> struct ethnl_reply_data *reply_base,
>> struct genl_info *info)
>> @@ -37,7 +119,7 @@ static int eeprom_data_prepare_data(const struct
>> ethnl_req_info *req_base,
>> int err;
>>
>> if (!dev->ethtool_ops->get_module_eeprom_data_by_page)
>> - return -EOPNOTSUPP;
>> + return eeprom_data_fallback(request, reply, info);
>>
>> page_data.offset = request->offset;
>> page_data.length = request->length;
>> --
>> 2.18.2
>
Powered by blists - more mailing lists