From 78c57f22e3c87ab0a2844d7c9a120eba51ae34f4 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:46 +0300 Subject: [PATCH 1/7] ethtool: Use correct command name in title The command is called 'ETHTOOL_MSG_MODULE_EEPROM_GET', not 'ETHTOOL_MSG_MODULE_EEPROM'. Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- Documentation/networking/ethtool-netlink.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index 25131df3c2bd..c3600f9c8988 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -1363,8 +1363,8 @@ in an implementation specific way. ``ETHTOOL_A_FEC_AUTO`` requests the driver to choose FEC mode based on SFP module parameters. This does not mean autonegotiation. -MODULE_EEPROM -============= +MODULE_EEPROM_GET +================= Fetch module EEPROM data dump. This interface is designed to allow dumps of at most 1/2 page at once. This From 913d026fbfaf114ff87afcc77fa4e9309f87f114 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:47 +0300 Subject: [PATCH 2/7] ethtool: Document correct attribute type 'ETHTOOL_A_MODULE_EEPROM_DATA' is a binary attribute, not a nested one. Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- Documentation/networking/ethtool-netlink.rst | 2 +- include/uapi/linux/ethtool_netlink.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index c3600f9c8988..8ae644f800f0 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -1388,7 +1388,7 @@ Kernel response contents: +---------------------------------------------+--------+---------------------+ | ``ETHTOOL_A_MODULE_EEPROM_HEADER`` | nested | reply header | +---------------------------------------------+--------+---------------------+ - | ``ETHTOOL_A_MODULE_EEPROM_DATA`` | nested | array of bytes from | + | ``ETHTOOL_A_MODULE_EEPROM_DATA`` | binary | array of bytes from | | | | module EEPROM | +---------------------------------------------+--------+---------------------+ diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index 825cfda1c5d5..c7135c9c37a5 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -675,7 +675,7 @@ enum { ETHTOOL_A_MODULE_EEPROM_PAGE, /* u8 */ ETHTOOL_A_MODULE_EEPROM_BANK, /* u8 */ ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS, /* u8 */ - ETHTOOL_A_MODULE_EEPROM_DATA, /* nested */ + ETHTOOL_A_MODULE_EEPROM_DATA, /* binary */ __ETHTOOL_A_MODULE_EEPROM_CNT, ETHTOOL_A_MODULE_EEPROM_MAX = (__ETHTOOL_A_MODULE_EEPROM_CNT - 1) From f5fe211d13af52077bb66e89a5410fa75f691fe8 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:48 +0300 Subject: [PATCH 3/7] ethtool: Decrease size of module EEPROM get policy array The 'ETHTOOL_A_MODULE_EEPROM_DATA' attribute is not part of the get request. Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- net/ethtool/netlink.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index 90b10966b16b..3e25a47fd482 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -380,7 +380,7 @@ extern const struct nla_policy ethnl_cable_test_tdr_act_policy[ETHTOOL_A_CABLE_T extern const struct nla_policy ethnl_tunnel_info_get_policy[ETHTOOL_A_TUNNEL_INFO_HEADER + 1]; extern const struct nla_policy ethnl_fec_get_policy[ETHTOOL_A_FEC_HEADER + 1]; extern const struct nla_policy ethnl_fec_set_policy[ETHTOOL_A_FEC_AUTO + 1]; -extern const struct nla_policy ethnl_module_eeprom_get_policy[ETHTOOL_A_MODULE_EEPROM_DATA + 1]; +extern const struct nla_policy ethnl_module_eeprom_get_policy[ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS + 1]; extern const struct nla_policy ethnl_stats_get_policy[ETHTOOL_A_STATS_GROUPS + 1]; int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info); From 37a025e83902903df658489665499a548a53423b Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:49 +0300 Subject: [PATCH 4/7] ethtool: Document behavior when module EEPROM bank attribute is omitted The kernel assumes bank 0 when 'ETHTOOL_MSG_MODULE_EEPROM_GET' is sent without 'ETHTOOL_A_MODULE_EEPROM_BANK'. Document it as part of the interface documentation. Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- Documentation/networking/ethtool-netlink.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index 8ae644f800f0..6ea91e41593f 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -1383,6 +1383,8 @@ Request contents: ``ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS`` u8 page I2C address ======================================= ====== ========================== +If ``ETHTOOL_A_MODULE_EEPROM_BANK`` is not specified, bank 0 is assumed. + Kernel response contents: +---------------------------------------------+--------+---------------------+ From b8c48be23c2d03834fe01c3ea757d9df8b97013d Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:50 +0300 Subject: [PATCH 5/7] ethtool: Use kernel data types for internal EEPROM struct The struct is not visible to user space and therefore should not use the user visible data types. Instead, use internal data types like other structures in the file. Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- include/linux/ethtool.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index e030f7510cd3..29dbb603bc91 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -401,12 +401,12 @@ struct ethtool_rmon_stats { * required information to the driver. */ struct ethtool_module_eeprom { - __u32 offset; - __u32 length; - __u8 page; - __u8 bank; - __u8 i2c_address; - __u8 *data; + u32 offset; + u32 length; + u8 page; + u8 bank; + u8 i2c_address; + u8 *data; }; /** From 0dc7dd02ba7ab5f623f5e3a36443ec441364285a Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:51 +0300 Subject: [PATCH 6/7] ethtool: Validate module EEPROM length as part of policy Validate the number of bytes to read from the module EEPROM as part of the netlink policy and remove the corresponding check from the code. This also makes it possible to query the length range from user space: $ genl ctrl policy name ethtool ... ID: 0x14 policy[32]:attr[3]: type=U32 range:[1,128] ... Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- net/ethtool/eeprom.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/net/ethtool/eeprom.c b/net/ethtool/eeprom.c index 5d38e90895ac..1e75d9c1b154 100644 --- a/net/ethtool/eeprom.c +++ b/net/ethtool/eeprom.c @@ -159,9 +159,6 @@ static int eeprom_parse_request(struct ethnl_req_info *req_info, struct nlattr * request->offset = nla_get_u32(tb[ETHTOOL_A_MODULE_EEPROM_OFFSET]); request->length = nla_get_u32(tb[ETHTOOL_A_MODULE_EEPROM_LENGTH]); - if (!request->length) - return -EINVAL; - /* The following set of conditions limit the API to only dump 1/2 * EEPROM page without crossing low page boundary located at offset 128. * This means user may only request dumps of length limited to 128 from @@ -237,7 +234,8 @@ const struct ethnl_request_ops ethnl_module_eeprom_request_ops = { const struct nla_policy ethnl_module_eeprom_get_policy[] = { [ETHTOOL_A_MODULE_EEPROM_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy), [ETHTOOL_A_MODULE_EEPROM_OFFSET] = { .type = NLA_U32 }, - [ETHTOOL_A_MODULE_EEPROM_LENGTH] = { .type = NLA_U32 }, + [ETHTOOL_A_MODULE_EEPROM_LENGTH] = + NLA_POLICY_RANGE(NLA_U32, 1, ETH_MODULE_EEPROM_PAGE_LEN), [ETHTOOL_A_MODULE_EEPROM_PAGE] = { .type = NLA_U8 }, [ETHTOOL_A_MODULE_EEPROM_BANK] = { .type = NLA_U8 }, [ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS] = From 88f9a87afeeec5dfdda3651f3db96d0006172d91 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Tue, 22 Jun 2021 09:50:52 +0300 Subject: [PATCH 7/7] ethtool: Validate module EEPROM offset as part of policy Validate the offset to read from module EEPROM as part of the netlink policy and remove the corresponding check from the code. This also makes it possible to query the offset range from user space: $ genl ctrl policy name ethtool ... ID: 0x14 policy[32]:attr[2]: type=U32 range:[0,255] ... Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- net/ethtool/eeprom.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/net/ethtool/eeprom.c b/net/ethtool/eeprom.c index 1e75d9c1b154..7e6b37a54add 100644 --- a/net/ethtool/eeprom.c +++ b/net/ethtool/eeprom.c @@ -177,10 +177,6 @@ static int eeprom_parse_request(struct ethnl_req_info *req_info, struct nlattr * NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_MODULE_EEPROM_LENGTH], "reading cross half page boundary is illegal"); return -EINVAL; - } else if (request->offset >= ETH_MODULE_EEPROM_PAGE_LEN * 2) { - NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_MODULE_EEPROM_OFFSET], - "offset is out of bounds"); - return -EINVAL; } else if (request->offset + request->length > ETH_MODULE_EEPROM_PAGE_LEN * 2) { NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_MODULE_EEPROM_LENGTH], "reading cross page boundary is illegal"); @@ -233,7 +229,8 @@ const struct ethnl_request_ops ethnl_module_eeprom_request_ops = { const struct nla_policy ethnl_module_eeprom_get_policy[] = { [ETHTOOL_A_MODULE_EEPROM_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy), - [ETHTOOL_A_MODULE_EEPROM_OFFSET] = { .type = NLA_U32 }, + [ETHTOOL_A_MODULE_EEPROM_OFFSET] = + NLA_POLICY_MAX(NLA_U32, ETH_MODULE_EEPROM_PAGE_LEN * 2 - 1), [ETHTOOL_A_MODULE_EEPROM_LENGTH] = NLA_POLICY_RANGE(NLA_U32, 1, ETH_MODULE_EEPROM_PAGE_LEN), [ETHTOOL_A_MODULE_EEPROM_PAGE] = { .type = NLA_U8 },