Re: [PATCH 6/7] net: core: Convert dev_set_mac_address() to struct sockaddr_storage

On 20/05/25 16:31, Kees Cook wrote:
> All users of dev_set_mac_address() are now using a struct sockaddr_storage.
> Convert the internal data type to struct sockaddr_storage, drop the casts,
> and update pointer types.
> 
> Signed-off-by: Kees Cook <[email protected]>

Acked-by: Gustavo A. R. Silva <[email protected]>

Thanks!
-Gustavo

> ---
> Cc: Jakub Kicinski <[email protected]>
> Cc: Jay Vosburgh <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: "K. Y. Srinivasan" <[email protected]>
> Cc: Haiyang Zhang <[email protected]>
> Cc: Wei Liu <[email protected]>
> Cc: Dexuan Cui <[email protected]>
> Cc: Jiri Pirko <[email protected]>
> Cc: Simon Horman <[email protected]>
> Cc: Alexander Aring <[email protected]>
> Cc: Stefan Schmidt <[email protected]>
> Cc: Miquel Raynal <[email protected]>
> Cc: Samuel Mendoza-Jonas <[email protected]>
> Cc: Paul Fertser <[email protected]>
> Cc: Hayes Wang <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Grant Grundler <[email protected]>
> Cc: Stanislav Fomichev <[email protected]>
> Cc: Cosmin Ratiu <[email protected]>
> Cc: Lei Yang <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> ---
>   include/linux/netdevice.h       |  2 +-
>   drivers/net/bonding/bond_alb.c  |  8 +++-----
>   drivers/net/bonding/bond_main.c | 10 ++++------
>   drivers/net/hyperv/netvsc_drv.c |  6 +++---
>   drivers/net/macvlan.c           | 10 +++++-----
>   drivers/net/team/team_core.c    |  2 +-
>   drivers/net/usb/r8152.c         |  2 +-
>   net/core/dev.c                  |  1 +
>   net/core/dev_api.c              |  6 +++---
>   net/ieee802154/nl-phy.c         |  2 +-
>   net/ncsi/ncsi-manage.c          |  2 +-
>   11 files changed, 24 insertions(+), 27 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 47200a394a02..b4242b997373 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4214,7 +4214,7 @@ int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
>   			      struct netlink_ext_ack *extack);
>   int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
>   			  struct netlink_ext_ack *extack);
> -int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
> +int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
>   			struct netlink_ext_ack *extack);
>   int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
>   			     struct netlink_ext_ack *extack);
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 7edf0fd58c34..2d37b07c8215 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -1035,7 +1035,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, const u8 addr[],
>   	 */
>   	memcpy(ss.__data, addr, len);
>   	ss.ss_family = dev->type;
> -	if (dev_set_mac_address(dev, (struct sockaddr *)&ss, NULL)) {
> +	if (dev_set_mac_address(dev, &ss, NULL)) {
>   		slave_err(slave->bond->dev, dev, "dev_set_mac_address on slave failed! ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n");
>   		return -EOPNOTSUPP;
>   	}
> @@ -1273,8 +1273,7 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
>   			break;
>   		bond_hw_addr_copy(tmp_addr, rollback_slave->dev->dev_addr,
>   				  rollback_slave->dev->addr_len);
> -		dev_set_mac_address(rollback_slave->dev,
> -				    (struct sockaddr *)&ss, NULL);
> +		dev_set_mac_address(rollback_slave->dev, &ss, NULL);
>   		dev_addr_set(rollback_slave->dev, tmp_addr);
>   	}
>   
> @@ -1763,8 +1762,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
>   				  bond->dev->addr_len);
>   		ss.ss_family = bond->dev->type;
>   		/* we don't care if it can't change its mac, best effort */
> -		dev_set_mac_address(new_slave->dev, (struct sockaddr *)&ss,
> -				    NULL);
> +		dev_set_mac_address(new_slave->dev, &ss, NULL);
>   
>   		dev_addr_set(new_slave->dev, tmp_addr);
>   	}
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 98cf4486fcee..b92e8935d686 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1112,8 +1112,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
>   			ss.ss_family = bond->dev->type;
>   		}
>   
> -		rv = dev_set_mac_address(new_active->dev,
> -					 (struct sockaddr *)&ss, NULL);
> +		rv = dev_set_mac_address(new_active->dev, &ss, NULL);
>   		if (rv) {
>   			slave_err(bond->dev, new_active->dev, "Error %d setting MAC of new active slave\n",
>   				  -rv);
> @@ -1127,8 +1126,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
>   				  new_active->dev->addr_len);
>   		ss.ss_family = old_active->dev->type;
>   
> -		rv = dev_set_mac_address(old_active->dev,
> -					 (struct sockaddr *)&ss, NULL);
> +		rv = dev_set_mac_address(old_active->dev, &ss, NULL);
>   		if (rv)
>   			slave_err(bond->dev, old_active->dev, "Error %d setting MAC of old active slave\n",
>   				  -rv);
> @@ -2455,7 +2453,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
>   		bond_hw_addr_copy(ss.__data, new_slave->perm_hwaddr,
>   				  new_slave->dev->addr_len);
>   		ss.ss_family = slave_dev->type;
> -		dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, NULL);
> +		dev_set_mac_address(slave_dev, &ss, NULL);
>   	}
>   
>   err_restore_mtu:
> @@ -2649,7 +2647,7 @@ static int __bond_release_one(struct net_device *bond_dev,
>   		bond_hw_addr_copy(ss.__data, slave->perm_hwaddr,
>   				  slave->dev->addr_len);
>   		ss.ss_family = slave_dev->type;
> -		dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, NULL);
> +		dev_set_mac_address(slave_dev, &ss, NULL);
>   	}
>   
>   	if (unregister) {
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index d8b169ac0343..14a0d04e21ae 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1371,7 +1371,7 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
>   	struct net_device_context *ndc = netdev_priv(ndev);
>   	struct net_device *vf_netdev = rtnl_dereference(ndc->vf_netdev);
>   	struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
> -	struct sockaddr *addr = p;
> +	struct sockaddr_storage *addr = p;
>   	int err;
>   
>   	err = eth_prepare_mac_addr_change(ndev, p);
> @@ -1387,12 +1387,12 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
>   			return err;
>   	}
>   
> -	err = rndis_filter_set_device_mac(nvdev, addr->sa_data);
> +	err = rndis_filter_set_device_mac(nvdev, addr->__data);
>   	if (!err) {
>   		eth_commit_mac_addr_change(ndev, p);
>   	} else if (vf_netdev) {
>   		/* rollback change on VF */
> -		memcpy(addr->sa_data, ndev->dev_addr, ETH_ALEN);
> +		memcpy(addr->__data, ndev->dev_addr, ETH_ALEN);
>   		dev_set_mac_address(vf_netdev, addr, NULL);
>   	}
>   
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 7045b1d58754..69e879780c36 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -754,13 +754,13 @@ static int macvlan_sync_address(struct net_device *dev,
>   static int macvlan_set_mac_address(struct net_device *dev, void *p)
>   {
>   	struct macvlan_dev *vlan = netdev_priv(dev);
> -	struct sockaddr *addr = p;
> +	struct sockaddr_storage *addr = p;
>   
> -	if (!is_valid_ether_addr(addr->sa_data))
> +	if (!is_valid_ether_addr(addr->__data))
>   		return -EADDRNOTAVAIL;
>   
>   	/* If the addresses are the same, this is a no-op */
> -	if (ether_addr_equal(dev->dev_addr, addr->sa_data))
> +	if (ether_addr_equal(dev->dev_addr, addr->__data))
>   		return 0;
>   
>   	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> @@ -768,10 +768,10 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p)
>   		return dev_set_mac_address(vlan->lowerdev, addr, NULL);
>   	}
>   
> -	if (macvlan_addr_busy(vlan->port, addr->sa_data))
> +	if (macvlan_addr_busy(vlan->port, addr->__data))
>   		return -EADDRINUSE;
>   
> -	return macvlan_sync_address(dev, addr->sa_data);
> +	return macvlan_sync_address(dev, addr->__data);
>   }
>   
>   static void macvlan_change_rx_flags(struct net_device *dev, int change)
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index d8fc0c79745d..a64e661c21a1 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -55,7 +55,7 @@ static int __set_port_dev_addr(struct net_device *port_dev,
>   
>   	memcpy(addr.__data, dev_addr, port_dev->addr_len);
>   	addr.ss_family = port_dev->type;
> -	return dev_set_mac_address(port_dev, (struct sockaddr *)&addr, NULL);
> +	return dev_set_mac_address(port_dev, &addr, NULL);
>   }
>   
>   static int team_port_set_orig_dev_addr(struct team_port *port)
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index b18dee1b1bb3..d6589b24c68d 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -8432,7 +8432,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
>   
>   	/* reset the MAC address in case of policy change */
>   	if (determine_ethernet_addr(tp, &ss) >= 0)
> -		dev_set_mac_address(tp->netdev, (struct sockaddr *)&ss, NULL);
> +		dev_set_mac_address(tp->netdev, &ss, NULL);
>   
>   	netdev = tp->netdev;
>   	if (!netif_running(netdev))
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f8c8aad7df2e..1f1900ec26b2 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9683,6 +9683,7 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
>   
>   DECLARE_RWSEM(dev_addr_sem);
>   
> +/* "sa" is a true struct sockaddr with limited "sa_data" member. */
>   int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name)
>   {
>   	size_t size = sizeof(sa->sa_data_min);
> diff --git a/net/core/dev_api.c b/net/core/dev_api.c
> index b5f293e637d9..e80404e76ca9 100644
> --- a/net/core/dev_api.c
> +++ b/net/core/dev_api.c
> @@ -319,20 +319,20 @@ EXPORT_SYMBOL(dev_set_allmulti);
>   /**
>    * dev_set_mac_address() - change Media Access Control Address
>    * @dev: device
> - * @sa: new address
> + * @ss: new address
>    * @extack: netlink extended ack
>    *
>    * Change the hardware (MAC) address of the device
>    *
>    * Return: 0 on success, -errno on failure.
>    */
> -int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
> +int dev_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
>   			struct netlink_ext_ack *extack)
>   {
>   	int ret;
>   
>   	netdev_lock_ops(dev);
> -	ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
> +	ret = netif_set_mac_address(dev, sa, extack);
>   	netdev_unlock_ops(dev);
>   
>   	return ret;
> diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
> index ee2b190e8e0d..4c07a475c567 100644
> --- a/net/ieee802154/nl-phy.c
> +++ b/net/ieee802154/nl-phy.c
> @@ -234,7 +234,7 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
>   		 * dev_set_mac_address require RTNL_LOCK
>   		 */
>   		rtnl_lock();
> -		rc = dev_set_mac_address(dev, (struct sockaddr *)&addr, NULL);
> +		rc = dev_set_mac_address(dev, &addr, NULL);
>   		rtnl_unlock();
>   		if (rc)
>   			goto dev_unregister;
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 0202db2aea3e..b36947063783 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1058,7 +1058,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>   		break;
>   	case ncsi_dev_state_config_apply_mac:
>   		rtnl_lock();
> -		ret = dev_set_mac_address(dev, (struct sockaddr *)&ndp->pending_mac, NULL);
> +		ret = dev_set_mac_address(dev, &ndp->pending_mac, NULL);
>   		rtnl_unlock();
>   		if (ret < 0)
>   			netdev_warn(dev, "NCSI: 'Writing MAC address to device failed\n");