| Message ID | 20250521204310.it.500-kees@kernel.org |
|---|---|
| Headers | show |
| Series | net: Convert dev_set_mac_address() to struct sockaddr_storage | expand |
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@kernel.org> Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks! -Gustavo > --- > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Andrew Lunn <andrew+netdev@lunn.ch> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Paolo Abeni <pabeni@redhat.com> > Cc: Simon Horman <horms@kernel.org> > Cc: Stanislav Fomichev <sdf@fomichev.me> > Cc: Cosmin Ratiu <cratiu@nvidia.com> > Cc: Vladimir Oltean <vladimir.oltean@nxp.com> > Cc: Florian Fainelli <florian.fainelli@broadcom.com> > Cc: Kory Maincent <kory.maincent@bootlin.com> > Cc: Maxim Georgiev <glipus@gmail.com> > Cc: Kuniyuki Iwashima <kuniyu@amazon.com> > Cc: <netdev@vger.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)
v2: - add conversion of dev_set_mac_address_user() (kuniyu) - fix missed sockaddr/sockaddr_storage conversion (kuba) v1: https://lore.kernel.org/all/20250520222452.work.063-kees@kernel.org/ 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(-)