net: Convert dev_set_mac_address() to struct sockaddr_storage

All the callers of inet_addr_is_any() have a sockaddr_storage-backed sockaddr. Avoid casts and switch prototype to the actual object being used.

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

Signed-off-by Kees Cook <[email protected]>

All the callers of inet_addr_is_any() have a sockaddr_storage-backed sockaddr. Avoid casts and switch prototype to the actual object being used.

In order to avoid passing around struct sockaddr that has a size the compiler cannot reason about (nor track at runtime), convert netif_set_mac_address() to take struct sockaddr_storage. This is just a cast conversion, so there is are no binary changes. Following patches will make actual allocation changes.

Signed-off-by Kees Cook <[email protected]>

---
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Stanislav Fomichev <[email protected]>
Cc: Cosmin Ratiu <[email protected]>
Cc: Lei Yang <[email protected]>
Cc: Kuniyuki Iwashima <[email protected]>
Cc: Ido Schimmel <[email protected]>
Cc: <[email protected]>

---
  include/linux/netdevice.h |  2 +-
  net/core/dev.c            | 10 +++++-----
  net/core/dev_api.c        |  4 ++--
  net/core/rtnetlink.c      |  2 +-
  4 files changed, 9 insertions(+), 9 deletions(-)

include/linux/netdevice.h +1 -1

index ea9d335de130..47200a394a02 100644
@@ -4212,7 +4212,7 @@ int netif_set_mtu(struct net_device *dev, int new_mtu);
  int dev_set_mtu(struct net_device *, int);
  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 *sa,
+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,
  			struct netlink_ext_ack *extack);

net/core/dev.c +5 -5

index fccf2167b235..f8c8aad7df2e 100644
@@ -9655,7 +9655,7 @@ int dev_pre_changeaddr_notify(struct net_device *dev, const char *addr,
  }
  EXPORT_SYMBOL(dev_pre_changeaddr_notify);
 
