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

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

[Xia Yan]

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@kernel.org/
Link: https://lore.kernel.org/netdev/20261016003009.2692872-1-kuba@kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---
 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)

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

[Xia Yan]

Hi John,

Adding you to Cc; I missed you on the original submission.

The patch fixes passthru promiscuity accounting when NOPROMISC is
changed while the macvlan interface is down:

https://lore.kernel.org/netdev/20261022090809.12019-1-yanxia@cmss.chinamobile.com/

Best regards,
Xia Yan

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@kernel.org/
> Link: https://lore.kernel.org/netdev/20261016003009.2692872-1-kuba@kernel.org/
> Cc: stable@vger.kernel.org
> Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
> ---
>  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

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

[Xia Yan]

Hi Hangbin,

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

WRITE_ONCE() still saves the new flags. The patch only skips the
lower device's promiscuity count update while the macvlan is down.

If NOPROMISC is cleared while down, macvlan_open() takes the
reference on the next open and macvlan_stop() releases it. If it is
set while down, the next open leaves the lower device's count alone.

I'd keep saving the requested flags. Clearing the bit would lose
the user's setting. Rejecting the update would require bringing the
interface up before changing this option.

Thanks,
Xia Yan

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

[Hangbin Liu]

On Fri, Oct 23, 2026 at 02:16:30PM +0800, Xia Yan wrote:
> Hi Hangbin,
> 
> > If we discard the MACVLAN_FLAG_NOPROMISC change, should we remove the bit
> > before calling WRITE_ONCE(vlan->flags, flags)?
> 
> WRITE_ONCE() still saves the new flags. The patch only skips the
> lower device's promiscuity count update while the macvlan is down.
> 
> If NOPROMISC is cleared while down, macvlan_open() takes the
> reference on the next open and macvlan_stop() releases it. If it is
> set while down, the next open leaves the lower device's count alone.
> 
> I'd keep saving the requested flags. Clearing the bit would lose
> the user's setting. Rejecting the update would require bringing the
> interface up before changing this option.

Ah, I see. Thanks.

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>