netdev - Re: [PATCH net 0/2] bond: fix xfrm offload feature during init

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: <459337c0-c625-4c5f-b668-19f24285ff19@nvidia.com>
Date: Thu, 9 Jan 2025 20:21:57 +0800
From: Jianbo Liu <jianbol@...dia.com>
To: Hangbin Liu <liuhangbin@...il.com>
CC: Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>, Jay Vosburgh
	<jv@...sburgh.net>, Andy Gospodarek <andy@...yhouse.net>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
	<pabeni@...hat.com>, Nikolay Aleksandrov <razor@...ckwall.org>, Simon Horman
	<horms@...nel.org>, Tariq Toukan <tariqt@...dia.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, Shuah Khan <shuah@...nel.org>, Steffen Klassert
	<steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>,
	Sabrina Dubroca <sd@...asysnail.net>, <linux-kselftest@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 0/2] bond: fix xfrm offload feature during init



On 1/9/2025 6:17 PM, Hangbin Liu wrote:
> On Thu, Jan 09, 2025 at 05:51:07PM +0800, Jianbo Liu wrote:
>>>> No, we don't need. But I am trying to understand what you said in your last
>>>> email about adding a new lock, or unlocking spin lock in
>>>
>>> I *thought* we need the spin lock in xfrm_state_delete(). So to protect xfrm_state,
>>
>> But not need in bond_ipsec_del_sa() because the state still hold by
>> xfrm_state_hold(), right?
> 
> Hmm, I'm not sure. If xfrm_state_hold() is safe. Why not just remove the spin
> lock in xfrm_state_delete(). This is more straightforward. e.g.
> 

We can't remove the spin lock in xfrm_state_delete(), but I think we can 
access the state while holding it, for example, checking (ipsec->xs == 
xs) as you mentioned before, because memory is not freed yet.

> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 67ca7ac955a3..150562abf513 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -784,9 +784,7 @@ int xfrm_state_delete(struct xfrm_state *x)
>   {
>          int err;
> 
> -       spin_lock_bh(&x->lock);
>          err = __xfrm_state_delete(x);
> -       spin_unlock_bh(&x->lock);
> 
>          return err;
>   }
> 
> We can even rename xfrm_state_delete() to xfrm_state_delete() directly.
> 
> Thanks
> Hangbin