From fb95cd8d1473b1cc90eccbd6a30641f3851c8506 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 15 May 2014 00:46:45 +0100 Subject: ethtool: Return immediately on error in ethtool_copy_validate_indir() We must return -EFAULT immediately rather than continuing into the loop. Similarly, we may as well return -EINVAL directly. Signed-off-by: Ben Hutchings --- net/core/ethtool.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'net/core') diff --git a/net/core/ethtool.c b/net/core/ethtool.c index aa8978ac..c834cb2 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -561,19 +561,17 @@ static int ethtool_copy_validate_indir(u32 *indir, void __user *useraddr, struct ethtool_rxnfc *rx_rings, u32 size) { - int ret = 0, i; + int i; if (copy_from_user(indir, useraddr, size * sizeof(indir[0]))) - ret = -EFAULT; + return -EFAULT; /* Validate ring indices */ - for (i = 0; i < size; i++) { - if (indir[i] >= rx_rings->data) { - ret = -EINVAL; - break; - } - } - return ret; + for (i = 0; i < size; i++) + if (indir[i] >= rx_rings->data) + return -EINVAL; + + return 0; } static noinline_for_stack int ethtool_get_rxfh_indir(struct net_device *dev, -- cgit v1.1 From 7455fa2422898eee3464032351d20695930d9542 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 15 May 2014 01:41:23 +0100 Subject: ethtool: Name the 'no change' value for setting RSS hash key but not indir table We usually allocate special values of u32 fields starting from the top down, so also change the value to 0xffffffff. As these operations haven't been included in a stable release yet, it's not too late to change. Signed-off-by: Ben Hutchings --- net/core/ethtool.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'net/core') diff --git a/net/core/ethtool.c b/net/core/ethtool.c index c834cb2..7156fe5 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -803,12 +803,13 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, /* If either indir or hash key is valid, proceed further. */ - if ((user_indir_size && ((user_indir_size != 0xDEADBEEF) && - user_indir_size != dev_indir_size)) || + if ((user_indir_size && + user_indir_size != ETH_RXFH_INDIR_NO_CHANGE && + user_indir_size != dev_indir_size) || (user_key_size && (user_key_size != dev_key_size))) return -EINVAL; - if (user_indir_size != 0xDEADBEEF) + if (user_indir_size != ETH_RXFH_INDIR_NO_CHANGE) indir_bytes = dev_indir_size * sizeof(indir[0]); rss_config = kzalloc(indir_bytes + user_key_size, GFP_USER); @@ -821,9 +822,10 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, goto out; /* user_indir_size == 0 means reset the indir table to default. - * user_indir_size == 0xDEADBEEF means indir setting is not requested. + * user_indir_size == ETH_RXFH_INDIR_NO_CHANGE means leave it unchanged. */ - if (user_indir_size && user_indir_size != 0xDEADBEEF) { + if (user_indir_size && + user_indir_size != ETH_RXFH_INDIR_NO_CHANGE) { indir = (u32 *)rss_config; ret = ethtool_copy_validate_indir(indir, useraddr + rss_cfg_offset, -- cgit v1.1 From 61d88c6811f216de4ec26aafe24e650dc1aeb00e Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 19 May 2014 01:29:42 +0100 Subject: ethtool: Disallow ETHTOOL_SRSSH with both indir table and hash key unchanged This would be a no-op, so there is no reason to request it. This also allows conversion of the current implementations of ethtool_ops::{get,set}_rxfh_indir to ethtool_ops::{get,set}_rxfh with no change other than their parameters. Signed-off-by: Ben Hutchings --- net/core/ethtool.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'net/core') diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 7156fe5..b885734 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -802,11 +802,14 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, return -EFAULT; /* If either indir or hash key is valid, proceed further. + * It is not valid to request that both be unchanged. */ if ((user_indir_size && user_indir_size != ETH_RXFH_INDIR_NO_CHANGE && user_indir_size != dev_indir_size) || - (user_key_size && (user_key_size != dev_key_size))) + (user_key_size && (user_key_size != dev_key_size)) || + (user_indir_size == ETH_RXFH_INDIR_NO_CHANGE && + user_key_size == 0)) return -EINVAL; if (user_indir_size != ETH_RXFH_INDIR_NO_CHANGE) -- cgit v1.1 From fe62d001372388abb15a324148c913f9b43722a8 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 15 May 2014 01:25:27 +0100 Subject: ethtool: Replace ethtool_ops::{get,set}_rxfh_indir() with {get,set}_rxfh() ETHTOOL_{G,S}RXFHINDIR and ETHTOOL_{G,S}RSSH should work for drivers regardless of whether they expose the hash key, unless you try to set a hash key for a driver that doesn't expose it. Signed-off-by: Ben Hutchings Acked-by: Jeff Kirsher --- net/core/ethtool.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'net/core') diff --git a/net/core/ethtool.c b/net/core/ethtool.c index b885734..8ae452a 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -582,7 +582,7 @@ static noinline_for_stack int ethtool_get_rxfh_indir(struct net_device *dev, int ret; if (!dev->ethtool_ops->get_rxfh_indir_size || - !dev->ethtool_ops->get_rxfh_indir) + !dev->ethtool_ops->get_rxfh) return -EOPNOTSUPP; dev_size = dev->ethtool_ops->get_rxfh_indir_size(dev); if (dev_size == 0) @@ -608,7 +608,7 @@ static noinline_for_stack int ethtool_get_rxfh_indir(struct net_device *dev, if (!indir) return -ENOMEM; - ret = dev->ethtool_ops->get_rxfh_indir(dev, indir); + ret = dev->ethtool_ops->get_rxfh(dev, indir, NULL); if (ret) goto out; @@ -632,7 +632,7 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev, int ret; u32 ringidx_offset = offsetof(struct ethtool_rxfh_indir, ring_index[0]); - if (!ops->get_rxfh_indir_size || !ops->set_rxfh_indir || + if (!ops->get_rxfh_indir_size || !ops->set_rxfh || !ops->get_rxnfc) return -EOPNOTSUPP; @@ -669,7 +669,7 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev, goto out; } - ret = ops->set_rxfh_indir(dev, indir); + ret = ops->set_rxfh(dev, indir, NULL); out: kfree(indir); -- cgit v1.1 From f062a3844845d267e3716cbc188ad502a15898b7 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 15 May 2014 16:28:07 +0100 Subject: ethtool: Check that reserved fields of struct ethtool_rxfh are 0 We should fail rather than silently ignoring use of these extensions. Signed-off-by: Ben Hutchings --- net/core/ethtool.c | 89 ++++++++++++++++++++++-------------------------------- 1 file changed, 36 insertions(+), 53 deletions(-) (limited to 'net/core') diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 8ae452a..17cb912 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -681,11 +681,11 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev, { int ret; const struct ethtool_ops *ops = dev->ethtool_ops; - u32 user_indir_size = 0, user_key_size = 0; + u32 user_indir_size, user_key_size; u32 dev_indir_size = 0, dev_key_size = 0; + struct ethtool_rxfh rxfh; u32 total_size; - u32 indir_offset, indir_bytes; - u32 key_offset; + u32 indir_bytes; u32 *indir = NULL; u8 *hkey = NULL; u8 *rss_config; @@ -697,33 +697,24 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev, if (ops->get_rxfh_indir_size) dev_indir_size = ops->get_rxfh_indir_size(dev); - - indir_offset = offsetof(struct ethtool_rxfh, indir_size); - - if (copy_from_user(&user_indir_size, - useraddr + indir_offset, - sizeof(user_indir_size))) - return -EFAULT; - - if (copy_to_user(useraddr + indir_offset, - &dev_indir_size, sizeof(dev_indir_size))) - return -EFAULT; - if (ops->get_rxfh_key_size) dev_key_size = ops->get_rxfh_key_size(dev); if ((dev_key_size + dev_indir_size) == 0) return -EOPNOTSUPP; - key_offset = offsetof(struct ethtool_rxfh, key_size); - - if (copy_from_user(&user_key_size, - useraddr + key_offset, - sizeof(user_key_size))) + if (copy_from_user(&rxfh, useraddr, sizeof(rxfh))) return -EFAULT; + user_indir_size = rxfh.indir_size; + user_key_size = rxfh.key_size; - if (copy_to_user(useraddr + key_offset, - &dev_key_size, sizeof(dev_key_size))) + /* Check that reserved fields are 0 for now */ + if (rxfh.rss_context || rxfh.rsvd[0] || rxfh.rsvd[1]) + return -EINVAL; + + rxfh.indir_size = dev_indir_size; + rxfh.key_size = dev_key_size; + if (copy_to_user(useraddr, &rxfh, sizeof(rxfh))) return -EFAULT; /* If the user buffer size is 0, this is just a query for the @@ -768,12 +759,11 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, int ret; const struct ethtool_ops *ops = dev->ethtool_ops; struct ethtool_rxnfc rx_rings; - u32 user_indir_size = 0, dev_indir_size = 0, i; - u32 user_key_size = 0, dev_key_size = 0; + struct ethtool_rxfh rxfh; + u32 dev_indir_size = 0, dev_key_size = 0, i; u32 *indir = NULL, indir_bytes = 0; u8 *hkey = NULL; u8 *rss_config; - u32 indir_offset, key_offset; u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]); if (!(ops->get_rxfh_indir_size || ops->get_rxfh_key_size) || @@ -782,40 +772,33 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, if (ops->get_rxfh_indir_size) dev_indir_size = ops->get_rxfh_indir_size(dev); - - indir_offset = offsetof(struct ethtool_rxfh, indir_size); - if (copy_from_user(&user_indir_size, - useraddr + indir_offset, - sizeof(user_indir_size))) - return -EFAULT; - if (ops->get_rxfh_key_size) dev_key_size = dev->ethtool_ops->get_rxfh_key_size(dev); - if ((dev_key_size + dev_indir_size) == 0) return -EOPNOTSUPP; - key_offset = offsetof(struct ethtool_rxfh, key_size); - if (copy_from_user(&user_key_size, - useraddr + key_offset, - sizeof(user_key_size))) + if (copy_from_user(&rxfh, useraddr, sizeof(rxfh))) return -EFAULT; + /* Check that reserved fields are 0 for now */ + if (rxfh.rss_context || rxfh.rsvd[0] || rxfh.rsvd[1]) + return -EINVAL; + /* If either indir or hash key is valid, proceed further. * It is not valid to request that both be unchanged. */ - if ((user_indir_size && - user_indir_size != ETH_RXFH_INDIR_NO_CHANGE && - user_indir_size != dev_indir_size) || - (user_key_size && (user_key_size != dev_key_size)) || - (user_indir_size == ETH_RXFH_INDIR_NO_CHANGE && - user_key_size == 0)) + if ((rxfh.indir_size && + rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE && + rxfh.indir_size != dev_indir_size) || + (rxfh.key_size && (rxfh.key_size != dev_key_size)) || + (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE && + rxfh.key_size == 0)) return -EINVAL; - if (user_indir_size != ETH_RXFH_INDIR_NO_CHANGE) + if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE) indir_bytes = dev_indir_size * sizeof(indir[0]); - rss_config = kzalloc(indir_bytes + user_key_size, GFP_USER); + rss_config = kzalloc(indir_bytes + rxfh.key_size, GFP_USER); if (!rss_config) return -ENOMEM; @@ -824,29 +807,29 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, if (ret) goto out; - /* user_indir_size == 0 means reset the indir table to default. - * user_indir_size == ETH_RXFH_INDIR_NO_CHANGE means leave it unchanged. + /* rxfh.indir_size == 0 means reset the indir table to default. + * rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE means leave it unchanged. */ - if (user_indir_size && - user_indir_size != ETH_RXFH_INDIR_NO_CHANGE) { + if (rxfh.indir_size && + rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE) { indir = (u32 *)rss_config; ret = ethtool_copy_validate_indir(indir, useraddr + rss_cfg_offset, &rx_rings, - user_indir_size); + rxfh.indir_size); if (ret) goto out; - } else if (user_indir_size == 0) { + } else if (rxfh.indir_size == 0) { indir = (u32 *)rss_config; for (i = 0; i < dev_indir_size; i++) indir[i] = ethtool_rxfh_indir_default(i, rx_rings.data); } - if (user_key_size) { + if (rxfh.key_size) { hkey = rss_config + indir_bytes; if (copy_from_user(hkey, useraddr + rss_cfg_offset + indir_bytes, - user_key_size)) { + rxfh.key_size)) { ret = -EFAULT; goto out; } -- cgit v1.1