[PATCH ethtool 0/2] harden module EEPROM length handling

[PATCH ethtool 0/2] harden module EEPROM length handling

[Prabhakar Pujeri]

These patches keep module EEPROM reads within the lengths reported by the
kernel.  The netlink path now limits its copy to the received attribute,
and the ioctl path validates offset and length arithmetic before allocating
its response buffer.

Tested with make check in the default and --disable-netlink builds.

Prabhakar Pujeri (2):
  netlink: module-eeprom: clamp data length to attribute payload size
  ethtool: validate module EEPROM offset and length

 ethtool.c               | 9 ++++++++-
 netlink/module-eeprom.c | 8 ++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.55.0

[PATCH ethtool 1/2] netlink: module-eeprom: clamp data length to attribute payload size

[Prabhakar Pujeri]

get_eeprom_page_reply_cb() memcpy()'s request->length bytes out of the
ETHTOOL_A_MODULE_EEPROM_DATA attribute without checking how many bytes
the kernel actually put into it. If the reply carries fewer bytes than
requested, the copy reads past the end of the message payload.

Use mnl_attr_get_payload_len() to learn the real size of the data
attribute, like other parsers in the tree (e.g. netlink/rss.c,
netlink/fec.c), and clamp request->length to it before allocating and
copying the data.

Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
---
 netlink/module-eeprom.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 40882ca..7e2af4e 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -133,6 +133,7 @@ static int get_eeprom_page_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	struct ethtool_module_eeprom *request = data;
 	DECLARE_ATTR_TB_INFO(tb);
 	u8 *eeprom_data;
+	u32 data_len;
 	int ret;
 
 	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
@@ -142,6 +143,13 @@ static int get_eeprom_page_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	if (!tb[ETHTOOL_A_MODULE_EEPROM_DATA])
 		return MNL_CB_ERROR;
 
+	/* The kernel may return fewer bytes than requested; never read
+	 * beyond the data actually present in the attribute.
+	 */
+	data_len = mnl_attr_get_payload_len(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
+	if (data_len < request->length)
+		request->length = data_len;
+
 	eeprom_data = mnl_attr_get_payload(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
 	request->data = malloc(request->length);
 	if (!request->data)
-- 
2.55.0

[PATCH ethtool 2/2] ethtool: validate module EEPROM offset and length

[Prabhakar Pujeri]

do_getmodule() clamps a requested range by adding its offset and length.
An offset beyond the EEPROM makes the later subtraction underflow, while
a very large length can make the addition wrap.  Either case can produce
an unexpectedly large allocation request.

Reject offsets at or beyond the reported EEPROM size.  Once the offset is
known to be valid, compare the requested length with the remaining size
instead of adding the two user-controlled values.

Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
---
 ethtool.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ethtool.c b/ethtool.c
index 3076c12..fa0f729 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5051,10 +5051,17 @@ static int do_getmodule(struct cmd_context *ctx)
 		return 1;
 	}
 
+	if (geeprom_offset >= modinfo.eeprom_len) {
+		fprintf(stderr,
+			"Invalid offset %u: module EEPROM is %u bytes\n",
+			geeprom_offset, modinfo.eeprom_len);
+		return 1;
+	}
+
 	if (!geeprom_length_seen)
 		geeprom_length = modinfo.eeprom_len;
 
-	if (modinfo.eeprom_len < geeprom_offset + geeprom_length)
+	if (geeprom_length > modinfo.eeprom_len - geeprom_offset)
 		geeprom_length = modinfo.eeprom_len - geeprom_offset;
 
 	eeprom = calloc(1, sizeof(*eeprom)+geeprom_length);
-- 
2.55.0