ethtool: fail closed if we can't get max channel used in indirection tables

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.

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>

diff --git a/net/ethtool/channels.c b/net/ethtool/channels.c
index 7b4bbd674bae771c165f7a8867eeda0b3ad37990..cee188da54f85f147853971c5bab0498f6247346 100644 (file)
--- a/net/ethtool/channels.c
+++ b/net/ethtool/channels.c
@@ -171,11 +171,9 @@ ethnl_set_channels(struct ethnl_req_info *req_info, struct genl_info *info)
         */
        if (ethtool_get_max_rxnfc_channel(dev, &max_rxnfc_in_use))
                max_rxnfc_in_use = 0;
-       if (!netif_is_rxfh_configured(dev) ||
-           ethtool_get_max_rxfh_channel(dev, &max_rxfh_in_use))
-               max_rxfh_in_use = 0;
+       max_rxfh_in_use = ethtool_get_max_rxfh_channel(dev);
        if (channels.combined_count + channels.rx_count <= max_rxfh_in_use) {
-               GENL_SET_ERR_MSG(info, "requested channel counts are too low for existing indirection table settings");
+               GENL_SET_ERR_MSG_FMT(info, "requested channel counts are too low for existing indirection table (%d)", max_rxfh_in_use);
                return -EINVAL;
        }
        if (channels.combined_count + channels.rx_count <= max_rxnfc_in_use) {

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 6b2a360dcdf069218becaf8cbd79ad3faefbcf90..8a62375ebd1f0aed4aa032c8db7aa6710a096d46 100644 (file)
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -587,35 +587,39 @@ err_free_info:
        return err;
 }
 
-int ethtool_get_max_rxfh_channel(struct net_device *dev, u32 *max)
+u32 ethtool_get_max_rxfh_channel(struct net_device *dev)
 {
        struct ethtool_rxfh_param rxfh = {};
-       u32 dev_size, current_max = 0;
+       u32 dev_size, current_max;
        int ret;
 
+       if (!netif_is_rxfh_configured(dev))
+               return 0;
+
        if (!dev->ethtool_ops->get_rxfh_indir_size ||
            !dev->ethtool_ops->get_rxfh)
-               return -EOPNOTSUPP;
+               return 0;
        dev_size = dev->ethtool_ops->get_rxfh_indir_size(dev);
        if (dev_size == 0)
-               return -EOPNOTSUPP;
+               return 0;
 
        rxfh.indir = kcalloc(dev_size, sizeof(rxfh.indir[0]), GFP_USER);
        if (!rxfh.indir)
-               return -ENOMEM;
+               return U32_MAX;
 
        ret = dev->ethtool_ops->get_rxfh(dev, &rxfh);
-       if (ret)
-               goto out;
+       if (ret) {
+               current_max = U32_MAX;
+               goto out_free;
+       }
 
+       current_max = 0;
        while (dev_size--)
                current_max = max(current_max, rxfh.indir[dev_size]);
 
-       *max = current_max;
-
-out:
+out_free:
        kfree(rxfh.indir);
-       return ret;
+       return current_max;
 }
 
 int ethtool_check_ops(const struct ethtool_ops *ops)

diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 28b8aaaf9bcb3c5dcfc10be9d0d9cab1b6e5c4e1..b55705a9ad5aa014cf2a888bd6c0c9e922d7608e 100644 (file)
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -42,7 +42,7 @@ int __ethtool_get_link(struct net_device *dev);
 bool convert_legacy_settings_to_link_ksettings(
        struct ethtool_link_ksettings *link_ksettings,
        const struct ethtool_cmd *legacy_settings);
-int ethtool_get_max_rxfh_channel(struct net_device *dev, u32 *max);
+u32 ethtool_get_max_rxfh_channel(struct net_device *dev);
 int ethtool_get_max_rxnfc_channel(struct net_device *dev, u64 *max);
 int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info);
 

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index d72b0fec89af6d08dcd4dfbebb4da2a99b665f32..615812ff89746f8c441f21bd2e654b1add4f0f10 100644 (file)
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2049,9 +2049,7 @@ static noinline_for_stack int ethtool_set_channels(struct net_device *dev,
         * indirection table/rxnfc settings */
        if (ethtool_get_max_rxnfc_channel(dev, &max_rxnfc_in_use))
                max_rxnfc_in_use = 0;
-       if (!netif_is_rxfh_configured(dev) ||
-           ethtool_get_max_rxfh_channel(dev, &max_rxfh_in_use))
-               max_rxfh_in_use = 0;
+       max_rxfh_in_use = ethtool_get_max_rxfh_channel(dev);
        if (channels.combined_count + channels.rx_count <=
            max_t(u64, max_rxnfc_in_use, max_rxfh_in_use))
                return -EINVAL;