netdev - Re: [PATCH net-next v2 3/8] net: s/dev_get_mac_address/netif_get_mac

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: <6862fb095090_183f832945b@willemb.c.googlers.com.notmuch>
Date: Mon, 30 Jun 2025 17:00:57 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Stanislav Fomichev <sdf@...ichev.me>, 
 netdev@...r.kernel.org
Cc: davem@...emloft.net, 
 edumazet@...gle.com, 
 kuba@...nel.org, 
 pabeni@...hat.com
Subject: Re: [PATCH net-next v2 3/8] net:
 s/dev_get_mac_address/netif_get_mac_address/

Stanislav Fomichev wrote:
> Commit cc34acd577f1 ("docs: net: document new locking reality")
> introduced netif_ vs dev_ function semantics: the former expects locked
> netdev, the latter takes care of the locking. We don't strictly
> follow this semantics on either side, but there are more dev_xxx handlers
> now that don't fit. Rename them to netif_xxx where appropriate.
> 
> netif_get_mac_address is used only by tun/tap, so move it into
> NETDEV_INTERNAL namespace.
> 
> Signed-off-by: Stanislav Fomichev <sdf@...ichev.me>
> ---
>  drivers/net/tap.c         | 6 ++++--
>  drivers/net/tun.c         | 4 +++-
>  include/linux/netdevice.h | 2 +-
>  net/core/dev.c            | 4 ++--
>  net/core/dev_ioctl.c      | 3 ++-
>  net/core/net-sysfs.c      | 2 +-
>  6 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index bdf0788d8e66..4c85770c809b 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -28,6 +28,8 @@
>  
>  #include "tun_vnet.h"
>  
> +MODULE_IMPORT_NS("NETDEV_INTERNAL");
> +
>  #define TAP_IFFEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE)
>  
>  static struct proto tap_proto = {
> @@ -1000,8 +1002,8 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>  			return -ENOLINK;
>  		}
>  		ret = 0;
> -		dev_get_mac_address((struct sockaddr *)&ss, dev_net(tap->dev),
> -				    tap->dev->name);
> +		netif_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, &ss, sizeof(ifr->ifr_hwaddr)))
>  			ret = -EFAULT;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index f8c5e2fd04df..4509ae68decf 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -85,6 +85,8 @@
>  
>  #include "tun_vnet.h"
>  
> +MODULE_IMPORT_NS("NETDEV_INTERNAL");
> +

Thanks for giving this a go. Now that you've implemented it, does the
risk (of overlooking callers, mainly) indeed seem acceptable?

Documentation/core-api/symbol-namespaces.rst says

  It is advisable to add the MODULE_IMPORT_NS() statement close to other module
  metadata definitions like MODULE_AUTHOR() or MODULE_LICENSE().

No need to respin just for this from me. Something to consider,
especially if anything else comes up.

Just curious, did you use the modpost and make nsdeps, or was it
sufficient to find the callers with tools like cscope and grep?