netdev - Re: [PATCH 7/7] rtnetlink: do_setlink: Use struct sockaddr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2af2857c-d4b9-45c1-b241-dffa23ff4f32@embeddedor.com>
Date: Tue, 20 May 2025 16:50:29 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Kees Cook <kees@...nel.org>, Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 "David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Ido Schimmel <idosch@...dia.com>,
 netdev@...r.kernel.org, Christoph Hellwig <hch@....de>,
 Sagi Grimberg <sagi@...mberg.me>, Chaitanya Kulkarni <kch@...dia.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>,
 Mike Christie <michael.christie@...cle.com>,
 Max Gurtovoy <mgurtovoy@...dia.com>, Maurizio Lombardi
 <mlombard@...hat.com>, Dmitry Bogdanov <d.bogdanov@...ro.com>,
 Mingzhe Zou <mingzhe.zou@...ystack.cn>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 "Dr. David Alan Gilbert" <linux@...blig.org>,
 Andrew Lunn <andrew+netdev@...n.ch>, Stanislav Fomichev <sdf@...ichev.me>,
 Cosmin Ratiu <cratiu@...dia.com>, Lei Yang <leiyang@...hat.com>,
 Samuel Mendoza-Jonas <sam@...dozajonas.com>,
 Paul Fertser <fercerpav@...il.com>, Alexander Aring <alex.aring@...il.com>,
 Stefan Schmidt <stefan@...enfreihafen.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>, Hayes Wang
 <hayeswang@...ltek.com>, Douglas Anderson <dianders@...omium.org>,
 Grant Grundler <grundler@...omium.org>, Jay Vosburgh <jv@...sburgh.net>,
 "K. Y. Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>, Jiri Pirko <jiri@...nulli.us>,
 Eric Biggers <ebiggers@...gle.com>, Milan Broz <gmazyland@...il.com>,
 Philipp Hahn <phahn-oss@....de>, Ard Biesheuvel <ardb@...nel.org>,
 Al Viro <viro@...iv.linux.org.uk>, Ahmed Zaki <ahmed.zaki@...el.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>,
 Xiao Liang <shaw.leon@...il.com>, linux-kernel@...r.kernel.org,
 linux-nvme@...ts.infradead.org, linux-scsi@...r.kernel.org,
 target-devel@...r.kernel.org, linux-wpan@...r.kernel.org,
 linux-usb@...r.kernel.org, linux-hyperv@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH 7/7] rtnetlink: do_setlink: Use struct sockaddr_storage

On 20/05/25 16:31, Kees Cook wrote:
> 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 <kees@...nel.org>

Acked-by: Gustavo A. R. Silva <gustavoars@...nel.org>

Thanks!
-Gustavo

> ---
> Cc: Kuniyuki Iwashima <kuniyu@...zon.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Simon Horman <horms@...nel.org>
> Cc: Ido Schimmel <idosch@...dia.com>
> Cc: <netdev@...r.kernel.org>
> ---
>   net/core/rtnetlink.c | 19 ++++---------------
>   1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 9743f1c2ae3c..f9a35bdc58ad 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -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;