Re: [PATCH net] macvlan: fix passthru promiscuity accounting while down

[Hangbin Liu]

Hi Yan Xia,
On Thu, Oct 22, 2026 at 05:08:09PM +0800, Xia Yan wrote:
> macvlan_changelink() adjusts the lower device's promiscuity count when
> NOPROMISC changes, even if the passthru macvlan is down. The reference
> is acquired in macvlan_open() and released in macvlan_stop(), so a down
> interface has no reference to change.
> 
> Clearing NOPROMISC while down leaves an extra reference after the next
> open/stop cycle. Setting it while down can remove a reference held by
> another user of the lower device.
> 
> Guard the count adjustment with netif_running(dev). Keep storing the
> requested flags while down so the next open uses the new setting.
> 
> Comparing changelink with open/stop exposed the unmatched count changes;
> the same issue was also described in earlier macvlan reviews. A netlink
> test on UML reproduced both DOWN cases before the fix and verified the
> expected counts afterwards. Repeated flag updates, two open/stop cycles,
> deletion and bidirectional small-frame delivery were also checked.
> 
> Full x86_64 builds completed with WERROR disabled for allmodconfig, and
> WERROR and KASAN disabled for allyesconfig, including the resulting
> dependency changes. Neither comparison showed new diagnostics.
> 
> Fixes: df8ef8f3aaa6 ("macvlan: add FDB bridge ops and macvlan flags")
> Link: https://lore.kernel.org/netdev/179170088566.434549.4699134797461783473@xxxxxxxxxx/
> Link: https://lore.kernel.org/netdev/20261016003009.2692872-1-kuba@xxxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Xia Yan <yanxia@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/net/macvlan.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index afad90b..a48c9e6 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -1664,7 +1664,8 @@ static int macvlan_changelink(struct net_device *dev,
>  	if (data && data[IFLA_MACVLAN_FLAGS]) {
>  		__u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
>  		bool promisc = (flags ^ vlan->flags) & MACVLAN_FLAG_NOPROMISC;
> -		if (macvlan_passthru(vlan->port) && promisc) {
> +		if (macvlan_passthru(vlan->port) && promisc &&
> +		    netif_running(dev)) {
>  			int err;
>  
>  			if (flags & MACVLAN_FLAG_NOPROMISC)
> 
> 

If we discard the MACVLAN_FLAG_NOPROMISC change, should we remove the bit
before calling WRITE_ONCE(vlan->flags, flags)?

Or set extack and tell user that setting this flag with link down is forbidden.

Thanks
Hangbin