-int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa,
+int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
  			  struct netlink_ext_ack *extack)
  {
  	const struct net_device_ops *ops = dev->netdev_ops;

@@ -9663,15 +9663,15 @@ int netif_set_mac_address(struct net_device *dev, struct sockaddr *sa,
 
  	if (!ops->ndo_set_mac_address)
  		return -EOPNOTSUPP;
-	if (sa->sa_family != dev->type)
+	if (ss->ss_family != dev->type)
  		return -EINVAL;
  	if (!netif_device_present(dev))
  		return -ENODEV;
-	err = dev_pre_changeaddr_notify(dev, sa->sa_data, extack);
+	err = dev_pre_changeaddr_notify(dev, ss->__data, extack);
  	if (err)
  		return err;
-	if (memcmp(dev->dev_addr, sa->sa_data, dev->addr_len)) {
-		err = ops->ndo_set_mac_address(dev, sa);
+	if (memcmp(dev->dev_addr, ss->__data, dev->addr_len)) {
+		err = ops->ndo_set_mac_address(dev, ss);
  		if (err)
  			return err;
  	}

net/core/dev_api.c +2 -2

index f9a160ab596f..b5f293e637d9 100644
@@ -91,7 +91,7 @@ int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
 
  	down_write(&dev_addr_sem);
  	netdev_lock_ops(dev);
-	ret = netif_set_mac_address(dev, sa, extack);
+	ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
  	netdev_unlock_ops(dev);
  	up_write(&dev_addr_sem);
 

@@ -332,7 +332,7 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
  	int ret;
 
  	netdev_lock_ops(dev);
-	ret = netif_set_mac_address(dev, sa, extack);
+	ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
  	netdev_unlock_ops(dev);
 
  	return ret;

net/core/rtnetlink.c +1 -1

index 8a914b37ef6e..9743f1c2ae3c 100644
@@ -3100,7 +3100,7 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
 
  		memcpy(sa->sa_data, nla_data(tb[IFLA_ADDRESS]),
  		       dev->addr_len);
-		err = netif_set_mac_address(dev, sa, extack);
+		err = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
  		kfree(sa);
  		if (err) {
  			up_write(&dev_addr_sem);

To avoid future casting with coming API type changes, switch struct ncsi_dev_priv::pending_mac to a full struct sockaddr_storage.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Samuel Mendoza-Jonas <[email protected]>
Cc: Paul Fertser <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: <[email protected]>

---
  net/ncsi/internal.h    |  2 +-
  net/ncsi/ncsi-manage.c |  2 +-
  net/ncsi/ncsi-rsp.c    | 18 +++++++++---------
  3 files changed, 11 insertions(+), 11 deletions(-)

net/ncsi/internal.h +1 -1

index 2c260f33b55c..e76c6de0c784 100644
@@ -322,7 +322,7 @@ struct ncsi_dev_priv {
  #define NCSI_DEV_RESHUFFLE	4
  #define NCSI_DEV_RESET		8            /* Reset state of NC          */
  	unsigned int        gma_flag;        /* OEM GMA flag               */
-	struct sockaddr     pending_mac;     /* MAC address received from GMA */
+	struct sockaddr_storage pending_mac; /* MAC address received from GMA */
  	spinlock_t          lock;            /* Protect the NCSI device    */
  	unsigned int        package_probe_id;/* Current ID during probe    */
  	unsigned int        package_num;     /* Number of packages         */

net/ncsi/ncsi-manage.c +1 -1

index b36947063783..0202db2aea3e 100644
@@ -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, &ndp->pending_mac, NULL);
+		ret = dev_set_mac_address(dev, (struct sockaddr *)&ndp->pending_mac, NULL);
  		rtnl_unlock();
  		if (ret < 0)
  			netdev_warn(dev, "NCSI: 'Writing MAC address to device failed\n");

net/ncsi/ncsi-rsp.c +9 -9

index 8668888c5a2f..472cc68ad86f 100644
@@ -628,7 +628,7 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
  static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
  {
  	struct ncsi_dev_priv *ndp = nr->ndp;
-	struct sockaddr *saddr = &ndp->pending_mac;
+	struct sockaddr_storage *saddr = &ndp->pending_mac;
  	struct net_device *ndev = ndp->ndev.dev;
  	struct ncsi_rsp_oem_pkt *rsp;
  	u32 mac_addr_off = 0;

@@ -644,11 +644,11 @@ static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
  	else if (mfr_id == NCSI_OEM_MFR_INTEL_ID)
  		mac_addr_off = INTEL_MAC_ADDR_OFFSET;
 
-	saddr->sa_family = ndev->type;
-	memcpy(saddr->sa_data, &rsp->data[mac_addr_off], ETH_ALEN);
+	saddr->ss_family = ndev->type;
+	memcpy(saddr->__data, &rsp->data[mac_addr_off], ETH_ALEN);
  	if (mfr_id == NCSI_OEM_MFR_BCM_ID || mfr_id == NCSI_OEM_MFR_INTEL_ID)
-		eth_addr_inc((u8 *)saddr->sa_data);
-	if (!is_valid_ether_addr((const u8 *)saddr->sa_data))
+		eth_addr_inc(saddr->__data);
+	if (!is_valid_ether_addr(saddr->__data))
  		return -ENXIO;
 
  	/* Set the flag for GMA command which should only be called once */

@@ -1088,7 +1088,7 @@ static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
  static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
  {
  	struct ncsi_dev_priv *ndp = nr->ndp;
-	struct sockaddr *saddr = &ndp->pending_mac;
+	struct sockaddr_storage *saddr = &ndp->pending_mac;
  	struct net_device *ndev = ndp->ndev.dev;
  	struct ncsi_rsp_gmcma_pkt *rsp;
  	int i;

@@ -1105,15 +1105,15 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
  			    rsp->addresses[i][4], rsp->addresses[i][5]);
  	}
 
-	saddr->sa_family = ndev->type;
+	saddr->ss_family = ndev->type;
  	for (i = 0; i < rsp->address_count; i++) {
  		if (!is_valid_ether_addr(rsp->addresses[i])) {
  			netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n",
  				    rsp->addresses[i]);
  			continue;
  		}
-		memcpy(saddr->sa_data, rsp->addresses[i], ETH_ALEN);
-		netdev_warn(ndev, "NCSI: Will set MAC address to %pM\n", saddr->sa_data);
+		memcpy(saddr->__data, rsp->addresses[i], ETH_ALEN);
+		netdev_warn(ndev, "NCSI: Will set MAC address to %pM\n", saddr->__data);
  		break;
  	}
 

Switch to struct sockaddr_storage for calling dev_set_mac_address(). Add a temporary cast to struct sockaddr, which will be removed in a subsequent patch.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Alexander Aring <[email protected]>
Cc: Stefan Schmidt <[email protected]>
Cc: Miquel Raynal <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>

---
  net/ieee802154/nl-phy.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

net/ieee802154/nl-phy.c +4 -4

index 359249ab77bf..ee2b190e8e0d 100644
@@ -224,17 +224,17 @@ int ieee802154_add_iface(struct sk_buff *skb, struct genl_info *info)
  	dev_hold(dev);
 
  	if (info->attrs[IEEE802154_ATTR_HW_ADDR]) {
-		struct sockaddr addr;
+		struct sockaddr_storage addr;
 
-		addr.sa_family = ARPHRD_IEEE802154;
-		nla_memcpy(&addr.sa_data, info->attrs[IEEE802154_ATTR_HW_ADDR],
+		addr.ss_family = ARPHRD_IEEE802154;
+		nla_memcpy(&addr.__data, info->attrs[IEEE802154_ATTR_HW_ADDR],
  			   IEEE802154_ADDR_LEN);
 
  		/* strangely enough, some callbacks (inetdev_event) from
  		 * dev_set_mac_address require RTNL_LOCK
  		 */
  		rtnl_lock();
-		rc = dev_set_mac_address(dev, &addr, NULL);
+		rc = dev_set_mac_address(dev, (struct sockaddr *)&addr, NULL);
  		rtnl_unlock();
  		if (rc)
  			goto dev_unregister;

To support coming API type changes, switch to sockaddr_storage usage internally.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Andrew Lunn <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Hayes Wang <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Grant Grundler <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>

---
  drivers/net/usb/r8152.c | 52 +++++++++++++++++++++--------------------
  1 file changed, 27 insertions(+), 25 deletions(-)

drivers/net/usb/r8152.c +27 -25

index 67f5d30ffcba..b18dee1b1bb3 100644
@@ -1665,14 +1665,14 @@ static int
  rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
  		  u32 advertising);
 
-static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
+static int __rtl8152_set_mac_address(struct net_device *netdev,
+				     struct sockaddr_storage *addr,
  				     bool in_resume)
  {
  	struct r8152 *tp = netdev_priv(netdev);
-	struct sockaddr *addr = p;
  	int ret = -EADDRNOTAVAIL;
 
-	if (!is_valid_ether_addr(addr->sa_data))
+	if (!is_valid_ether_addr(addr->__data))
  		goto out1;
 
  	if (!in_resume) {

@@ -1683,10 +1683,10 @@ static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
 
  	mutex_lock(&tp->control);
 
-	eth_hw_addr_set(netdev, addr->sa_data);
+	eth_hw_addr_set(netdev, addr->__data);
 
  	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
-	pla_ocp_write(tp, PLA_IDR, BYTE_EN_SIX_BYTES, 8, addr->sa_data);
+	pla_ocp_write(tp, PLA_IDR, BYTE_EN_SIX_BYTES, 8, addr->__data);
  	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
 
  	mutex_unlock(&tp->control);

@@ -1706,7 +1706,8 @@ static int rtl8152_set_mac_address(struct net_device *netdev, void *p)
   * host system provided MAC address.
   * Examples of this are Dell TB15 and Dell WD15 docks
   */
-static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
+static int vendor_mac_passthru_addr_read(struct r8152 *tp,
+					 struct sockaddr_storage *ss)
  {
  	acpi_status status;
  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };

@@ -1774,47 +1775,48 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
  		ret = -EINVAL;
  		goto amacout;
  	}
-	memcpy(sa->sa_data, buf, 6);
+	memcpy(ss->__data, buf, 6);
  	tp->netdev->addr_assign_type = NET_ADDR_STOLEN;
  	netif_info(tp, probe, tp->netdev,
-		   "Using pass-thru MAC addr %pM\n", sa->sa_data);
+		   "Using pass-thru MAC addr %pM\n", ss->__data);
 
  amacout:
  	kfree(obj);
  	return ret;
  }
 
-static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa)
+static int determine_ethernet_addr(struct r8152 *tp,
+				   struct sockaddr_storage *ss)
  {
  	struct net_device *dev = tp->netdev;
  	int ret;
 
-	sa->sa_family = dev->type;
+	ss->ss_family = dev->type;
 
-	ret = eth_platform_get_mac_address(&tp->udev->dev, sa->sa_data);
+	ret = eth_platform_get_mac_address(&tp->udev->dev, ss->__data);
  	if (ret < 0) {
  		if (tp->version == RTL_VER_01) {
-			ret = pla_ocp_read(tp, PLA_IDR, 8, sa->sa_data);
+			ret = pla_ocp_read(tp, PLA_IDR, 8, ss->__data);
  		} else {
  			/* if device doesn't support MAC pass through this will
  			 * be expected to be non-zero
  			 */
-			ret = vendor_mac_passthru_addr_read(tp, sa);
+			ret = vendor_mac_passthru_addr_read(tp, ss);
  			if (ret < 0)
  				ret = pla_ocp_read(tp, PLA_BACKUP, 8,
-						   sa->sa_data);
+						   ss->__data);
  		}
  	}
 
  	if (ret < 0) {
  		netif_err(tp, probe, dev, "Get ether addr fail\n");
-	} else if (!is_valid_ether_addr(sa->sa_data)) {
+	} else if (!is_valid_ether_addr(ss->__data)) {
  		netif_err(tp, probe, dev, "Invalid ether addr %pM\n",
-			  sa->sa_data);
+			  ss->__data);
  		eth_hw_addr_random(dev);
-		ether_addr_copy(sa->sa_data, dev->dev_addr);
+		ether_addr_copy(ss->__data, dev->dev_addr);
  		netif_info(tp, probe, dev, "Random ether addr %pM\n",
-			   sa->sa_data);
+			   ss->__data);
  		return 0;
  	}
 

