netdev - Re: [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device

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: <b13b4fab-22e7-56dc-caea-6ee3a67ad882@blackwall.org>
Date: Fri, 14 Jul 2023 09:51:48 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Hangbin Liu <liuhangbin@...il.com>, netdev@...r.kernel.org
Cc: Jay Vosburgh <j.vosburgh@...il.com>,
 "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 Liang Li <liali@...hat.com>, Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH net 1/2] bonding: reset bond's flags when down link is P2P
 device

On 14/07/2023 05:52, Hangbin Liu wrote:
> When adding a point to point downlink to the bond, we neglected to reset
> the bond's flags, which were still using flags like BROADCAST and
> MULTICAST. Consequently, this would initiate ARP/DAD for P2P downlink
> interfaces, such as when adding a GRE device to the bonding.
> 
> To address this issue, let's reset the bond's flags for P2P interfaces.
> 
> Before fix:
> 7: gre0@...E: <POINTOPOINT,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond0 state UNKNOWN group default qlen 1000
>     link/gre6 2006:70:10::1 peer 2006:70:10::2 permaddr 167f:18:f188::
> 8: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
>     link/gre6 2006:70:10::1 brd 2006:70:10::2
>     inet6 fe80::200:ff:fe00:0/64 scope link
>        valid_lft forever preferred_lft forever
> 
> After fix:
> 7: gre0@...E: <POINTOPOINT,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond2 state UNKNOWN group default qlen 1000
>     link/gre6 2006:70:10::1 peer 2006:70:10::2 permaddr c29e:557a:e9d9::
> 8: bond0: <POINTOPOINT,NOARP,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
>     link/gre6 2006:70:10::1 peer 2006:70:10::2
>     inet6 fe80::1/64 scope link
>        valid_lft forever preferred_lft forever
> 
> Reported-by: Liang Li <liali@...hat.com>
> Links: https://bugzilla.redhat.com/show_bug.cgi?id=2221438
> Fixes: 872254dd6b1f ("net/bonding: Enable bonding to enslave non ARPHRD_ETHER")
> Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> ---
>  drivers/net/bonding/bond_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7a0f25301f7e..0186b2d19e8d 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1508,6 +1508,10 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
>  
>  	memcpy(bond_dev->broadcast, slave_dev->broadcast,
>  		slave_dev->addr_len);
> +
> +	if (slave_dev->flags & IFF_POINTOPOINT)
> +		bond_dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST);
> +		bond_dev->flags |= (IFF_POINTOPOINT | IFF_NOARP);

missing {} ?

>  }
>  
>  /* On bonding slaves other than the currently active slave, suppress