netdev - Re: [PATCH net] bonding: fix ad_actor_system option setting to default

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]

Message-ID: <f70ec480-f224-d90c-8f96-53b5692e1494@riseup.net>
Date:   Sat, 18 Dec 2021 02:28:22 +0100
From:   "Fernando F. Mancera" <ffmancera@...eup.net>
To:     Jay Vosburgh <jay.vosburgh@...onical.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net] bonding: fix ad_actor_system option setting to
 default

On 12/17/21 23:22, Jay Vosburgh wrote:
> Fernando Fernandez Mancera <ffmancera@...eup.net> wrote:
> 
>> When 802.3ad bond mode is configured the ad_actor_system option is set
>> to "00:00:00:00:00:00". But when trying to set the default value
>> manually it was failing with EINVAL.
>>
>> A zero ethernet address is valid, only multicast addresses are not valid
>> values.
> 
> 	Your intent here by setting ad_actor_system to all zeroes is to
> induce bonding to actually set ad_actor_system to the MAC of the bond
> itself?

Yes, as the user should be able to set it back to the MAC of the bond 
itself. Basically, restore it to the default value.

> 
> 	If so, please also update Documentation/networking/bonding.rst,
> as the current text says
> 
>          In an AD system, this specifies the mac-address for the actor in
>          protocol packet exchanges (LACPDUs). The value cannot be NULL or
>          multicast. It is preferred to have the local-admin bit set for this
>          mac but driver does not enforce it. If the value is not given then
>          system defaults to using the masters' mac address as actors' system
>          address.
> 
> 	I'd suggest something like "The value cannot be a multicast
> address.  If the all-zeroes MAC is specified, bonding will internally
> use the MAC of the bond itself."

Sure, that would be good. I will do it. Thank you!

> 
> 	-J
> 
>> Signed-off-by: Fernando Fernandez Mancera <ffmancera@...eup.net>
>> ---
>> drivers/net/bonding/bond_options.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>> index a8fde3bc458f..b93337b5a721 100644
>> --- a/drivers/net/bonding/bond_options.c
>> +++ b/drivers/net/bonding/bond_options.c
>> @@ -1526,7 +1526,7 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
>> 		mac = (u8 *)&newval->value;
>> 	}
>>
>> -	if (!is_valid_ether_addr(mac))
>> +	if (is_multicast_ether_addr(mac))
>> 		goto err;
>>
>> 	netdev_dbg(bond->dev, "Setting ad_actor_system to %pM\n", mac);
>> -- 
>> 2.30.2
>>
> 
> ---
> 	-Jay Vosburgh, jay.vosburgh@...onical.com
>