[PATCH 6.1.y] ethtool: fail closed if we can't get max channel used in indirection tables

[PATCH 6.1.y] ethtool: fail closed if we can't get max channel used in indirection tables

[Artem Dinaburg]

From: Jakub Kicinski <kuba@kernel.org>

[ Upstream commit 2899d58462ba868287d6ff3acad3675e7adf934f ]

Commit 0d1b7d6c9274 ("bnxt: fix crashes when reducing ring count with
active RSS contexts") proves that allowing indirection table to contain
channels with out of bounds IDs may lead to crashes. Currently the
max channel check in the core gets skipped if driver can't fetch
the indirection table or when we can't allocate memory.

Both of those conditions should be extremely rare but if they do
happen we should try to be safe and fail the channel change.

[ Backport to 6.1.y: kept the older int-returning
  ethtool_get_max_rxfh_channel() and propagated its errors at both
  channel-change call sites instead of the upstream u32/U32_MAX
  conversion. A failure now returns the helper's errno (-ENOMEM or the
  driver's get_rxfh() error) rather than -EINVAL, and -EOPNOTSUPP also
  fails closed; on 6.1 IFF_RXFH_CONFIGURED can only be set via set_rxfh
  paths that require a non-zero indirection table size. ]

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20240710174043.754664-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Assisted-by: LLM
Signed-off-by: Artem Dinaburg <artem@trailofbits.com>
---
Hi Greg, Sasha, and net maintainers,

I am working through the small CVE backports still missing from 6.1.y.
This one addresses CVE-2024-46834. It aborts a channel reduction when the
core cannot prove RSS table entries remain valid.

The corresponding 6.6.y backport is already in the 6.6.y stable queue.
The fix is already present in 6.12.y, 6.18.y, and 7.2.y, but not
in 6.1.y.
The target-specific adjustment is recorded in the bracketed note above.

Could you please queue it for 6.1.y?

CVE: CVE-2024-46834
Upstream: 2899d58462ba868287d6ff3acad3675e7adf934f

AI assistance: An LLM helped identify, adapt, and validate this backport; I
reviewed the resulting code and validation evidence.

Thanks,
Artem Dinaburg

 net/ethtool/channels.c |  9 ++++++---
 net/ethtool/ioctl.c    | 11 +++++++----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/net/ethtool/channels.c b/net/ethtool/channels.c
index 40315886201158..05f3068ec0fe35 100644
--- a/net/ethtool/channels.c
+++ b/net/ethtool/channels.c
@@ -191,9 +191,12 @@ int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
 	/* ensure the new Rx count fits within the configured Rx flow
 	 * indirection table settings
 	 */
-	if (netif_is_rxfh_configured(dev) &&
-	    !ethtool_get_max_rxfh_channel(dev, &max_rx_in_use) &&
-	    (channels.combined_count + channels.rx_count) <= max_rx_in_use) {
+	if (netif_is_rxfh_configured(dev)) {
+		ret = ethtool_get_max_rxfh_channel(dev, &max_rx_in_use);
+		if (ret)
+			goto out_ops;
+	}
+	if (channels.combined_count + channels.rx_count <= max_rx_in_use) {
 		ret = -EINVAL;
 		GENL_SET_ERR_MSG(info, "requested channel counts are too low for existing indirection table settings");
 		goto out_ops;
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 1f64ff5f3f21cb..439871dd1ca83b 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1832,10 +1832,13 @@ static noinline_for_stack int ethtool_set_channels(struct net_device *dev,
 
 	/* ensure the new Rx count fits within the configured Rx flow
 	 * indirection table settings */
-	if (netif_is_rxfh_configured(dev) &&
-	    !ethtool_get_max_rxfh_channel(dev, &max_rx_in_use) &&
-	    (channels.combined_count + channels.rx_count) <= max_rx_in_use)
-	    return -EINVAL;
+	if (netif_is_rxfh_configured(dev)) {
+		ret = ethtool_get_max_rxfh_channel(dev, &max_rx_in_use);
+		if (ret)
+			return ret;
+	}
+	if (channels.combined_count + channels.rx_count <= max_rx_in_use)
+		return -EINVAL;
 
 	/* Disabling channels, query zero-copy AF_XDP sockets */
 	from_channel = channels.combined_count +
-- 
2.39.5

Re: [PATCH 6.1.y] ethtool: fail closed if we can't get max channel used in indirection tables

[Sasha Levin]

> [ Upstream commit 2899d58462ba868287d6ff3acad3675e7adf934f ]

Queued for 6.1, thanks.

-- 
Thanks,
Sasha