linux-kernel - Re: [PATCH net-next] net/core: Allow live renaming when an interface is up

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] [day] [month] [year] [list]

Date:   Thu, 3 Nov 2022 19:50:37 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andy Ren <andy.ren@...cruise.com>
Cc:     netdev@...r.kernel.org, richardbgobert@...il.com,
        davem@...emloft.net, wsa+renesas@...g-engineering.com,
        edumazet@...gle.com, petrm@...dia.com, pabeni@...hat.com,
        corbet@....net, andrew@...n.ch, dsahern@...il.com,
        sthemmin@...rosoft.com, idosch@...sch.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        roman.gushchin@...ux.dev
Subject: Re: [PATCH net-next] net/core: Allow live renaming when an
 interface is up

On Thu,  3 Nov 2022 16:58:47 -0700 Andy Ren wrote:
> @@ -1691,7 +1690,6 @@ enum netdev_priv_flags {
>  	IFF_FAILOVER			= 1<<27,
>  	IFF_FAILOVER_SLAVE		= 1<<28,
>  	IFF_L3MDEV_RX_HANDLER		= 1<<29,
> -	IFF_LIVE_RENAME_OK		= 1<<30,

As Stephen says the hole should be somehow noted.
Comment saying what it was, or just a comment saying there 
is a hole that can be reused.

>  	IFF_TX_SKB_NO_LINEAR		= BIT_ULL(31),
>  	IFF_CHANGE_PROTO_DOWN		= BIT_ULL(32),
>  };
> @@ -1726,7 +1724,6 @@ enum netdev_priv_flags {
>  #define IFF_FAILOVER			IFF_FAILOVER
>  #define IFF_FAILOVER_SLAVE		IFF_FAILOVER_SLAVE
>  #define IFF_L3MDEV_RX_HANDLER		IFF_L3MDEV_RX_HANDLER
> -#define IFF_LIVE_RENAME_OK		IFF_LIVE_RENAME_OK
>  #define IFF_TX_SKB_NO_LINEAR		IFF_TX_SKB_NO_LINEAR
>  
>  /* Specifies the type of the struct net_device::ml_priv pointer */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2e4f1c97b59e..a2d650ae15d7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1163,22 +1163,6 @@ int dev_change_name(struct net_device *dev, const char *newname)
>  
>  	net = dev_net(dev);
>  
> -	/* Some auto-enslaved devices e.g. failover slaves are
> -	 * special, as userspace might rename the device after
> -	 * the interface had been brought up and running since
> -	 * the point kernel initiated auto-enslavement. Allow
> -	 * live name change even when these slave devices are
> -	 * up and running.
> -	 *
> -	 * Typically, users of these auto-enslaving devices
> -	 * don't actually care about slave name change, as
> -	 * they are supposed to operate on master interface
> -	 * directly.
> -	 */
> -	if (dev->flags & IFF_UP &&
> -	    likely(!(dev->priv_flags & IFF_LIVE_RENAME_OK)))
> -		return -EBUSY;
> -

Let's leave a hint for potential triage and add something extra to the
netdev_info() print a few lines down in case the interface is renamed
while UP. Perhaps:

	netdev_info(dev, "renamed from %s%s\n", oldname,
		    dev->flags & IFF_UP ? " (while UP)" : "");

or some such.