1. Patchew
  2. linux
  3. [PATCH net-next v2 0/8] net: Convert dev_set_mac_address() to struct sockaddr_storage
  4. View patch

[PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage

Kees Cook posted 8 patches 7 months ago
[PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
Posted by Kees Cook 7 months ago 
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>
---
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)
-- 
2.34.1
Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
Posted by Paolo Abeni 6 months, 3 weeks ago 
On 5/21/25 10:46 PM, Kees Cook wrote:
> 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);

Side note for a possible follow-up: the above pattern is repeated a
couple of times: IMHO consolidating it into an helper would be nice.
Also such helper could/should explicitly convert ifr->ifr_hwaddr to
sockaddr_storage and avoid the cast.

/P
Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
Posted by Kees Cook 6 months, 3 weeks ago 
On Tue, May 27, 2025 at 09:02:28AM +0200, Paolo Abeni wrote:
> On 5/21/25 10:46 PM, Kees Cook wrote:
> > 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);
> 
> Side note for a possible follow-up: the above pattern is repeated a
> couple of times: IMHO consolidating it into an helper would be nice.

Yeah, I will look at that.

> Also such helper could/should explicitly convert ifr->ifr_hwaddr to
> sockaddr_storage and avoid the cast.

It's UAPI, so it looked verrrry painful to change.

-- 
Kees Cook
Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
Posted by Gustavo A. R. Silva 6 months, 4 weeks ago 

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)

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.