[PATCH net v2 0/2] Fix link_mode derived params functionality

[PATCH net v2 0/2] Fix link_mode derived params functionality

[Danielle Ratson]

Currently, link_mode parameter derives 3 other link parameters, speed,
lanes and duplex, and the derived information is sent to user space.

Two bugs were found in that functionality.
First, some drivers clear the 'ethtool_link_ksettings' struct in their
get_link_ksettings() callback and cause receiving wrong link mode
information in user space. And also, some drivers can report random
values in the 'link_mode' field and cause general protection fault.

Second, the link parameters are only derived in netlink path so in ioctl
path, we don't any reasonable values.

Patch #1 solves the first problem by introducing a new capability bit for
supporting link_mode in driver.
Patch #2 solves the second one, by deriving the parameters in ioctl path
as well.

v2:
	* Add patch #2.
	* Introduce 'cap_link_mode_supported' instead of adding a
	  validity field to 'ethtool_link_ksettings' struct in patch #1.

Danielle Ratson (2):
  ethtool: Add link_mode parameter capability bit to ethtool_ops
  ethtool: Derive parameters from link_mode in ioctl path

 .../mellanox/mlxsw/spectrum_ethtool.c         |  1 +
 include/linux/ethtool.h                       |  5 ++-
 net/ethtool/ioctl.c                           | 34 ++++++++++++++-----
 3 files changed, 31 insertions(+), 9 deletions(-)

-- 
2.26.2

[PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops

[Danielle Ratson]

Some drivers clear the 'ethtool_link_ksettings' struct in their
get_link_ksettings() callback, before populating it with actual values.
Such drivers will set the new 'link_mode' field to zero, resulting in
user space receiving wrong link mode information given that zero is a
valid value for the field.

Fix this by introducing a new capability bit ('cap_link_mode_supported')
to ethtool_ops, which indicates whether the driver supports 'link_mode'
parameter or not. Set it to true in mlxsw which is currently the only
driver supporting 'link_mode'.

Another problem is that some drivers (notably tun) can report random
values in the 'link_mode' field. This can result in a general protection
fault when the field is used as an index to the 'link_mode_params' array
[1].

This happens because such drivers implement their set_link_ksettings()
callback by simply overwriting their private copy of
'ethtool_link_ksettings' struct with the one they get from the stack,
which is not always properly initialized.

Fix this by making sure that the implied parameters will be derived from
the 'link_mode' parameter only when the 'cap_link_mode_supported' is set.

v2:
	* Introduce 'cap_link_mode_supported' instead of adding a
	  validity field to 'ethtool_link_ksettings' struct.

[1]
general protection fault, probably for non-canonical address 0xdffffc00f14cc32c: 0000 [#1] PREEMPT SMP KASAN
KASAN: probably user-memory-access in range [0x000000078a661960-0x000000078a661967]
CPU: 0 PID: 8452 Comm: syz-executor360 Not tainted 5.11.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:__ethtool_get_link_ksettings+0x1a3/0x3a0 net/ethtool/ioctl.c:446
Code: b7 3e fa 83 fd ff 0f 84 30 01 00 00 e8 16 b0 3e fa 48 8d 3c ed 60 d5 69 8a 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48 89 f8 83 e0 07 83 c0 03
+38 d0 7c 08 84 d2 0f 85 b9
RSP: 0018:ffffc900019df7a0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff888026136008 RCX: 0000000000000000
RDX: 00000000f14cc32c RSI: ffffffff873439ca RDI: 000000078a661960
RBP: 00000000ffff8880 R08: 00000000ffffffff R09: ffff88802613606f
R10: ffffffff873439bc R11: 0000000000000000 R12: 0000000000000000
R13: ffff88802613606c R14: ffff888011d0c210 R15: ffff888011d0c210
FS:  0000000000749300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004b60f0 CR3: 00000000185c2000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 linkinfo_prepare_data+0xfd/0x280 net/ethtool/linkinfo.c:37
 ethnl_default_notify+0x1dc/0x630 net/ethtool/netlink.c:586
 ethtool_notify+0xbd/0x1f0 net/ethtool/netlink.c:656
 ethtool_set_link_ksettings+0x277/0x330 net/ethtool/ioctl.c:620
 dev_ethtool+0x2b35/0x45d0 net/ethtool/ioctl.c:2842
 dev_ioctl+0x463/0xb70 net/core/dev_ioctl.c:440
 sock_do_ioctl+0x148/0x2d0 net/socket.c:1060
 sock_ioctl+0x477/0x6a0 net/socket.c:1177
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl fs/ioctl.c:739 [inline]
 __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: c8907043c6ac9 ("ethtool: Get link mode in use instead of speed and duplex parameters")
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c | 1 +
 include/linux/ethtool.h                                | 5 ++++-
 net/ethtool/ioctl.c                                    | 8 ++++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index 0bd64169bf81..54f04c94210c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -1061,6 +1061,7 @@ mlxsw_sp_get_ts_info(struct net_device *netdev, struct ethtool_ts_info *info)
 
 const struct ethtool_ops mlxsw_sp_port_ethtool_ops = {
 	.cap_link_lanes_supported	= true,
+	.cap_link_mode_supported	= true,
 	.get_drvinfo			= mlxsw_sp_port_get_drvinfo,
 	.get_link			= ethtool_op_get_link,
 	.get_link_ext_state		= mlxsw_sp_port_get_link_ext_state,
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index ec4cd3921c67..9e6eb6df375d 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -269,6 +269,8 @@ struct ethtool_pause_stats {
  * struct ethtool_ops - optional netdev operations
  * @cap_link_lanes_supported: indicates if the driver supports lanes
  *	parameter.
+ * @cap_link_mode_supported: indicates if the driver supports link_mode
+ *	parameter.
  * @supported_coalesce_params: supported types of interrupt coalescing.
  * @get_drvinfo: Report driver/device information.  Should only set the
  *	@driver, @version, @fw_version and @bus_info fields.  If not
@@ -424,7 +426,8 @@ struct ethtool_pause_stats {
  * of the generic netdev features interface.
  */
 struct ethtool_ops {
-	u32     cap_link_lanes_supported:1;
+	u32     cap_link_lanes_supported:1,
+		cap_link_mode_supported:1;
 	u32	supported_coalesce_params;
 	void	(*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
 	int	(*get_regs_len)(struct net_device *);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 24783b71c584..cebbf93b27a7 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -436,12 +436,16 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
 
 	memset(link_ksettings, 0, sizeof(*link_ksettings));
 
-	link_ksettings->link_mode = -1;
 	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
 	if (err)
 		return err;
 
-	if (link_ksettings->link_mode != -1) {
+	if (dev->ethtool_ops->cap_link_mode_supported &&
+	    link_ksettings->link_mode != -1) {
+		if (WARN_ON_ONCE(link_ksettings->link_mode >=
+				 __ETHTOOL_LINK_MODE_MASK_NBITS))
+			return -EINVAL;
+
 		link_info = &link_mode_params[link_ksettings->link_mode];
 		link_ksettings->base.speed = link_info->speed;
 		link_ksettings->lanes = link_info->lanes;
-- 
2.26.2

Re: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops

[Andrew Lunn]

On Sun, Apr 04, 2021 at 11:14:32AM +0300, Danielle Ratson wrote:
> Some drivers clear the 'ethtool_link_ksettings' struct in their
> get_link_ksettings() callback, before populating it with actual values.
> Such drivers will set the new 'link_mode' field to zero, resulting in
> user space receiving wrong link mode information given that zero is a
> valid value for the field.

That sounds like the bug, that 0 means something, not that the
structure is zeroed. It is good practice to zero structures about to
be returned to user space otherwise you could leak stack information
etc.

Do we still have time to fix the KAPI, so zero means unset? Where in
the workflow is the patch adding this feature? Is it in a released
kernel? or -rc kernel?

> Fix this by introducing a new capability bit ('cap_link_mode_supported')
> to ethtool_ops, which indicates whether the driver supports 'link_mode'
> parameter or not. Set it to true in mlxsw which is currently the only
> driver supporting 'link_mode'.
> 
> Another problem is that some drivers (notably tun) can report random
> values in the 'link_mode' field. This can result in a general protection
> fault when the field is used as an index to the 'link_mode_params' array
> [1].
> 
> This happens because such drivers implement their set_link_ksettings()
> callback by simply overwriting their private copy of
> 'ethtool_link_ksettings' struct with the one they get from the stack,
> which is not always properly initialized.
> 
> Fix this by making sure that the implied parameters will be derived from
> the 'link_mode' parameter only when the 'cap_link_mode_supported' is set.

So you left in place the kernel memory leak to user space? Or is there
an additional patch to fix tun as well?

   Andrew

RE: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops

[Danielle Ratson]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Sunday, April 4, 2021 5:18 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org; eric.dumazet@gmail.com; mkubecek@suse.cz;
> f.fainelli@gmail.com; acardace@redhat.com; irusskikh@marvell.com; gustavo@embeddedor.com; magnus.karlsson@intel.com;
> ecree@solarflare.com; Ido Schimmel <idosch@nvidia.com>; Jiri Pirko <jiri@nvidia.com>; mlxsw <mlxsw@nvidia.com>
> Subject: Re: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops
> 
> On Sun, Apr 04, 2021 at 11:14:32AM +0300, Danielle Ratson wrote:
> > Some drivers clear the 'ethtool_link_ksettings' struct in their
> > get_link_ksettings() callback, before populating it with actual values.
> > Such drivers will set the new 'link_mode' field to zero, resulting in
> > user space receiving wrong link mode information given that zero is a
> > valid value for the field.
> 
> That sounds like the bug, that 0 means something, not that the structure is zeroed. It is good practice to zero structures about to be
> returned to user space otherwise you could leak stack information etc.
> 
> Do we still have time to fix the KAPI, so zero means unset? Where in the workflow is the patch adding this feature? Is it in a released
> kernel? or -rc kernel?

First, it is not the API structure that is passed to user space. Please pay attention that the link_mode field is added to "ethtool_link_ksettings" and not "ethtool_link_settings", which is the API.
So I am not sure what leak could happen in that situation, could you explain?

Second, the link_mode indices are uAPI, so 0 is not forbidden.

> 
> > Fix this by introducing a new capability bit
> > ('cap_link_mode_supported') to ethtool_ops, which indicates whether the driver supports 'link_mode'
> > parameter or not. Set it to true in mlxsw which is currently the only
> > driver supporting 'link_mode'.
> >
> > Another problem is that some drivers (notably tun) can report random
> > values in the 'link_mode' field. This can result in a general
> > protection fault when the field is used as an index to the
> > 'link_mode_params' array [1].
> >
> > This happens because such drivers implement their set_link_ksettings()
> > callback by simply overwriting their private copy of
> > 'ethtool_link_ksettings' struct with the one they get from the stack,
> > which is not always properly initialized.
> >
> > Fix this by making sure that the implied parameters will be derived
> > from the 'link_mode' parameter only when the 'cap_link_mode_supported' is set.
> 
> So you left in place the kernel memory leak to user space? Or is there an additional patch to fix tun as well?
> 
>    Andrew

Again, not sure what is the memory leak you are talking about. 
This patch solves the protection fault in tun driver, by the fact that now we are paying attention to the link_mode value only if the driver confirmed it supports the link_mode and set it properly.

Re: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops

[Andrew Lunn]

> First, it is not the API structure that is passed to user space. Please pay attention

Yes, sorry. Jumped to assumptions without checking.

Let me try again.

    Andrew

Re: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops

[Andrew Lunn]

> @@ -436,12 +436,16 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
>  
>  	memset(link_ksettings, 0, sizeof(*link_ksettings));
>  
> -	link_ksettings->link_mode = -1;
>  	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
>  	if (err)
>  		return err;
>  
> -	if (link_ksettings->link_mode != -1) {
> +	if (dev->ethtool_ops->cap_link_mode_supported &&
> +	    link_ksettings->link_mode != -1) {

Is this -1 behaviour documented anywhere? It seems like you just
changed its meaning. It used to mean, this field has not been set,
ignore it. Adding the cap_link_mode_supported it now means, we have
field has been set, but we have no idea what link mode is being used.
So you should probably add something to the documentation of struct
ethtool_link_ksettings.

I wonder if we should actually add ETHTOOL_LINK_MODE_UNKNOWN to enum
ethtool_link_mode_bit_indices?

> +		if (WARN_ON_ONCE(link_ksettings->link_mode >=
> +				 __ETHTOOL_LINK_MODE_MASK_NBITS))
> +			return -EINVAL;
> +
>  		link_info = &link_mode_params[link_ksettings->link_mode];
>  		link_ksettings->base.speed = link_info->speed;
>  		link_ksettings->lanes = link_info->lanes;

If dev->ethtool_ops->cap_link_mode_supported &&
link_ksettings->link_mode == -1 should you be setting speed to
SPEED_UNKNOWN, and lanes to LANE_UNKNOWN? Or is that already the
default?

But over all, this API between the core and the driver seems
messy. Why not just add a helper in common.c which translates link
mode to speed/duplex/lanes and call it in the driver. Then you don't
need this capability flags, which i doubt any other driver will ever
use. And you don't need to worry about drivers returning random
values. As far as i can see, the link_mode returned by the driver is
not used for anything other than for this translation. So i don't see
a need for it outside of the driver. Or maybe i'm missing something?

	Andrew

RE: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops

[Danielle Ratson]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Sunday, April 4, 2021 7:33 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org; eric.dumazet@gmail.com; mkubecek@suse.cz;
> f.fainelli@gmail.com; acardace@redhat.com; irusskikh@marvell.com; gustavo@embeddedor.com; magnus.karlsson@intel.com;
> ecree@solarflare.com; Ido Schimmel <idosch@nvidia.com>; Jiri Pirko <jiri@nvidia.com>; mlxsw <mlxsw@nvidia.com>
> Subject: Re: [PATCH net v2 1/2] ethtool: Add link_mode parameter capability bit to ethtool_ops
> 
> > @@ -436,12 +436,16 @@ int __ethtool_get_link_ksettings(struct
> > net_device *dev,
> >
> >  	memset(link_ksettings, 0, sizeof(*link_ksettings));
> >
> > -	link_ksettings->link_mode = -1;
> >  	err = dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> >  	if (err)
> >  		return err;
> >
> > -	if (link_ksettings->link_mode != -1) {
> > +	if (dev->ethtool_ops->cap_link_mode_supported &&
> > +	    link_ksettings->link_mode != -1) {
> 
> Is this -1 behaviour documented anywhere? It seems like you just changed its meaning. It used to mean, this field has not been set,
> ignore it. Adding the cap_link_mode_supported it now means, we have field has been set, but we have no idea what link mode is
> being used.
> So you should probably add something to the documentation of struct ethtool_link_ksettings.
> 
> I wonder if we should actually add ETHTOOL_LINK_MODE_UNKNOWN to enum ethtool_link_mode_bit_indices?
> 
> > +		if (WARN_ON_ONCE(link_ksettings->link_mode >=
> > +				 __ETHTOOL_LINK_MODE_MASK_NBITS))
> > +			return -EINVAL;
> > +
> >  		link_info = &link_mode_params[link_ksettings->link_mode];
> >  		link_ksettings->base.speed = link_info->speed;
> >  		link_ksettings->lanes = link_info->lanes;
> 
> If dev->ethtool_ops->cap_link_mode_supported && link_ksettings->link_mode == -1 should you be setting speed to
> SPEED_UNKNOWN, and lanes to LANE_UNKNOWN? Or is that already the default?
> 
> But over all, this API between the core and the driver seems messy. Why not just add a helper in common.c which translates link
> mode to speed/duplex/lanes and call it in the driver. Then you don't need this capability flags, which i doubt any other driver will ever
> use. And you don't need to worry about drivers returning random values. As far as i can see, the link_mode returned by the driver is
> not used for anything other than for this translation. So i don't see a need for it outside of the driver. Or maybe i'm missing
> something?
> 
> 	Andrew

Hi Andrew,

Please see my patch here: https://github.com/daniellerts/linux_mlxsw/commit/72ca614951418843aa87323630c354691d9e50d4
Since a lack of time until the window closes, please see if that is what you meant and you are ok with it, before I am sending another version.

Thanks,
Danielle 

[PATCH net v2 2/2] ethtool: Derive parameters from link_mode in ioctl path

[Danielle Ratson]

Currently, some ethtool link parameters, like speed, lanes and duplex, are
derived from the link_mode parameter. These link parameters are only
derived in __ethtool_get_link_ksettings(), but the ioctl path does not go
through this function. This means that old ethtool (w/o netlink) won't get
any reasonable values.

Add a function that derives the parameters from link_mode and use it in
ioctl paths as well.

Output before:

$ ethtool --version
ethtool version 5.4
$ ethtool swp13
Settings for swp13:
        Supported ports: [ FIBRE Backplane ]
        Supported link modes:   1000baseKX/Full
                                10000baseKR/Full
                                25000baseCR/Full
                                25000baseKR/Full
                                25000baseSR/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Supported FEC modes: Not reported
        Advertised link modes:  1000baseKX/Full
                                10000baseKR/Full
                                25000baseCR/Full
                                25000baseKR/Full
                                25000baseSR/Full
        Advertised pause frame use: No
        Advertised auto-negotiation: Yes
        Advertised FEC modes: Not reported
        Speed: Unknown!
        Duplex: Half
        Port: Direct Attach Copper
        PHYAD: 0
        Transceiver: internal
        Auto-negotiation: on
        Link detected: yes

Output after:

$ ethtool swp13
Settings for swp13:
        Supported ports: [ FIBRE Backplane ]
        Supported link modes:   1000baseKX/Full
                                10000baseKR/Full
                                25000baseCR/Full
                                25000baseKR/Full
                                25000baseSR/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Supported FEC modes: Not reported
        Advertised link modes:  1000baseKX/Full
                                10000baseKR/Full
                                25000baseCR/Full
                                25000baseKR/Full
                                25000baseSR/Full
        Advertised pause frame use: No
        Advertised auto-negotiation: Yes
        Advertised FEC modes: Not reported
        Speed: 25000Mb/s
        Duplex: Full
        Port: Direct Attach Copper
        PHYAD: 0
        Transceiver: internal
        Auto-negotiation: on
        Link detected: yes

Fixes: c8907043c6ac9 ("ethtool: Get link mode in use instead of speed and duplex parameters")
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reported-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/ethtool/ioctl.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index cebbf93b27a7..943162ef080c 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -422,11 +422,31 @@ struct ethtool_link_usettings {
 	} link_modes;
 };
 
+static int
+ethtool_params_from_link_mode(const struct net_device *dev,
+			      struct ethtool_link_ksettings *link_ksettings)
+{
+	const struct link_mode_info *link_info;
+
+	if (dev->ethtool_ops->cap_link_mode_supported &&
+	    link_ksettings->link_mode != -1) {
+		if (WARN_ON_ONCE(link_ksettings->link_mode >=
+				 __ETHTOOL_LINK_MODE_MASK_NBITS))
+			return -EINVAL;
+
+		link_info = &link_mode_params[link_ksettings->link_mode];
+		link_ksettings->base.speed = link_info->speed;
+		link_ksettings->lanes = link_info->lanes;
+		link_ksettings->base.duplex = link_info->duplex;
+	}
+
+	return 0;
+}
+
 /* Internal kernel helper to query a device ethtool_link_settings. */
 int __ethtool_get_link_ksettings(struct net_device *dev,
 				 struct ethtool_link_ksettings *link_ksettings)
 {
-	const struct link_mode_info *link_info;
 	int err;
 
 	ASSERT_RTNL();
@@ -440,17 +460,7 @@ int __ethtool_get_link_ksettings(struct net_device *dev,
 	if (err)
 		return err;
 
-	if (dev->ethtool_ops->cap_link_mode_supported &&
-	    link_ksettings->link_mode != -1) {
-		if (WARN_ON_ONCE(link_ksettings->link_mode >=
-				 __ETHTOOL_LINK_MODE_MASK_NBITS))
-			return -EINVAL;
-
-		link_info = &link_mode_params[link_ksettings->link_mode];
-		link_ksettings->base.speed = link_info->speed;
-		link_ksettings->lanes = link_info->lanes;
-		link_ksettings->base.duplex = link_info->duplex;
-	}
+	ethtool_params_from_link_mode(dev, link_ksettings);
 
 	return 0;
 }
@@ -572,6 +582,8 @@ static int ethtool_get_link_ksettings(struct net_device *dev,
 	if (err < 0)
 		return err;
 
+	ethtool_params_from_link_mode(dev, &link_ksettings);
+
 	/* make sure we tell the right values to user */
 	link_ksettings.base.cmd = ETHTOOL_GLINKSETTINGS;
 	link_ksettings.base.link_mode_masks_nwords
@@ -673,6 +685,8 @@ static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 		return err;
 	convert_link_ksettings_to_legacy_settings(&cmd, &link_ksettings);
 
+	ethtool_params_from_link_mode(dev, &link_ksettings);
+
 	/* send a sensible cmd tag back to user */
 	cmd.cmd = ETHTOOL_GSET;
 
-- 
2.26.2