[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6a0ecc78-6e9e-453e-83e5-f281c6c85476@embeddedor.com>
Date: Tue, 20 May 2025 16:50:09 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Kees Cook <kees@...nel.org>, Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: Jakub Kicinski <kuba@...nel.org>, Jay Vosburgh <jv@...sburgh.net>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>, "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>,
Simon Horman <horms@...nel.org>, Alexander Aring <alex.aring@...il.com>,
Stefan Schmidt <stefan@...enfreihafen.org>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Samuel Mendoza-Jonas <sam@...dozajonas.com>,
Paul Fertser <fercerpav@...il.com>, Hayes Wang <hayeswang@...ltek.com>,
Douglas Anderson <dianders@...omium.org>,
Grant Grundler <grundler@...omium.org>, Stanislav Fomichev
<sdf@...ichev.me>, Cosmin Ratiu <cratiu@...dia.com>,
Lei Yang <leiyang@...hat.com>, netdev@...r.kernel.org,
linux-hyperv@...r.kernel.org, linux-usb@...r.kernel.org,
linux-wpan@...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>,
Ido Schimmel <idosch@...dia.com>, 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-hardening@...r.kernel.org
Subject: Re: [PATCH 6/7] net: core: Convert dev_set_mac_address() to struct
sockaddr_storage
On 20/05/25 16:31, Kees Cook wrote:
> 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 <kees@...nel.org>
Acked-by: Gustavo A. R. Silva <gustavoars@...nel.org>
Thanks!
-Gustavo
> ---
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Jay Vosburgh <jv@...sburgh.net>
> Cc: Andrew Lunn <andrew+netdev@...n.ch>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: "K. Y. Srinivasan" <kys@...rosoft.com>
> Cc: Haiyang Zhang <haiyangz@...rosoft.com>
> Cc: Wei Liu <wei.liu@...nel.org>
> Cc: Dexuan Cui <decui@...rosoft.com>
> Cc: Jiri Pirko <jiri@...nulli.us>
> Cc: Simon Horman <horms@...nel.org>
> Cc: Alexander Aring <alex.aring@...il.com>
> Cc: Stefan Schmidt <stefan@...enfreihafen.org>
> Cc: Miquel Raynal <miquel.raynal@...tlin.com>
> Cc: Samuel Mendoza-Jonas <sam@...dozajonas.com>
> Cc: Paul Fertser <fercerpav@...il.com>
> Cc: Hayes Wang <hayeswang@...ltek.com>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Grant Grundler <grundler@...omium.org>
> Cc: Stanislav Fomichev <sdf@...ichev.me>
> Cc: Cosmin Ratiu <cratiu@...dia.com>
> Cc: Lei Yang <leiyang@...hat.com>
> Cc: <netdev@...r.kernel.org>
> Cc: <linux-hyperv@...r.kernel.org>
> Cc: <linux-usb@...r.kernel.org>
> Cc: <linux-wpan@...r.kernel.org>
> ---
> 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(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 47200a394a02..b4242b997373 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -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);
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 7edf0fd58c34..2d37b07c8215 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -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);
> }
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 98cf4486fcee..b92e8935d686 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -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) {
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index d8b169ac0343..14a0d04e21ae 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -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);
> }
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 7045b1d58754..69e879780c36 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -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)
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index d8fc0c79745d..a64e661c21a1 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -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)
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index b18dee1b1bb3..d6589b24c68d 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -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))
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f8c8aad7df2e..1f1900ec26b2 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -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);
> diff --git a/net/core/dev_api.c b/net/core/dev_api.c
> index b5f293e637d9..e80404e76ca9 100644
> --- a/net/core/dev_api.c
> +++ b/net/core/dev_api.c
> @@ -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;
> diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
> index ee2b190e8e0d..4c07a475c567 100644
> --- a/net/ieee802154/nl-phy.c
> +++ b/net/ieee802154/nl-phy.c
> @@ -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;
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 0202db2aea3e..b36947063783 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -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");
Powered by blists - more mailing lists