linux-kernel - 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z3-iHO1gqQt3T0Tl@fedora>
Date: Thu, 9 Jan 2025 10:17:00 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Jianbo Liu <jianbol@...dia.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 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.

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