@@ -1824,17 +1826,17 @@ static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa)
  static int set_ethernet_addr(struct r8152 *tp, bool in_resume)
  {
  	struct net_device *dev = tp->netdev;
-	struct sockaddr sa;
+	struct sockaddr_storage ss;
  	int ret;
 
-	ret = determine_ethernet_addr(tp, &sa);
+	ret = determine_ethernet_addr(tp, &ss);
  	if (ret < 0)
  		return ret;
 
  	if (tp->version == RTL_VER_01)
-		eth_hw_addr_set(dev, sa.sa_data);
+		eth_hw_addr_set(dev, ss.__data);
  	else
-		ret = __rtl8152_set_mac_address(dev, &sa, in_resume);
+		ret = __rtl8152_set_mac_address(dev, &ss, in_resume);
 
  	return ret;
  }

@@ -8421,7 +8423,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
  {
  	struct r8152 *tp = usb_get_intfdata(intf);
  	struct net_device *netdev;
-	struct sockaddr sa;
+	struct sockaddr_storage ss;
 
  	if (!tp || !test_bit(PROBED_WITH_NO_ERRORS, &tp->flags))
  		goto exit;

@@ -8429,8 +8431,8 @@ static int rtl8152_post_reset(struct usb_interface *intf)
  	rtl_set_accessible(tp);
 
  	/* reset the MAC address in case of policy change */
-	if (determine_ethernet_addr(tp, &sa) >= 0)
-		dev_set_mac_address (tp->netdev, &sa, NULL);
+	if (determine_ethernet_addr(tp, &ss) >= 0)
+		dev_set_mac_address(tp->netdev, (struct sockaddr *)&ss, NULL);
 
  	netdev = tp->netdev;
  	if (!netif_running(netdev))

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]>

