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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <b5ccef63-4c16-0371-6dda-b3d1f9dfa5d6@linux.dev>
Date: Thu, 10 Aug 2023 01:17:55 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Zhengchao Shao <shaozhengchao@...wei.com>, 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 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.

>   {
> -	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);
>   
-- 
pw-bot: cr