| 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
| ||
|
Message-ID: <20210304090933.3538255-1-danieller@nvidia.com>
Date: Thu, 4 Mar 2021 11:09:33 +0200
From: Danielle Ratson <danieller@...dia.com>
To: <netdev@...r.kernel.org>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <eric.dumazet@...il.com>,
<andrew@...n.ch>, <mkubecek@...e.cz>, <f.fainelli@...il.com>,
<acardace@...hat.com>, <irusskikh@...vell.com>,
<gustavo@...eddedor.com>, <magnus.karlsson@...el.com>,
<ecree@...arflare.com>, <idosch@...dia.com>, <jiri@...dia.com>,
<mlxsw@...dia.com>, Danielle Ratson <danieller@...dia.com>
Subject: [PATCH net] ethtool: Add indicator field for link_mode validity to link_ksettings
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 boolean field ('link_mode_valid'), which
indicates whether the 'link_mode' field is valid or not. Set it in mlxsw
which is currently the only driver supporting the new API.
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 new 'link_mode_valid' field is always
initialized to 'false' before invoking the set_link_ksettings()
callback.
[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@...dia.com>
Reported-by: Eric Dumazet <eric.dumazet@...il.com>
Reviewed-by: Ido Schimmel <idosch@...dia.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c | 9 ++++-----
include/linux/ethtool.h | 1 +
net/ethtool/ioctl.c | 6 ++++--
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index 0bd64169bf81..66eb1cd17bc4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -1232,14 +1232,14 @@ mlxsw_sp1_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
{
int i;
- cmd->link_mode = -1;
-
if (!carrier_ok)
return;
for (i = 0; i < MLXSW_SP1_PORT_LINK_MODE_LEN; i++) {
- if (ptys_eth_proto & mlxsw_sp1_port_link_mode[i].mask)
+ if (ptys_eth_proto & mlxsw_sp1_port_link_mode[i].mask) {
cmd->link_mode = mlxsw_sp1_port_link_mode[i].mask_ethtool;
+ cmd->link_mode_valid = true;
+ }
}
}
@@ -1672,8 +1672,6 @@ mlxsw_sp2_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
struct mlxsw_sp2_port_link_mode link;
int i;
- cmd->link_mode = -1;
-
if (!carrier_ok)
return;
@@ -1681,6 +1679,7 @@ mlxsw_sp2_from_ptys_link_mode(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
if (ptys_eth_proto & mlxsw_sp2_port_link_mode[i].mask) {
link = mlxsw_sp2_port_link_mode[i];
cmd->link_mode = link.mask_ethtool[1];
+ cmd->link_mode_valid = true;
}
}
}
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index ec4cd3921c67..1f11577dc189 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -130,6 +130,7 @@ struct ethtool_link_ksettings {
} link_modes;
u32 lanes;
enum ethtool_link_mode_bit_indices link_mode;
+ bool link_mode_valid;
};
/**
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 24783b71c584..5aafa30f07ad 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -436,12 +436,11 @@ 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 (link_ksettings->link_mode_valid) {
link_info = &link_mode_params[link_ksettings->link_mode];
link_ksettings->base.speed = link_info->speed;
link_ksettings->lanes = link_info->lanes;
@@ -615,6 +614,8 @@ static int ethtool_set_link_ksettings(struct net_device *dev,
link_ksettings.base.master_slave_state)
return -EINVAL;
+ link_ksettings.link_mode_valid = false;
+
err = dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
if (err >= 0) {
ethtool_notify(dev, ETHTOOL_MSG_LINKINFO_NTF, NULL);
@@ -701,6 +702,7 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr)
if (!convert_legacy_settings_to_link_ksettings(&link_ksettings, &cmd))
return -EINVAL;
+ link_ksettings.link_mode_valid = false;
link_ksettings.base.link_mode_masks_nwords =
__ETHTOOL_LINK_MODE_MASK_NU32;
ret = dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
--
2.26.2
Powered by blists - more mailing lists