net: fix dev_ifsioc_locked() race condition
authorCong Wang <cong.wang@bytedance.com>
Thu, 11 Feb 2021 19:34:10 +0000 (11:34 -0800)
committerDavid S. Miller <davem@davemloft.net>
Fri, 12 Feb 2021 02:14:19 +0000 (18:14 -0800)
commit3b23a32a63219f51a5298bc55a65ecee866e79d0
tree79f9a923b4679addfc28e39340e9834b922f7e40
parent7867299cde34e9c2d2c676f2a384a9d5853b914d
net: fix dev_ifsioc_locked() race condition

dev_ifsioc_locked() is called with only RCU read lock, so when
there is a parallel writer changing the mac address, it could
get a partially updated mac address, as shown below:

Thread 1 Thread 2
// eth_commit_mac_addr_change()
memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
// dev_ifsioc_locked()
memcpy(ifr->ifr_hwaddr.sa_data,
dev->dev_addr,...);

Close this race condition by guarding them with a RW semaphore,
like netdev_get_name(). We can not use seqlock here as it does not
allow blocking. The writers already take RTNL anyway, so this does
not affect the slow path. To avoid bothering existing
dev_set_mac_address() callers in drivers, introduce a new wrapper
just for user-facing callers on ioctl and rtnetlink paths.

Note, bonding also changes slave mac addresses but that requires
a separate patch due to the complexity of bonding code.

Fixes: 3710becf8a58 ("net: RCU locking for simple ioctl()")
Reported-by: "Gong, Sishuai" <sishuai@purdue.edu>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/tap.c
drivers/net/tun.c
include/linux/netdevice.h
net/core/dev.c
net/core/dev_ioctl.c
net/core/rtnetlink.c