netdev - Re: [PATCH net-next 4/5] bonding: use bond_set_slave

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <90d2aee8-45f9-6b3c-e34e-8c34cd980226@huawei.com>
Date: Thu, 10 Aug 2023 16:10:48 +0800
From: shaozhengchao <shaozhengchao@...wei.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, <netdev@...r.kernel.org>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>
CC: <j.vosburgh@...il.com>, <andy@...yhouse.net>, <weiyongjun1@...wei.com>,
	<yuehaibing@...wei.com>
Subject: Re: [PATCH net-next 4/5] bonding: use bond_set_slave_arr to simplify
 code



On 2023/8/10 8:17, Vadim Fedorenko wrote:
> On 09/08/2023 13:41, Zhengchao Shao wrote:
>> In bond_reset_slave_arr(), values are assigned and memory is released 
>> only
>> when the variables "usable" and "all" are not NULL. But even if the
>> "usable" and "all" variables are NULL, they can still work, because value
>> will be checked in kfree_rcu. Therefore, use bond_set_slave_arr() and set
>> the input parameters "usable_slaves" and "all_slaves" to NULL to simplify
>> the code in bond_reset_slave_arr(). And the same to bond_uninit().
>>
>> Signed-off-by: Zhengchao Shao <shaozhengchao@...wei.com>
>> ---
>>   drivers/net/bonding/bond_main.c | 29 +++--------------------------
>>   1 file changed, 3 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c 
>> b/drivers/net/bonding/bond_main.c
>> index 6636638f5d97..dcc67bd4d5cf 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -5044,21 +5044,9 @@ static void bond_set_slave_arr(struct bonding 
>> *bond,
>>       kfree_rcu(all, rcu);
>>   }
>> -static void bond_reset_slave_arr(struct bonding *bond)
>> +static inline void bond_reset_slave_arr(struct bonding *bond)
> 
> No explicit inline in c files. Remove it and let the compiler decide.
> 
Hi Vadim:
	Thank you for your review. I will remove it in v2.

Zhengchao Shao
>>   {
>> -    struct bond_up_slave *usable, *all;
>> -
>> -    usable = rtnl_dereference(bond->usable_slaves);
>> -    if (usable) {
>> -        RCU_INIT_POINTER(bond->usable_slaves, NULL);
>> -        kfree_rcu(usable, rcu);
>> -    }
>> -
>> -    all = rtnl_dereference(bond->all_slaves);
>> -    if (all) {
>> -        RCU_INIT_POINTER(bond->all_slaves, NULL);
>> -        kfree_rcu(all, rcu);
>> -    }
>> +    bond_set_slave_arr(bond, NULL, NULL);
>>   }
>>   /* Build the usable slaves array in control path for modes that use 
>> xmit-hash
>> @@ -5951,7 +5939,6 @@ void bond_setup(struct net_device *bond_dev)
>>   static void bond_uninit(struct net_device *bond_dev)
>>   {
>>       struct bonding *bond = netdev_priv(bond_dev);
>> -    struct bond_up_slave *usable, *all;
>>       struct list_head *iter;
>>       struct slave *slave;
>> @@ -5962,17 +5949,7 @@ static void bond_uninit(struct net_device 
>> *bond_dev)
>>           __bond_release_one(bond_dev, slave->dev, true, true);
>>       netdev_info(bond_dev, "Released all slaves\n");
>> -    usable = rtnl_dereference(bond->usable_slaves);
>> -    if (usable) {
>> -        RCU_INIT_POINTER(bond->usable_slaves, NULL);
>> -        kfree_rcu(usable, rcu);
>> -    }
>> -
>> -    all = rtnl_dereference(bond->all_slaves);
>> -    if (all) {
>> -        RCU_INIT_POINTER(bond->all_slaves, NULL);
>> -        kfree_rcu(all, rcu);
>> -    }
>> +    bond_set_slave_arr(bond, NULL, NULL);
>>       list_del(&bond->bond_list);