| 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
| ||
|
Message-ID: <ebba72b4-9245-4be5-8f0d-87f7b326d468@embeddedor.com>
Date: Wed, 21 May 2025 17:07:28 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Kees Cook <kees@...nel.org>, Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
Jason Wang <jasowang@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
Cosmin Ratiu <cratiu@...dia.com>, Vladimir Oltean <vladimir.oltean@....com>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Kory Maincent <kory.maincent@...tlin.com>, Maxim Georgiev
<glipus@...il.com>, netdev@...r.kernel.org,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
Chaitanya Kulkarni <kch@...dia.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>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>, Lei Yang
<leiyang@...hat.com>, Ido Schimmel <idosch@...dia.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>,
Aleksander Jan Bajkowski <olek2@...pl>, Philipp Hahn <phahn-oss@....de>,
Eric Biggers <ebiggers@...gle.com>, 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 net-next v2 8/8] net: core: Convert
dev_set_mac_address_user() to use struct sockaddr_storage
On 21/05/25 14:46, Kees Cook wrote:
> 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 <kees@...nel.org>
Acked-by: Gustavo A. R. Silva <gustavoars@...nel.org>
Thanks!
-Gustavo
> ---
> Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>
> Cc: Jason Wang <jasowang@...hat.com>
> Cc: Andrew Lunn <andrew+netdev@...n.ch>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Simon Horman <horms@...nel.org>
> Cc: Stanislav Fomichev <sdf@...ichev.me>
> Cc: Cosmin Ratiu <cratiu@...dia.com>
> Cc: Vladimir Oltean <vladimir.oltean@....com>
> Cc: Florian Fainelli <florian.fainelli@...adcom.com>
> Cc: Kory Maincent <kory.maincent@...tlin.com>
> Cc: Maxim Georgiev <glipus@...il.com>
> Cc: Kuniyuki Iwashima <kuniyu@...zon.com>
> Cc: <netdev@...r.kernel.org>
> ---
> 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(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index b4242b997373..adb14db25798 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -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,
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index d4ece538f1b2..bdf0788d8e66 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -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;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 7babd1e9a378..1207196cbbed 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -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:
> diff --git a/net/core/dev_api.c b/net/core/dev_api.c
> index 6011a5ef649d..1bf0153195f2 100644
> --- a/net/core/dev_api.c
> +++ b/net/core/dev_api.c
> @@ -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);
>
> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> index fff13a8b48f1..616479e71466 100644
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -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)
Powered by blists - more mailing lists