---
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(-)

include/linux/netdevice.h +1 -1

index 47200a394a02..b4242b997373 100644
@@ -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);

drivers/net/bonding/bond_alb.c +3 -5

index 7edf0fd58c34..2d37b07c8215 100644
@@ -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);
  	}

drivers/net/bonding/bond_main.c +4 -6

index 98cf4486fcee..b92e8935d686 100644
@@ -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) {

drivers/net/hyperv/netvsc_drv.c +3 -3

index d8b169ac0343..14a0d04e21ae 100644
@@ -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);
  	}
 

drivers/net/macvlan.c +5 -5

index 7045b1d58754..69e879780c36 100644
@@ -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)

drivers/net/team/team_core.c +1 -1

index d8fc0c79745d..a64e661c21a1 100644
@@ -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)

drivers/net/usb/r8152.c +1 -1

index b18dee1b1bb3..d6589b24c68d 100644
@@ -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))

net/core/dev.c +1 -0

index f8c8aad7df2e..1f1900ec26b2 100644
@@ -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);

net/core/dev_api.c +3 -3

index b5f293e637d9..e80404e76ca9 100644
@@ -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;

net/ieee802154/nl-phy.c +1 -1

index ee2b190e8e0d..4c07a475c567 100644
@@ -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;

net/ncsi/ncsi-manage.c +1 -1

