netdev - Re: [PATCH net-next v2 1/3] net: add check for current MAC address in dev_set_mac

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: <1e2404ff-232b-5999-cdb2-4205c58b071f@molgen.mpg.de>
Date: Tue, 13 Jun 2023 15:23:30 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
 Piotr Gardocki <piotrx.gardocki@...el.com>
Cc: netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
 przemyslaw.kitszel@...el.com, michal.swiatkowski@...ux.intel.com,
 kuba@...nel.org, anthony.l.nguyen@...el.com, simon.horman@...igine.com,
 aleksander.lobakin@...el.com
Subject: Re: [PATCH net-next v2 1/3] net: add check for current MAC address in
 dev_set_mac_address

Dear Maciej,


Am 13.06.23 um 15:10 schrieb Maciej Fijalkowski:
> On Tue, Jun 13, 2023 at 02:24:18PM +0200, Piotr Gardocki wrote:
>> In some cases it is possible for kernel to come with request
>> to change primary MAC address to the address that is already
>> set on the given interface.
>>
>> This patch adds proper check to return fast from the function
>> in these cases.
> 
> Please use imperative mood here - "add proper check..."

Just a note, that in “add check …” the verb *add* is already in 
imperative mood. (You can shorten “add noun …” often to just use the 
verb for the noun. In this case:

Check current MAC address in `dev_set_mac_address`

But it’s not specific enough. Maybe:

Avoid setting same MAC address


Kind regards,

Paul


>> An example of such case is adding an interface to bonding
>> channel in balance-alb mode:
>> modprobe bonding mode=balance-alb miimon=100 max_bonds=1
>> ip link set bond0 up
>> ifenslave bond0 <eth>
>>
>> Signed-off-by: Piotr Gardocki <piotrx.gardocki@...el.com>
> 
> I'll let Kuba ack it.
> 
>> ---
>>   net/core/dev.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index c2456b3667fe..8f1c49ab17df 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -8754,6 +8754,8 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa,
>>   		return -EINVAL;
>>   	if (!netif_device_present(dev))
>>   		return -ENODEV;
>> +	if (!memcmp(dev->dev_addr, sa->sa_data, dev->addr_len))
>> +		return 0;
>>   	err = dev_pre_changeaddr_notify(dev, sa->sa_data, extack);
>>   	if (err)
>>   		return err;
>> -- 
>> 2.34.1
>>