Skip to contentSkip to conversation

net: Convert dev_set_mac_address() to struct sockaddr_storage

PATCH In review Kees Cook 4 participants 13 messages May 21, 2025 – May 27, 2025

Subsystems (15)
Networking Network drivers SCSI USB +11 more subsystems
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

v2 cover net: Convert dev_set_mac_address() to struct sockaddr_storage

v2:
- add conversion of dev_set_mac_address_user() (kuniyu)
- fix missed sockaddr/sockaddr_storage conversion (kuba)
v1: https://lore.kernel.org/all/[email protected]/

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

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

-Kees

Kees Cook (8):
net: core: Convert inet_addr_is_any() to sockaddr_storage
net: core: Switch netif_set_mac_address() to struct sockaddr_storage net/ncsi: Use struct sockaddr_storage for pending_mac
ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
net: usb: r8152: Convert to use struct sockaddr_storage internally
net: core: Convert dev_set_mac_address() to struct sockaddr_storage
rtnetlink: do_setlink: Use struct sockaddr_storage
net: core: Convert dev_set_mac_address_user() to use struct
sockaddr_storage

 include/linux/inet.h                |  2 +-
 include/linux/netdevice.h           |  6 ++--
 net/ncsi/internal.h                 |  2 +-
 drivers/net/bonding/bond_alb.c      |  8 ++---
 drivers/net/bonding/bond_main.c     | 15 ++++-----
 drivers/net/hyperv/netvsc_drv.c     |  6 ++--
 drivers/net/macvlan.c               | 18 +++++-----
 drivers/net/tap.c                   | 14 +++++---
 drivers/net/team/team_core.c        |  2 +-
 drivers/net/tun.c                   |  8 ++++-
 drivers/net/usb/r8152.c             | 52 +++++++++++++++--------------
 drivers/nvme/target/rdma.c          |  2 +-
 drivers/nvme/target/tcp.c           |  2 +-
 drivers/target/iscsi/iscsi_target.c |  2 +-
 net/core/dev.c                      | 11 +++---
 net/core/dev_api.c                  | 11 +++---
 net/core/dev_ioctl.c                |  6 ++--
 net/core/rtnetlink.c                | 19 +++--------
 net/core/utils.c                    |  8 ++---
 net/ieee802154/nl-phy.c             |  6 ++--
 net/ncsi/ncsi-rsp.c                 | 18 +++++-----
 21 files changed, 109 insertions(+), 109 deletions(-)
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH v2 1/8 net: core: Convert inet_addr_is_any() to 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.

Reviewed-by Kuniyuki Iwashima <[email protected]>
Reviewed-by Martin K. Petersen <[email protected]> # SCSI
Signed-off-by Kees Cook <[email protected]>

---
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: Chaitanya Kulkarni <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Mike Christie <[email protected]>
Cc: Max Gurtovoy <[email protected]>
Cc: Maurizio Lombardi <[email protected]>
Cc: Dmitry Bogdanov <[email protected]>
Cc: Mingzhe Zou <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: "Dr. David Alan Gilbert" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

---
 include/linux/inet.h                | 2 +-
 drivers/nvme/target/rdma.c          | 2 +-
 drivers/nvme/target/tcp.c           | 2 +-
 drivers/target/iscsi/iscsi_target.c | 2 +-
 net/core/utils.c                    | 8 ++++----
 5 files changed, 8 insertions(+), 8 deletions(-)