index 0202db2aea3e..b36947063783 100644
@@ -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");

Instead of a heap allocating a variably sized struct sockaddr and lying about the type in the call to netif_set_mac_address(), use a stack allocated struct sockaddr_storage. This lets us drop the cast and avoid the allocation.

Putting "ss" on the stack means it will get a reused stack slot since it is the same size (128B) as other existing single-scope stack variables, like the vfinfo array (128B), so no additional stack space is used by this function.

Signed-off-by Kees Cook <[email protected]>

---
Cc: Kuniyuki Iwashima <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: Ido Schimmel <[email protected]>
Cc: <[email protected]>

---
  net/core/rtnetlink.c | 19 ++++---------------
  1 file changed, 4 insertions(+), 15 deletions(-)

net/core/rtnetlink.c +4 -15

index 9743f1c2ae3c..f9a35bdc58ad 100644
@@ -3080,17 +3080,7 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
  	}
 
  	if (tb[IFLA_ADDRESS]) {
-		struct sockaddr *sa;
-		int len;
-
-		len = sizeof(sa_family_t) + max_t(size_t, dev->addr_len,
-						  sizeof(*sa));
-		sa = kmalloc(len, GFP_KERNEL);
-		if (!sa) {
-			err = -ENOMEM;
-			goto errout;
-		}
-		sa->sa_family = dev->type;
+		struct sockaddr_storage ss = { };
 
  		netdev_unlock_ops(dev);
 

@@ -3098,10 +3088,9 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
  		down_write(&dev_addr_sem);
  		netdev_lock_ops(dev);
 
-		memcpy(sa->sa_data, nla_data(tb[IFLA_ADDRESS]),
-		       dev->addr_len);
-		err = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
-		kfree(sa);
+		ss.ss_family = dev->type;
+		memcpy(ss.__data, nla_data(tb[IFLA_ADDRESS]), dev->addr_len);
+		err = netif_set_mac_address(dev, &ss, extack);
  		if (err) {
  			up_write(&dev_addr_sem);
  			goto errout;

Hi,

As part of the effort to allow the compiler to reason about object sizes, we need to deal with the problematic variably sized struct sockaddr, which has no internal runtime size tracking. In much of the network stack the use of struct sockaddr_storage has been adopted. Continue the transition toward this for more of the internal APIs. Specifically:

- inet_addr_is_any()
- netif_set_mac_address()
- dev_set_mac_address()

Only 3 callers of dev_set_mac_address() needed adjustment; all others were already using struct sockaddr_storage internally.

From: Kees Cook <[email protected]>
Date: Tue, 20 May 2025 15:30:59 -0700

Quoted 12 lines

Hi,

As part of the effort to allow the compiler to reason about object sizes, we need to deal with the problematic variably sized struct sockaddr, which has no internal runtime size tracking. In much of the network stack the use of struct sockaddr_storage has been adopted. Continue the transition toward this for more of the internal APIs. Specifically:

- inet_addr_is_any()
- netif_set_mac_address()
- dev_set_mac_address()

Quoted 2 lines

Only 3 callers of dev_set_mac_address() needed adjustment; all others were already using struct sockaddr_storage internally.

I guess dev_set_mac_address_user() was missed on the way ?

For example, tap_ioctl() still uses sockaddr and calls dev_set_mac_address_user(), which cast it to _storage.

Ah yes, I can include that in the next version if you want? I was trying to find a stopping point since everything kind of touches everything ...

Quoted 2 lines · On Tue, 20 May 2025 17:42:32 -0700 Kees Cook wrote:

Ah yes, I can include that in the next version if you want? I was trying to find a stopping point since everything kind of touches everything ...

Looks like the build considers -Wincompatible-pointer-types to always imply -Werror or some such? We explicitly disable CONFIG_WERROR in our CI, but we still get:

drivers/net/macvlan.c:1302:34: error: incompatible pointer types passing 'struct sockaddr *' to parameter of type 'struct __kernel_sockaddr_storage *' [-Werror,-Wincompatible-pointer-types]
1302 | dev_set_mac_address(port->dev, &sa, NULL);
| ^~~

on this series :(