Skip to contentSkip to conversation

rtnetlink: do_setlink: Use struct sockaddr_storage

PATCH In review Kees Cook 1 participant 2 messages May 20, 2025

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH v2 1/1 rtnetlink: do_setlink: Use struct sockaddr_storage

Instead of a heap allocation a variably sized struct sockaddr and lie 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 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]>

---
v1: https://lore.kernel.org/lkml/[email protected]/
v2: rebase, use struct sockaddr_storage now
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 6b7731739bbf..4953e202d0c0 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->sa_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
#1 lore raw
Quoted 9 lines · Kees Cook
net/core/rtnetlink.c +0 -0
index 6b7731739bbf..4953e202d0c0 100644
[...]

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

net/core/rtnetlink.c static int do_setlink(const struct sk_buff *skb, struct net_device *dev, · line 3096

Quoted 10 lines
net/core/rtnetlink.c +3 -4
-		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->sa_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;

Ugh, sorry, this has a dependency on a separate patch. Please ignore this; I will send them as a set.

Signature · 1 line
Kees Cook