include/linux/inet.h +1 -1
index bd8276e96e60..9158772f3559 100644
@@ -55,6 +55,6 @@ extern int in6_pton(const char *src, int srclen, u8 *dst, int delim, const char
 
 extern int inet_pton_with_scope(struct net *net, unsigned short af,
 		const char *src, const char *port, struct sockaddr_storage *addr);
-extern bool inet_addr_is_any(struct sockaddr *addr);
+bool inet_addr_is_any(struct sockaddr_storage *addr);
 
 #endif	/* _LINUX_INET_H */
drivers/nvme/target/rdma.c +1 -1
index 2a4536ef6184..79a5aad2e9d0 100644
@@ -1999,7 +1999,7 @@ static void nvmet_rdma_disc_port_addr(struct nvmet_req *req,
 	struct nvmet_rdma_port *port = nport->priv;
 	struct rdma_cm_id *cm_id = port->cm_id;
 
-	if (inet_addr_is_any((struct sockaddr *)&cm_id->route.addr.src_addr)) {
+	if (inet_addr_is_any(&cm_id->route.addr.src_addr)) {
 		struct nvmet_rdma_rsp *rsp =
 			container_of(req, struct nvmet_rdma_rsp, req);
 		struct rdma_cm_id *req_cm_id = rsp->queue->cm_id;
drivers/nvme/target/tcp.c +1 -1
index 12a5cb8641ca..5cd1cf74f8ff 100644
@@ -2194,7 +2194,7 @@ static void nvmet_tcp_disc_port_addr(struct nvmet_req *req,
 {
 	struct nvmet_tcp_port *port = nport->priv;
 
-	if (inet_addr_is_any((struct sockaddr *)&port->addr)) {
+	if (inet_addr_is_any(&port->addr)) {
 		struct nvmet_tcp_cmd *cmd =
 			container_of(req, struct nvmet_tcp_cmd, req);
 		struct nvmet_tcp_queue *queue = cmd->queue;
drivers/target/iscsi/iscsi_target.c +1 -1
index 620ba6e0ab07..a2dde08c8a62 100644
@@ -3419,7 +3419,7 @@ iscsit_build_sendtargets_response(struct iscsit_cmd *cmd,
 					}
 				}
 
-				if (inet_addr_is_any((struct sockaddr *)&np->np_sockaddr))
+				if (inet_addr_is_any(&np->np_sockaddr))
 					sockaddr = &conn->local_sockaddr;
 				else
 					sockaddr = &np->np_sockaddr;
net/core/utils.c +4 -4
index 27f4cffaae05..e47feeaa5a49 100644
@@ -399,9 +399,9 @@ int inet_pton_with_scope(struct net *net, __kernel_sa_family_t af,
 }
 EXPORT_SYMBOL(inet_pton_with_scope);
 
-bool inet_addr_is_any(struct sockaddr *addr)
+bool inet_addr_is_any(struct sockaddr_storage *addr)
 {
-	if (addr->sa_family == AF_INET6) {
+	if (addr->ss_family == AF_INET6) {
 		struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)addr;
 		const struct sockaddr_in6 in6_any =
 			{ .sin6_addr = IN6ADDR_ANY_INIT };

@@ -409,13 +409,13 @@ bool inet_addr_is_any(struct sockaddr *addr)
 		if (!memcmp(in6->sin6_addr.s6_addr,
 			    in6_any.sin6_addr.s6_addr, 16))
 			return true;
-	} else if (addr->sa_family == AF_INET) {
+	} else if (addr->ss_family == AF_INET) {
 		struct sockaddr_in *in = (struct sockaddr_in *)addr;
 
 		if (in->sin_addr.s_addr == htonl(INADDR_ANY))
 			return true;
 	} else {
-		pr_warn("unexpected address family %u\n", addr->sa_family);
+		pr_warn("unexpected address family %u\n", addr->ss_family);
 	}
 
 	return false;
Signature · 1 line
2.34.1
author
#2 lore raw

PATCH v2 2/8 net: core: Switch netif_set_mac_address() to struct sockaddr_storage

In reply to Kees Cook

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.

Acked-by Gustavo A. R. Silva <[email protected]>
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);
Signature · 1 line
2.34.1
author
#3 lore raw

PATCH v2 3/8 net/ncsi: Use struct sockaddr_storage for pending_mac

In reply to Kees Cook

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

Acked-by Gustavo A. R. Silva <[email protected]>
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;
 	}
Signature · 1 line
2.34.1
author
#4 lore raw

PATCH v2 4/8 ieee802154: Use struct sockaddr_storage with dev_set_mac_address()

In reply to Kees Cook

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.

Acked-by Gustavo A. R. Silva <[email protected]>
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;
Signature · 1 line
2.34.1
author
#5 lore raw

PATCH v2 5/8 net: usb: r8152: Convert to use struct sockaddr_storage internally

In reply to Kees Cook

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

Acked-by Gustavo A. R. Silva <[email protected]>
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))
Signature · 1 line
2.34.1
author
#6 lore raw

PATCH v2 6/8 net: core: Convert dev_set_mac_address() to struct sockaddr_storage

In reply to Kees Cook

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.

Acked-by Gustavo A. R. Silva <[email protected]>
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 | 15 ++++++---------
 drivers/net/hyperv/netvsc_drv.c |  6 +++---
 drivers/net/macvlan.c           | 18 +++++++++---------
 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, 30 insertions(+), 34 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 +6 -9
index 98cf4486fcee..c4d53e8e7c15 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);

@@ -2127,7 +2125,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	}
 
 	ss.ss_family = slave_dev->type;
-	res = dev_set_mac_address(slave_dev, (struct sockaddr *)&ss, extack);
+	res = dev_set_mac_address(slave_dev, &ss, extack);
 	if (res) {
 		slave_err(bond_dev, slave_dev, "Error %d calling set_mac_address\n", res);
 		goto err_restore_mtu;

@@ -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) {

@@ -4936,8 +4934,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 		if (rollback_slave == slave)
 			break;
 
-		tmp_res = dev_set_mac_address(rollback_slave->dev,
-					      (struct sockaddr *)&tmp_ss, NULL);
+		tmp_res = dev_set_mac_address(rollback_slave->dev, &tmp_ss, NULL);
 		if (tmp_res) {
 			slave_dbg(bond_dev, rollback_slave->dev, "%s: unwind err %d\n",
 				   __func__, tmp_res);
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 +9 -9
index 7045b1d58754..4df991e494bd 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)

@@ -1295,11 +1295,11 @@ static void macvlan_port_destroy(struct net_device *dev)
 	 */
 	if (macvlan_passthru(port) &&
 	    !ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
-		struct sockaddr sa;
+		struct sockaddr_storage ss;
 
-		sa.sa_family = port->dev->type;
-		memcpy(&sa.sa_data, port->perm_addr, port->dev->addr_len);
-		dev_set_mac_address(port->dev, &sa, NULL);
+		ss.ss_family = port->dev->type;
+		memcpy(&ss.__data, port->perm_addr, port->dev->addr_len);
+		dev_set_mac_address(port->dev, &ss, NULL);
 	}
 
 	kfree(port);
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..6011a5ef649d 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, ss, 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");
Signature · 1 line
2.34.1
author
#7 lore raw

PATCH v2 7/8 rtnetlink: do_setlink: Use struct sockaddr_storage

In reply to Kees Cook

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.

Acked-by Gustavo A. R. Silva <[email protected]>
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;
Signature · 1 line
2.34.1
author
#8 lore raw

PATCH v2 8/8 net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage

In reply to Kees Cook

Convert callers of dev_set_mac_address_user() to use struct sockaddr_storage. Add sanity checks on dev->addr_len usage.

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

---
Cc: Willem de Bruijn <[email protected]>
Cc: Jason Wang <[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: Simon Horman <[email protected]>
Cc: Stanislav Fomichev <[email protected]>
Cc: Cosmin Ratiu <[email protected]>
Cc: Vladimir Oltean <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Kory Maincent <[email protected]>
Cc: Maxim Georgiev <[email protected]>
Cc: Kuniyuki Iwashima <[email protected]>
Cc: <[email protected]>

---
 include/linux/netdevice.h |  2 +-
 drivers/net/tap.c         | 14 +++++++++-----
 drivers/net/tun.c         |  8 +++++++-
 net/core/dev_api.c        |  5 +++--
 net/core/dev_ioctl.c      |  6 ++++--
 5 files changed, 24 insertions(+), 11 deletions(-)
include/linux/netdevice.h +1 -1
index b4242b997373..adb14db25798 100644
@@ -4216,7 +4216,7 @@ 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_storage *ss,
 			struct netlink_ext_ack *extack);
-int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
+int dev_set_mac_address_user(struct net_device *dev, struct sockaddr_storage *ss,
 			     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,
drivers/net/tap.c +9 -5
index d4ece538f1b2..bdf0788d8e66 100644
@@ -923,7 +923,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 	unsigned int __user *up = argp;
 	unsigned short u;
 	int __user *sp = argp;
-	struct sockaddr sa;
+	struct sockaddr_storage ss;
 	int s;
 	int ret;
 

@@ -1000,16 +1000,17 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 			return -ENOLINK;
 		}
 		ret = 0;
-		dev_get_mac_address(&sa, dev_net(tap->dev), tap->dev->name);
+		dev_get_mac_address((struct sockaddr *)&ss, dev_net(tap->dev),
+				    tap->dev->name);
 		if (copy_to_user(&ifr->ifr_name, tap->dev->name, IFNAMSIZ) ||
-		    copy_to_user(&ifr->ifr_hwaddr, &sa, sizeof(sa)))
+		    copy_to_user(&ifr->ifr_hwaddr, &ss, sizeof(ifr->ifr_hwaddr)))
 			ret = -EFAULT;
 		tap_put_tap_dev(tap);
 		rtnl_unlock();
 		return ret;
 
 	case SIOCSIFHWADDR:
-		if (copy_from_user(&sa, &ifr->ifr_hwaddr, sizeof(sa)))
+		if (copy_from_user(&ss, &ifr->ifr_hwaddr, sizeof(ifr->ifr_hwaddr)))
 			return -EFAULT;
 		rtnl_lock();
 		tap = tap_get_tap_dev(q);

@@ -1017,7 +1018,10 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 			rtnl_unlock();
 			return -ENOLINK;
 		}
-		ret = dev_set_mac_address_user(tap->dev, &sa, NULL);
+		if (tap->dev->addr_len > sizeof(ifr->ifr_hwaddr))
+			ret = -EINVAL;
+		else
+			ret = dev_set_mac_address_user(tap->dev, &ss, NULL);
 		tap_put_tap_dev(tap);
 		rtnl_unlock();
 		return ret;
drivers/net/tun.c +7 -1
index 7babd1e9a378..1207196cbbed 100644
@@ -3193,7 +3193,13 @@ 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);
+		if (tun->dev->addr_len > sizeof(ifr.ifr_hwaddr)) {
+			ret = -EINVAL;
+			break;
+		}
+		ret = dev_set_mac_address_user(tun->dev,
+					       (struct sockaddr_storage *)&ifr.ifr_hwaddr,
+					       NULL);
 		break;
 
 	case TUNGETSNDBUF:
