netdev - Re: [PATCH net v2 1/3] ethtool: Fix preserving of wanted feature bits in netlink interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200818221559.2iyoukyo5jlkkd4p@lion.mk-sys.cz>
Date:   Wed, 19 Aug 2020 00:15:59 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Maxim Mikityanskiy <maximmi@...lanox.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
        Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net v2 1/3] ethtool: Fix preserving of wanted feature
 bits in netlink interface

On Mon, Aug 17, 2020 at 04:34:05PM +0300, Maxim Mikityanskiy wrote:
> Currently, ethtool-netlink calculates new wanted bits as:
> (req_wanted & req_mask) | (old_active & ~req_mask)
> 
> It completely discards the old wanted bits, so they are forgotten with
> the next ethtool command. Sample steps to reproduce:
> 
> 1. ethtool -k eth0
>    tx-tcp-segmentation: on # TSO is on from the beginning
> 2. ethtool -K eth0 tx off
>    tx-tcp-segmentation: off [not requested]
> 3. ethtool -k eth0
>    tx-tcp-segmentation: off [requested on]
> 4. ethtool -K eth0 rx off # Some change unrelated to TSO
> 5. ethtool -k eth0
>    tx-tcp-segmentation: off # "Wanted on" is forgotten
> 
> This commit fixes it by changing the formula to:
> (req_wanted & req_mask) | (old_wanted & ~req_mask),
> where old_active was replaced by old_wanted to account for the wanted
> bits.
> 
> The shortcut condition for the case where nothing was changed now
> compares wanted bitmasks, instead of wanted to active.
> 
> Fixes: 0980bfcd6954 ("ethtool: set netdev features with FEATURES_SET request")
> Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com>
> ---

Reviewed-by: Michal Kubecek <mkubecek@...e.cz>

>  net/ethtool/features.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ethtool/features.c b/net/ethtool/features.c
> index 4e632dc987d8..ec196f0fddc9 100644
> --- a/net/ethtool/features.c
> +++ b/net/ethtool/features.c
> @@ -224,7 +224,9 @@ int ethnl_set_features(struct sk_buff *skb, struct genl_info *info)
>  	DECLARE_BITMAP(wanted_diff_mask, NETDEV_FEATURE_COUNT);
>  	DECLARE_BITMAP(active_diff_mask, NETDEV_FEATURE_COUNT);
>  	DECLARE_BITMAP(old_active, NETDEV_FEATURE_COUNT);
> +	DECLARE_BITMAP(old_wanted, NETDEV_FEATURE_COUNT);
>  	DECLARE_BITMAP(new_active, NETDEV_FEATURE_COUNT);
> +	DECLARE_BITMAP(new_wanted, NETDEV_FEATURE_COUNT);
>  	DECLARE_BITMAP(req_wanted, NETDEV_FEATURE_COUNT);
>  	DECLARE_BITMAP(req_mask, NETDEV_FEATURE_COUNT);
>  	struct nlattr *tb[ETHTOOL_A_FEATURES_MAX + 1];
> @@ -250,6 +252,7 @@ int ethnl_set_features(struct sk_buff *skb, struct genl_info *info)
>  
>  	rtnl_lock();
>  	ethnl_features_to_bitmap(old_active, dev->features);
> +	ethnl_features_to_bitmap(old_wanted, dev->wanted_features);
>  	ret = ethnl_parse_bitset(req_wanted, req_mask, NETDEV_FEATURE_COUNT,
>  				 tb[ETHTOOL_A_FEATURES_WANTED],
>  				 netdev_features_strings, info->extack);
> @@ -261,11 +264,11 @@ int ethnl_set_features(struct sk_buff *skb, struct genl_info *info)
>  		goto out_rtnl;
>  	}
>  
> -	/* set req_wanted bits not in req_mask from old_active */
> +	/* set req_wanted bits not in req_mask from old_wanted */
>  	bitmap_and(req_wanted, req_wanted, req_mask, NETDEV_FEATURE_COUNT);
> -	bitmap_andnot(new_active, old_active, req_mask, NETDEV_FEATURE_COUNT);
> -	bitmap_or(req_wanted, new_active, req_wanted, NETDEV_FEATURE_COUNT);
> -	if (bitmap_equal(req_wanted, old_active, NETDEV_FEATURE_COUNT)) {
> +	bitmap_andnot(new_wanted, old_wanted, req_mask, NETDEV_FEATURE_COUNT);
> +	bitmap_or(req_wanted, new_wanted, req_wanted, NETDEV_FEATURE_COUNT);
> +	if (bitmap_equal(req_wanted, old_wanted, NETDEV_FEATURE_COUNT)) {
>  		ret = 0;
>  		goto out_rtnl;
>  	}
> -- 
> 2.25.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)