diff options
| author | Cong Wang <[email protected]> | 2021-02-11 11:34:10 -0800 | 
|---|---|---|
| committer | David S. Miller <[email protected]> | 2021-02-11 18:14:19 -0800 | 
| commit | 3b23a32a63219f51a5298bc55a65ecee866e79d0 (patch) | |
| tree | 79f9a923b4679addfc28e39340e9834b922f7e40 /net/core/dev.c | |
| parent | 7867299cde34e9c2d2c676f2a384a9d5853b914d (diff) | |
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" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Signed-off-by: Cong Wang <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Diffstat (limited to 'net/core/dev.c')
| -rw-r--r-- | net/core/dev.c | 42 | 
1 files changed, 42 insertions, 0 deletions
| diff --git a/net/core/dev.c b/net/core/dev.c index 321d41a110e7..ce6291bc2e16 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -8937,6 +8937,48 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,  }  EXPORT_SYMBOL(dev_set_mac_address); +static DECLARE_RWSEM(dev_addr_sem); + +int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, +			     struct netlink_ext_ack *extack) +{ +	int ret; + +	down_write(&dev_addr_sem); +	ret = dev_set_mac_address(dev, sa, extack); +	up_write(&dev_addr_sem); +	return ret; +} +EXPORT_SYMBOL(dev_set_mac_address_user); + +int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name) +{ +	size_t size = sizeof(sa->sa_data); +	struct net_device *dev; +	int ret = 0; + +	down_read(&dev_addr_sem); +	rcu_read_lock(); + +	dev = dev_get_by_name_rcu(net, dev_name); +	if (!dev) { +		ret = -ENODEV; +		goto unlock; +	} +	if (!dev->addr_len) +		memset(sa->sa_data, 0, size); +	else +		memcpy(sa->sa_data, dev->dev_addr, +		       min_t(size_t, size, dev->addr_len)); +	sa->sa_family = dev->type; + +unlock: +	rcu_read_unlock(); +	up_read(&dev_addr_sem); +	return ret; +} +EXPORT_SYMBOL(dev_get_mac_address); +  /**   *	dev_change_carrier - Change device carrier   *	@dev: device |