net/core/dev_api.c +3 -2
index 6011a5ef649d..1bf0153195f2 100644
@@ -84,14 +84,15 @@ 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,
+int dev_set_mac_address_user(struct net_device *dev,
+			     struct sockaddr_storage *ss,
 			     struct netlink_ext_ack *extack)
 {
 	int ret;
 
 	down_write(&dev_addr_sem);
 	netdev_lock_ops(dev);
-	ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
+	ret = netif_set_mac_address(dev, ss, extack);
 	netdev_unlock_ops(dev);
 	up_write(&dev_addr_sem);
net/core/dev_ioctl.c +4 -2
index fff13a8b48f1..616479e71466 100644
@@ -572,9 +572,11 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
 		return dev_set_mtu(dev, ifr->ifr_mtu);
 
 	case SIOCSIFHWADDR:
-		if (dev->addr_len > sizeof(struct sockaddr))
+		if (dev->addr_len > sizeof(ifr->ifr_hwaddr))
 			return -EINVAL;
-		return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL);
+		return dev_set_mac_address_user(dev,
+						(struct sockaddr_storage *)&ifr->ifr_hwaddr,
+						NULL);
 
 	case SIOCSIFHWBROADCAST:
 		if (ifr->ifr_hwaddr.sa_family != dev->type)
