Commit df43d8bf authored by Stanislav Fomichev's avatar Stanislav Fomichev Committed by Jakub Kicinski
Browse files

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: default avatarStanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250305163732.2766420-12-sdf@fomichev.me


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 2bcf4772

drivers/net/tap.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

drivers/net/tun.c

+1 −1
Original line number Diff line number Diff line
@@ -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:

include/linux/netdevice.h

+1 −5
Original line number Diff line number Diff line
@@ -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);

net/core/dev.c

+27 −25
Original line number Diff line number Diff line
@@ -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);

net/core/dev.h

+1 −2
Original line number Diff line number Diff line
@@ -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);