forked from mirrors/linux
		
	net: replace dev_addr_sem with netdev instance lock
Lockdep reports possible circular dependency in [0]. Instead of fixing the ordering, replace global dev_addr_sem with netdev instance lock. Most of the paths that set/get mac are RTNL protected. Two places where it's not, convert to explicit locking: - sysfs address_show - dev_get_mac_address via dev_ioctl 0: https://netdev-3.bots.linux.dev/vmksft-forwarding-dbg/results/993321/24-router-bridge-1d-lag-sh/stderr Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> Link: https://patch.msgid.link/20250305163732.2766420-12-sdf@fomichev.me Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
		
							parent
							
								
									2bcf4772e4
								
							
						
					
					
						commit
						df43d8bf10
					
				
					 9 changed files with 41 additions and 56 deletions
				
			
		|  | @ -1017,7 +1017,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd, | |||
| 			rtnl_unlock(); | ||||
| 			return -ENOLINK; | ||||
| 		} | ||||
| 		ret = dev_set_mac_address_user(tap->dev, &sa, NULL); | ||||
| 		ret = dev_set_mac_address(tap->dev, &sa, NULL); | ||||
| 		tap_put_tap_dev(tap); | ||||
| 		rtnl_unlock(); | ||||
| 		return ret; | ||||
|  |  | |||
|  | @ -3175,7 +3175,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd, | |||
| 
 | ||||
| 	case SIOCSIFHWADDR: | ||||
| 		/* Set hw address */ | ||||
| 		ret = dev_set_mac_address_user(tun->dev, &ifr.ifr_hwaddr, NULL); | ||||
| 		ret = dev_set_mac_address(tun->dev, &ifr.ifr_hwaddr, NULL); | ||||
| 		break; | ||||
| 
 | ||||
| 	case TUNGETSNDBUF: | ||||
|  |  | |||
|  | @ -2492,7 +2492,7 @@ struct net_device { | |||
| 	 * | ||||
| 	 * Protects: | ||||
| 	 *	@gro_flush_timeout, @napi_defer_hard_irqs, @napi_list, | ||||
| 	 *	@net_shaper_hierarchy, @reg_state, @threaded | ||||
| 	 *	@net_shaper_hierarchy, @reg_state, @threaded, @dev_addr | ||||
| 	 * | ||||
| 	 * Partially protects (writers must hold both @lock and rtnl_lock): | ||||
| 	 *	@up | ||||
|  | @ -4262,10 +4262,6 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa, | |||
| 			  struct netlink_ext_ack *extack); | ||||
| int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa, | ||||
| 			struct netlink_ext_ack *extack); | ||||
| int netif_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, | ||||
| 			       struct netlink_ext_ack *extack); | ||||
| int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, | ||||
| 			     struct netlink_ext_ack *extack); | ||||
| int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name); | ||||
| int dev_get_port_parent_id(struct net_device *dev, | ||||
| 			   struct netdev_phys_item_id *ppid, bool recurse); | ||||
|  |  | |||
|  | @ -1058,6 +1058,28 @@ struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex) | |||
| 	return __netdev_put_lock(dev); | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  * netdev_get_by_name_lock() - find a device by its name | ||||
|  * @net: the applicable net namespace | ||||
|  * @name: name of device | ||||
|  * | ||||
|  * Search for an interface by name. If a valid device | ||||
|  * with @name is found it will be returned with netdev->lock held. | ||||
|  * netdev_unlock() must be called to release it. | ||||
|  * | ||||
|  * Return: pointer to a device with lock held, NULL if not found. | ||||
|  */ | ||||
| struct net_device *netdev_get_by_name_lock(struct net *net, const char *name) | ||||
| { | ||||
| 	struct net_device *dev; | ||||
| 
 | ||||
| 	dev = dev_get_by_name(net, name); | ||||
| 	if (!dev) | ||||
| 		return NULL; | ||||
| 
 | ||||
| 	return __netdev_put_lock(dev); | ||||
| } | ||||
| 
 | ||||
| struct net_device * | ||||
| netdev_xa_find_lock(struct net *net, struct net_device *dev, | ||||
| 		    unsigned long *index) | ||||
|  | @ -9589,44 +9611,24 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa, | |||
| 	return 0; | ||||
| } | ||||
| 
 | ||||
| DECLARE_RWSEM(dev_addr_sem); | ||||
| 
 | ||||
| int netif_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, | ||||
| 			       struct netlink_ext_ack *extack) | ||||
| { | ||||
| 	int ret; | ||||
| 
 | ||||
| 	down_write(&dev_addr_sem); | ||||
| 	ret = netif_set_mac_address(dev, sa, extack); | ||||
| 	up_write(&dev_addr_sem); | ||||
| 	return ret; | ||||
| } | ||||
| 
 | ||||