Signature · 1 line
2.34.1
Acked-by on v2 8/8
#9 lore raw

Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage

Quoted 4 lines · Kees Cook

Convert callers of dev_set_mac_address_user() to use struct sockaddr_storage. Add sanity checks on dev->addr_len usage.

Signed-off-by Kees Cook <[email protected]>
Acked-by Gustavo A. R. Silva <[email protected]>

Thanks!
-Gustavo

Quoted 143 lines

---
Cc: Willem de Bruijn <[email protected]>
Cc: Jason Wang <[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: Simon Horman <[email protected]>
Cc: Stanislav Fomichev <[email protected]>
Cc: Cosmin Ratiu <[email protected]>
Cc: Vladimir Oltean <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Kory Maincent <[email protected]>
Cc: Maxim Georgiev <[email protected]>
Cc: Kuniyuki Iwashima <[email protected]>
Cc: <[email protected]>

---
  include/linux/netdevice.h |  2 +-
  drivers/net/tap.c         | 14 +++++++++-----
  drivers/net/tun.c         |  8 +++++++-
  net/core/dev_api.c        |  5 +++--
  net/core/dev_ioctl.c      |  6 ++++--
  5 files changed, 24 insertions(+), 11 deletions(-)
include/linux/netdevice.h +1 -1
index b4242b997373..adb14db25798 100644
@@ -4216,7 +4216,7 @@ 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_storage *ss,
  			struct netlink_ext_ack *extack);
