netdev - Re: [PATCH net-next] ethtool: Set cmd field in ETHTOOL

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]

Date:	Tue, 15 Mar 2016 19:00:15 +0000 (UTC)
From:	david decotigny <ddecotig@...il.com>
To:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] ethtool: Set cmd field in ETHTOOL_GLINKSETTINGS response to wrong nwords

Ben Hutchings <ben <at> decadent.org.uk> writes:
> 
> When the ETHTOOL_GLINKSETTINGS implementation finds that userland is
> using the wrong number of words of link mode bitmaps (or is trying to
> find out the right numbers) it sets the cmd field to 0 in the response
> structure.
> 
> This is inconsistent with the implementation of every other ethtool
> command, so let's remove that inconsistency before it gets into a
> stable release.
> 
> Fixes: 3f1ac7a700d03 ("net: ethtool: add new ETHTOOL_xLINKSETTINGS API")
> Signed-off-by: Ben Hutchings <ben <at> decadent.org.uk>
> ---
> David, please can you include this in changes for 4.6?
> 
> Ben.
> 
>  net/core/ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 2966cd0d7c93..f426c5ad6149 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
>  <at>  <at>  -655,7 +655,7  <at>  <at>  static int 
ethtool_get_link_ksettings(struct net_device *dev,
>  	    != link_ksettings.base.link_mode_masks_nwords) {
>  		/* wrong link mode nbits requested */
>  		memset(&link_ksettings, 0, sizeof(link_ksettings));
> -		/* keep cmd field reset to 0 */
> +		link_ksettings.base.cmd = ETHTOOL_GLINKSETTINGS;
>  		/* send back number of words required as negative val */
>  		compiletime_assert(__ETHTOOL_LINK_MODE_MASK_NU32 <= S8_MAX,
>  				   "need too many bits for link modes!");
> 

thanks!

Please also update the comments in ethtool.h, proposed change below:

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 2835b07..9222db8 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1648,9 +1648,9 @@ enum ethtool_reset_flags {
  *	%ETHTOOL_GLINKSETTINGS: on entry, number of words passed by user
  *	(>= 0); on return, if handshake in progress, negative if
  *	request size unsupported by kernel: absolute value indicates
- *	kernel recommended size and cmd field is 0, as well as all the
- *	other fields; otherwise (handshake completed), strictly
- *	positive to indicate size used by kernel and cmd field is
+ *	kernel expected size and all the other fields but cmd
+ *	are 0; otherwise (handshake completed), strictly positive
+ *	to indicate size used by kernel and cmd field stays
  *	%ETHTOOL_GLINKSETTINGS, all other fields populated by driver. For
  *	%ETHTOOL_SLINKSETTINGS: must be valid on entry, ie. a positive
  *	value returned previously by %ETHTOOL_GLINKSETTINGS, otherwise