| int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name) | ||||
| { | ||||
| 	size_t size = sizeof(sa->sa_data_min); | ||||
| 	struct net_device *dev; | ||||
| 	int ret = 0; | ||||
| 
 | ||||
| 	down_read(&dev_addr_sem); | ||||
| 	rcu_read_lock(); | ||||
| 	dev = netdev_get_by_name_lock(net, dev_name); | ||||
| 	if (!dev) | ||||
| 		return -ENODEV; | ||||
| 
 | ||||
| 	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; | ||||
| 	netdev_unlock(dev); | ||||
| 
 | ||||
| unlock: | ||||
| 	rcu_read_unlock(); | ||||
| 	up_read(&dev_addr_sem); | ||||
| 	return ret; | ||||
| 	return 0; | ||||
| } | ||||
| EXPORT_SYMBOL(dev_get_mac_address); | ||||
| 
 | ||||
|  |  | |||
|  | @ -28,6 +28,7 @@ netdev_napi_by_id_lock(struct net *net, unsigned int napi_id); | |||
| struct net_device *dev_get_by_napi_id(unsigned int napi_id); | ||||
| 
 | ||||
| struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex); | ||||
| struct net_device *netdev_get_by_name_lock(struct net *net, const char *name); | ||||
| struct net_device *__netdev_put_lock(struct net_device *dev); | ||||
| struct net_device * | ||||
| netdev_xa_find_lock(struct net *net, struct net_device *dev, | ||||
|  | @ -69,8 +70,6 @@ extern int		weight_p; | |||
| extern int		dev_weight_rx_bias; | ||||
| extern int		dev_weight_tx_bias; | ||||
| 
 | ||||
| extern struct rw_semaphore dev_addr_sem; | ||||
| 
 | ||||
| /* rtnl helpers */ | ||||
| extern struct list_head net_todo_list; | ||||
| void netdev_run_todo(void); | ||||
|  |  | |||
|  | @ -82,19 +82,6 @@ void dev_set_group(struct net_device *dev, int new_group) | |||
| 	netdev_unlock_ops(dev); | ||||
| } | ||||
| 
 | ||||
| int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa, | ||||
| 			     struct netlink_ext_ack *extack) | ||||
| { | ||||
| 	int ret; | ||||
| 
 | ||||
| 	netdev_lock_ops(dev); | ||||
| 	ret = netif_set_mac_address_user(dev, sa, extack); | ||||
| 	netdev_unlock_ops(dev); | ||||
| 
 | ||||
| 	return ret; | ||||
| } | ||||
| EXPORT_SYMBOL(dev_set_mac_address_user); | ||||
| 
 | ||||
| /**
 | ||||
|  * dev_change_net_namespace() - move device to different nethost namespace | ||||
|  * @dev: device | ||||
|  | @ -310,9 +297,9 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa, | |||
| { | ||||
| 	int ret; | ||||
| 
 | ||||
| 	netdev_lock_ops(dev); | ||||
| 	netdev_lock(dev); | ||||
| 	ret = netif_set_mac_address(dev, sa, extack); | ||||
| 	netdev_unlock_ops(dev); | ||||
| 	netdev_unlock(dev); | ||||
| 
 | ||||
| 	return ret; | ||||
| } | ||||
|  |  | |||
|  | @ -574,7 +574,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, | |||
| 	case SIOCSIFHWADDR: | ||||
| 		if (dev->addr_len > sizeof(struct sockaddr)) | ||||
| 			return -EINVAL; | ||||
| 		return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL); | ||||
| 		return dev_set_mac_address(dev, &ifr->ifr_hwaddr, NULL); | ||||
| 
 | ||||
| 	case SIOCSIFHWBROADCAST: | ||||
| 		if (ifr->ifr_hwaddr.sa_family != dev->type) | ||||
|  |  | |||
|  | @ -262,14 +262,11 @@ static ssize_t address_show(struct device *dev, struct device_attribute *attr, | |||
| 	struct net_device *ndev = to_net_dev(dev); | ||||
| 	ssize_t ret = -EINVAL; | ||||
| 
 | ||||
| 	down_read(&dev_addr_sem); | ||||
| 
 | ||||
| 	rcu_read_lock(); | ||||
| 	netdev_lock(ndev); | ||||
| 	if (dev_isalive(ndev)) | ||||
| 		ret = sysfs_format_mac(buf, ndev->dev_addr, ndev->addr_len); | ||||
| 	rcu_read_unlock(); | ||||
| 	netdev_unlock(ndev); | ||||
| 
 | ||||
| 	up_read(&dev_addr_sem); | ||||
| 	return ret; | ||||
| } | ||||
| static DEVICE_ATTR_RO(address); | ||||
|  |  | |||
|  | @ -3089,7 +3089,11 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev, | |||
| 		sa->sa_family = dev->type; | ||||
| 		memcpy(sa->sa_data, nla_data(tb[IFLA_ADDRESS]), | ||||
| 		       dev->addr_len); | ||||
| 		err = netif_set_mac_address_user(dev, sa, extack); | ||||
| 		if (!netdev_need_ops_lock(dev)) | ||||
| 			netdev_lock(dev); | ||||
| 		err = netif_set_mac_address(dev, sa, extack); | ||||
| 		if (!netdev_need_ops_lock(dev)) | ||||
| 			netdev_unlock(dev); | ||||
| 		kfree(sa); | ||||
| 		if (err) | ||||
| 			goto errout; | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Stanislav Fomichev
						Stanislav Fomichev