-int dev_set_mac_address_user(struct net_device *dev, struct sockaddr *sa,
+int dev_set_mac_address_user(struct net_device *dev, struct sockaddr_storage *ss,
  			     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,
drivers/net/tap.c +9 -5
index d4ece538f1b2..bdf0788d8e66 100644
@@ -923,7 +923,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
  	unsigned int __user *up = argp;
  	unsigned short u;
  	int __user *sp = argp;
-	struct sockaddr sa;
+	struct sockaddr_storage ss;
  	int s;
  	int ret;
 

@@ -1000,16 +1000,17 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
  			return -ENOLINK;
  		}
  		ret = 0;
-		dev_get_mac_address(&sa, dev_net(tap->dev), tap->dev->name);
+		dev_get_mac_address((struct sockaddr *)&ss, dev_net(tap->dev),
+				    tap->dev->name);
  		if (copy_to_user(&ifr->ifr_name, tap->dev->name, IFNAMSIZ) ||
-		    copy_to_user(&ifr->ifr_hwaddr, &sa, sizeof(sa)))
+		    copy_to_user(&ifr->ifr_hwaddr, &ss, sizeof(ifr->ifr_hwaddr)))
  			ret = -EFAULT;
  		tap_put_tap_dev(tap);
  		rtnl_unlock();
  		return ret;
 
  	case SIOCSIFHWADDR:
-		if (copy_from_user(&sa, &ifr->ifr_hwaddr, sizeof(sa)))
+		if (copy_from_user(&ss, &ifr->ifr_hwaddr, sizeof(ifr->ifr_hwaddr)))
  			return -EFAULT;
  		rtnl_lock();
  		tap = tap_get_tap_dev(q);

@@ -1017,7 +1018,10 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
  			rtnl_unlock();
  			return -ENOLINK;
  		}
-		ret = dev_set_mac_address_user(tap->dev, &sa, NULL);
+		if (tap->dev->addr_len > sizeof(ifr->ifr_hwaddr))
+			ret = -EINVAL;
+		else
+			ret = dev_set_mac_address_user(tap->dev, &ss, NULL);
  		tap_put_tap_dev(tap);
  		rtnl_unlock();
  		return ret;
drivers/net/tun.c +7 -1
index 7babd1e9a378..1207196cbbed 100644
@@ -3193,7 +3193,13 @@ 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);
+		if (tun->dev->addr_len > sizeof(ifr.ifr_hwaddr)) {
+			ret = -EINVAL;
+			break;
+		}
+		ret = dev_set_mac_address_user(tun->dev,
+					       (struct sockaddr_storage *)&ifr.ifr_hwaddr,
+					       NULL);
  		break;
 
  	case TUNGETSNDBUF:
net/core/dev_api.c +3 -2
index 6011a5ef649d..1bf0153195f2 100644
@@ -84,14 +84,15 @@ 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,
+int dev_set_mac_address_user(struct net_device *dev,
+			     struct sockaddr_storage *ss,
  			     struct netlink_ext_ack *extack)
  {
  	int ret;
 
  	down_write(&dev_addr_sem);
  	netdev_lock_ops(dev);
-	ret = netif_set_mac_address(dev, (struct sockaddr_storage *)sa, extack);
+	ret = netif_set_mac_address(dev, ss, extack);
  	netdev_unlock_ops(dev);
  	up_write(&dev_addr_sem);
net/core/dev_ioctl.c +4 -2
index fff13a8b48f1..616479e71466 100644
@@ -572,9 +572,11 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
  		return dev_set_mtu(dev, ifr->ifr_mtu);
 
  	case SIOCSIFHWADDR:
-		if (dev->addr_len > sizeof(struct sockaddr))
+		if (dev->addr_len > sizeof(ifr->ifr_hwaddr))
  			return -EINVAL;
-		return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL);
+		return dev_set_mac_address_user(dev,
+						(struct sockaddr_storage *)&ifr->ifr_hwaddr,
+						NULL);
 
  	case SIOCSIFHWBROADCAST:
  		if (ifr->ifr_hwaddr.sa_family != dev->type)
