ethtool: make .get_dump_data() harder to misuse by drivers (c590b5e2) · Commits · 戴 / test

As the patch "bnx2x: remove zeroing of dump data buffer" showed, it is too easy implement .get_dump_data incorrectly in a driver. Let's make sure drivers cannot get confused by userspace requesting a too big dump. Also WARN if the driver sets dump->len to something weird and make sure the length reported to userspace is the actual length of data copied to userspace. Signed-off-by:

Michal Schmidt <mschmidt@redhat.com> Reviewed-by:

Ben Hutchings <ben@decadent.org.uk> Signed-off-by:

David S. Miller <davem@davemloft.net>

net/core/ethtool.c

+20 −1

Original line number	Diff line number	Diff line
		@@ -1320,10 +1320,19 @@ static int ethtool_get_dump_data(struct net_device *dev,
		if (ret)
		return ret;

		len = (tmp.len > dump.len) ? dump.len : tmp.len;
		len = min(tmp.len, dump.len);
		if (!len)
		return -EFAULT;

		/* Don't ever let the driver think there's more space available
		* than it requested with .get_dump_flag().
		*/
		dump.len = len;

		/* Always allocate enough space to hold the whole thing so that the
		* driver does not need to check the length and bother with partial
		* dumping.
		*/
		data = vzalloc(tmp.len);
		if (!data)
		return -ENOMEM;
		@@ -1331,6 +1340,16 @@ static int ethtool_get_dump_data(struct net_device *dev,
		if (ret)
		goto out;

		/* There are two sane possibilities:
		* 1. The driver's .get_dump_data() does not touch dump.len.
		* 2. Or it may set dump.len to how much it really writes, which
		* should be tmp.len (or len if it can do a partial dump).
		* In any case respond to userspace with the actual length of data
		* it's receiving.
		*/
		WARN_ON(dump.len != len && dump.len != tmp.len);
		dump.len = len;

		if (copy_to_user(useraddr, &dump, sizeof(dump))) {
		ret = -EFAULT;
		goto out;

Admin message