linux-kernel - Re: [PATCH net-next] xdp: create locked/unlocked instances of xdp redirect target setters

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: <aAfUX_jDZpe4Vx_M@mini-arch>
Date: Tue, 22 Apr 2025 10:39:43 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Joshua Washington <joshwash@...gle.com>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org,
	Mina Almasry <almasrymina@...gle.com>,
	Willem de Bruijn <willemb@...gle.com>,
	Harshitha Ramamurthy <hramamurthy@...gle.com>,
	Jeroen de Borst <jeroendb@...gle.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Simon Horman <horms@...nel.org>,
	Praveen Kaligineedi <pkaligineedi@...gle.com>,
	Shailend Chand <shailend@...gle.com>,
	Stanislav Fomichev <sdf@...ichev.me>,
	Martin KaFai Lau <martin.lau@...nel.org>,
	Joe Damato <jdamato@...tly.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] xdp: create locked/unlocked instances of xdp
 redirect target setters

On 04/21, Joshua Washington wrote:
> Commit 03df156dd3a6 ("xdp: double protect netdev->xdp_flags with
> netdev->lock") introduces the netdev lock to xdp_set_features_flag().
> The change includes a _locked version of the method, as it is possible
> for a driver to have already acquired the netdev lock before calling
> this helper. However, the same applies to
> xdp_features_(set|clear)_redirect_flags(), which ends up calling the
> unlocked version of xdp_set_features_flags() leading to deadlocks in
> GVE, which grabs the netdev lock as part of its suspend, reset, and
> shutdown processes:
> 
> [  833.265543] WARNING: possible recursive locking detected
> [  833.270949] 6.15.0-rc1 #6 Tainted: G            E
> [  833.276271] --------------------------------------------
> [  833.281681] systemd-shutdow/1 is trying to acquire lock:
> [  833.287090] ffff949d2b148c68 (&dev->lock){+.+.}-{4:4}, at: xdp_set_features_flag+0x29/0x90
> [  833.295470]
> [  833.295470] but task is already holding lock:
> [  833.301400] ffff949d2b148c68 (&dev->lock){+.+.}-{4:4}, at: gve_shutdown+0x44/0x90 [gve]
> [  833.309508]
> [  833.309508] other info that might help us debug this:
> [  833.316130]  Possible unsafe locking scenario:
> [  833.316130]
> [  833.322142]        CPU0
> [  833.324681]        ----
> [  833.327220]   lock(&dev->lock);
> [  833.330455]   lock(&dev->lock);
> [  833.333689]
> [  833.333689]  *** DEADLOCK ***
> [  833.333689]
> [  833.339701]  May be due to missing lock nesting notation
> [  833.339701]
> [  833.346582] 5 locks held by systemd-shutdow/1:
> [  833.351205]  #0: ffffffffa9c89130 (system_transition_mutex){+.+.}-{4:4}, at: __se_sys_reboot+0xe6/0x210
> [  833.360695]  #1: ffff93b399e5c1b8 (&dev->mutex){....}-{4:4}, at: device_shutdown+0xb4/0x1f0
> [  833.369144]  #2: ffff949d19a471b8 (&dev->mutex){....}-{4:4}, at: device_shutdown+0xc2/0x1f0
> [  833.377603]  #3: ffffffffa9eca050 (rtnl_mutex){+.+.}-{4:4}, at: gve_shutdown+0x33/0x90 [gve]
> [  833.386138]  #4: ffff949d2b148c68 (&dev->lock){+.+.}-{4:4}, at: gve_shutdown+0x44/0x90 [gve]
> 
> Introduce xdp_features_(set|clear)_redirect_target_locked() versions
> which assume that the netdev lock has already been acquired before
> setting the XDP feature flag and update GVE to use the locked version.
> 
> Cc: bpf@...r.kernel.org
> Fixes: 03df156dd3a6 ("xdp: double protect netdev->xdp_flags with netdev->lock")
> Tested-by: Mina Almasry <almasrymina@...gle.com>
> Reviewed-by: Willem de Bruijn <willemb@...gle.com>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@...gle.com>
> Signed-off-by: Joshua Washington <joshwash@...gle.com>

Acked-by: Stanislav Fomichev <sdf@...ichev.me>