maintainer
#10 lore raw

Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage

In reply to Kees Cook
Quoted 5 lines · Kees Cook
net/core/dev_ioctl.c +0 -0
index fff13a8b48f1..616479e71466 100644

net/core/dev_ioctl.c static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, · line 579

Quoted 10 lines
net/core/dev_ioctl.c +4 -2
@@ -572,9 +572,11 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
 		return dev_set_mtu(dev, ifr->ifr_mtu);
 
 	case SIOCSIFHWADDR:
-		if (dev->addr_len > sizeof(struct sockaddr))
+		if (dev->addr_len > sizeof(ifr->ifr_hwaddr))
 			return -EINVAL;
-		return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL);
+		return dev_set_mac_address_user(dev,
+						(struct sockaddr_storage *)&ifr->ifr_hwaddr,
+						NULL);

Side note for a possible follow-up: the above pattern is repeated a couple of times: IMHO consolidating it into an helper would be nice. Also such helper could/should explicitly convert ifr->ifr_hwaddr to sockaddr_storage and avoid the cast.

/P

author
#11 lore raw

Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage

Quoted 8 lines · Paolo Abeni
Quoted 7 lines · Kees Cook
net/core/dev_ioctl.c +0 -0
index fff13a8b48f1..616479e71466 100644
@@ -572,9 +572,11 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
 		return dev_set_mtu(dev, ifr->ifr_mtu);
Quoted 11 lines

net/core/dev_ioctl.c static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data, · line 579

Quoted 8 lines
net/core/dev_ioctl.c +4 -2
 	case SIOCSIFHWADDR:
-		if (dev->addr_len > sizeof(struct sockaddr))
+		if (dev->addr_len > sizeof(ifr->ifr_hwaddr))
 			return -EINVAL;
-		return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL);
+		return dev_set_mac_address_user(dev,
+						(struct sockaddr_storage *)&ifr->ifr_hwaddr,
+						NULL);

Side note for a possible follow-up: the above pattern is repeated a couple of times: IMHO consolidating it into an helper would be nice.

Yeah, I will look at that.

Quoted 2 lines

Also such helper could/should explicitly convert ifr->ifr_hwaddr to sockaddr_storage and avoid the cast.

It's UAPI, so it looked verrrry painful to change.

Signature · 1 line
Kees Cook
CI
#12 lore raw
In reply to Kees Cook
This series was applied to netdev/net-next.git (main) by Paolo Abeni <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Paolo Abeni <[email protected]>:

Quoted 8 lines · On Wed, 21 May 2025 13:46:08 -0700 you wrote:

v2:
- add conversion of dev_set_mac_address_user() (kuniyu)
- fix missed sockaddr/sockaddr_storage conversion (kuba)
v1: https://lore.kernel.org/all/[email protected]/

Hi,

[...]

Here is the summary with links:
- [net-next,v2,1/8] net: core: Convert inet_addr_is_any() to sockaddr_storage (no matching commit)
- [net-next,v2,2/8] net: core: Switch netif_set_mac_address() to struct sockaddr_storage

    https://git.kernel.org/netdev/net-next/c/161972650d67
  - [net-next,v2,3/8] net/ncsi: Use struct sockaddr_storage for pending_mac
    https://git.kernel.org/netdev/net-next/c/db586cad6f45
  - [net-next,v2,4/8] ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
    https://git.kernel.org/netdev/net-next/c/7da6117ea144
  - [net-next,v2,5/8] net: usb: r8152: Convert to use struct sockaddr_storage internally
    https://git.kernel.org/netdev/net-next/c/79deac8d538d
  - [net-next,v2,6/8] net: core: Convert dev_set_mac_address() to struct sockaddr_storage
    (no matching commit)
  - [net-next,v2,7/8] rtnetlink: do_setlink: Use struct sockaddr_storage
    https://git.kernel.org/netdev/net-next/c/6b12e0a3c3c9

- [net-next,v2,8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage (no matching commit)

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html