Commit cc26c266 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

net: fix data-race in dev_isalive()

dev_isalive() is called under RTNL or dev_base_lock protection.

This means that changes to dev->reg_state should be done with both locks held.

syzbot reported:

BUG: KCSAN: data-race in register_netdevice / type_show

write to 0xffff888144ecf518 of 1 bytes by task 20886 on cpu 0:
register_netdevice+0xb9f/0xdf0 net/core/dev.c:10050
lapbeth_new_device drivers/net/wan/lapbether.c:414 [inline]
lapbeth_device_event+0x4a0/0x6c0 drivers/net/wan/lapbether.c:456
notifier_call_chain kernel/notifier.c:87 [inline]
raw_notifier_call_chain+0x53/0xb0 kernel/notifier.c:455
__dev_notify_flags+0x1d6/0x3a0
dev_change_flags+0xa2/0xc0 net/core/dev.c:8607
do_setlink+0x778/0x2230 net/core/rtnetlink.c:2780
__rtnl_newlink net/core/rtnetlink.c:3546 [inline]
rtnl_newlink+0x114c/0x16a0 net/core/rtnetlink.c:3593
rtnetlink_rcv_msg+0x811/0x8c0 net/core/rtnetlink.c:6089
netlink_rcv_skb+0x13e/0x240 net/netlink/af_netlink.c:2501
rtnetlink_rcv+0x18/0x20 net/core/rtnetlink.c:6107
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x58a/0x660 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x661/0x750 net/netlink/af_netlink.c:1921
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg net/socket.c:734 [inline]
__sys_sendto+0x21e/0x2c0 net/socket.c:2119
__do_sys_sendto net/socket.c:2131 [inline]
__se_sys_sendto net/socket.c:2127 [inline]
__x64_sys_sendto+0x74/0x90 net/socket.c:2127
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x46/0xb0

read to 0xffff888144ecf518 of 1 bytes by task 20423 on cpu 1:
dev_isalive net/core/net-sysfs.c:38 [inline]
netdev_show net/core/net-sysfs.c:50 [inline]
type_show+0x24/0x90 net/core/net-sysfs.c:112
dev_attr_show+0x35/0x90 drivers/base/core.c:2095
sysfs_kf_seq_show+0x175/0x240 fs/sysfs/file.c:59
kernfs_seq_show+0x75/0x80 fs/kernfs/file.c:162
seq_read_iter+0x2c3/0x8e0 fs/seq_file.c:230
kernfs_fop_read_iter+0xd1/0x2f0 fs/kernfs/file.c:235
call_read_iter include/linux/fs.h:2052 [inline]
new_sync_read fs/read_write.c:401 [inline]
vfs_read+0x5a5/0x6a0 fs/read_write.c:482
ksys_read+0xe8/0x1a0 fs/read_write.c:620
__do_sys_read fs/read_write.c:630 [inline]
__se_sys_read fs/read_write.c:628 [inline]
__x64_sys_read+0x3e/0x50 fs/read_write.c:628
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x46/0xb0

value changed: 0x00 -> 0x01

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 20423 Comm: udevd Tainted: G W 5.19.0-rc2-syzkaller-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

Fixes: 1da177e4 ("Linux-2.6.12-rc2")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 9b7fd167
...@@ -397,16 +397,18 @@ static void list_netdevice(struct net_device *dev) ...@@ -397,16 +397,18 @@ static void list_netdevice(struct net_device *dev)
/* Device list removal /* Device list removal
* caller must respect a RCU grace period before freeing/reusing dev * caller must respect a RCU grace period before freeing/reusing dev
*/ */
static void unlist_netdevice(struct net_device *dev) static void unlist_netdevice(struct net_device *dev, bool lock)
{ {
ASSERT_RTNL(); ASSERT_RTNL();
/* Unlink dev from the device chain */ /* Unlink dev from the device chain */
write_lock(&dev_base_lock); if (lock)
write_lock(&dev_base_lock);
list_del_rcu(&dev->dev_list); list_del_rcu(&dev->dev_list);
netdev_name_node_del(dev->name_node); netdev_name_node_del(dev->name_node);
hlist_del_rcu(&dev->index_hlist); hlist_del_rcu(&dev->index_hlist);
write_unlock(&dev_base_lock); if (lock)
write_unlock(&dev_base_lock);
dev_base_seq_inc(dev_net(dev)); dev_base_seq_inc(dev_net(dev));
} }
...@@ -10043,11 +10045,11 @@ int register_netdevice(struct net_device *dev) ...@@ -10043,11 +10045,11 @@ int register_netdevice(struct net_device *dev)
goto err_uninit; goto err_uninit;
ret = netdev_register_kobject(dev); ret = netdev_register_kobject(dev);
if (ret) { write_lock(&dev_base_lock);
dev->reg_state = NETREG_UNREGISTERED; dev->reg_state = ret ? NETREG_UNREGISTERED : NETREG_REGISTERED;
write_unlock(&dev_base_lock);
if (ret)
goto err_uninit; goto err_uninit;
}
dev->reg_state = NETREG_REGISTERED;
__netdev_update_features(dev); __netdev_update_features(dev);
...@@ -10329,7 +10331,9 @@ void netdev_run_todo(void) ...@@ -10329,7 +10331,9 @@ void netdev_run_todo(void)
continue; continue;
} }
write_lock(&dev_base_lock);
dev->reg_state = NETREG_UNREGISTERED; dev->reg_state = NETREG_UNREGISTERED;
write_unlock(&dev_base_lock);
linkwatch_forget_dev(dev); linkwatch_forget_dev(dev);
} }
...@@ -10810,9 +10814,10 @@ void unregister_netdevice_many(struct list_head *head) ...@@ -10810,9 +10814,10 @@ void unregister_netdevice_many(struct list_head *head)
list_for_each_entry(dev, head, unreg_list) { list_for_each_entry(dev, head, unreg_list) {
/* And unlink it from device chain. */ /* And unlink it from device chain. */
unlist_netdevice(dev); write_lock(&dev_base_lock);
unlist_netdevice(dev, false);
dev->reg_state = NETREG_UNREGISTERING; dev->reg_state = NETREG_UNREGISTERING;
write_unlock(&dev_base_lock);
} }
flush_all_backlogs(); flush_all_backlogs();
...@@ -10959,7 +10964,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net, ...@@ -10959,7 +10964,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
dev_close(dev); dev_close(dev);
/* And unlink it from device chain */ /* And unlink it from device chain */
unlist_netdevice(dev); unlist_netdevice(dev, true);
synchronize_net(); synchronize_net();
......
...@@ -33,6 +33,7 @@ static const char fmt_dec[] = "%d\n"; ...@@ -33,6 +33,7 @@ static const char fmt_dec[] = "%d\n";
static const char fmt_ulong[] = "%lu\n"; static const char fmt_ulong[] = "%lu\n";
static const char fmt_u64[] = "%llu\n"; static const char fmt_u64[] = "%llu\n";
/* Caller holds RTNL or dev_base_lock */
static inline int dev_isalive(const struct net_device *dev) static inline int dev_isalive(const struct net_device *dev)
{ {
return dev->reg_state <= NETREG_REGISTERED; return dev->reg_state <= NETREG_REGISTERED;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment