mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock
@ 2026-09-16  6:13 Yun Zhou
  2026-09-16  8:20 ` Ido Schimmel
  2026-09-20  6:34 ` netdev-bot+sashiko
  0 siblings, 2 replies; 5+ messages in thread
From: Yun Zhou @ 2026-09-16  6:13 UTC (permalink / raw)
  To: dsahern, idosch, davem, edumazet, kuba, pabeni, horms
  Cc: netdev, linux-kernel, yun.zhou

erspan_xmit() re-enters the network stack via ip_tunnel_xmit(), causing
nested acquisition of _xmit_lock on the underlay device while already
holding the ERSPAN device's _xmit_lock, creating an ABBA deadlock:

  sch_direct_xmit [lock erspan] -> erspan_xmit -> ip_tunnel_xmit ->
  ip_output -> __dev_queue_xmit -> sch_direct_xmit [lock underlay]

Set dev->lltx = true so HARD_TX_LOCK() skips the spinlock for ERSPAN.
This is safe as erspan_xmit() has no shared mutable state: o_seqno is
atomic, TX stats are per-CPU u64_stats, dst_cache is per-CPU, and
o_flags is no longer modified in the xmit path since commit 9958e69b9893
("gre: fix ERSPAN o_flags race/corruption in xmit and fill_info").
GRETAP, the sibling device with identical xmit structure, already sets
lltx.

Reported-by: syzbot+9bda1b9fbb7fbdf9b62b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9bda1b9fbb7fbdf9b62b
Fixes: 84e54fe0a5ea ("gre: introduce native tunnel support for ERSPAN")
Fixes: 5a963eb61b7c ("ip6_gre: Add ERSPAN native tunnel support")
Cc: stable@vger.kernel.org
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
v4:
  - refine commit message

v3:
  - add fix for IPv6

v2:
  - change subject prefix to [PATCH net]

 net/ipv4/ip_gre.c  | 2 ++
 net/ipv6/ip6_gre.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 82309efd417e..0058cb20e36a 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1367,6 +1367,8 @@ static int erspan_tunnel_init(struct net_device *dev)
 	dev->features		|= GRE_FEATURES;
 	dev->hw_features	|= GRE_FEATURES;
 	dev->priv_flags		|= IFF_LIVE_ADDR_CHANGE;
+	/* Skip TX lock: xmit re-enters stack, risking ABBA with underlay */
+	dev->lltx = true;
 	netif_keep_dst(dev);
 
 	return ip_tunnel_init(dev);
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 8ebda0b6a78b..578fb3475f8c 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1871,6 +1871,8 @@ static int ip6erspan_tap_init(struct net_device *dev)
 		dev->mtu -= 8;
 
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	/* Skip TX lock: xmit re-enters stack, risking ABBA with underlay */
+	dev->lltx = true;
 	ip6erspan_tnl_link_config(tunnel, 1);
 
 	netdev_hold(dev, &tunnel->dev_tracker, GFP_KERNEL);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock
  2026-09-16  6:13 [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock Yun Zhou
@ 2026-09-16  8:20 ` Ido Schimmel
  2026-09-20  2:48   ` Zhou, Yun
  2026-09-20  6:34 ` netdev-bot+sashiko
  1 sibling, 1 reply; 5+ messages in thread
From: Ido Schimmel @ 2026-09-16  8:20 UTC (permalink / raw)
  To: Yun Zhou
  Cc: dsahern, davem, edumazet, kuba, pabeni, horms, netdev, linux-kernel

On Wed, Sep 16, 2026 at 02:13:14PM +0800, Yun Zhou wrote:
> erspan_xmit() re-enters the network stack via ip_tunnel_xmit(), causing
> nested acquisition of _xmit_lock on the underlay device while already
> holding the ERSPAN device's _xmit_lock, creating an ABBA deadlock:
> 
>   sch_direct_xmit [lock erspan] -> erspan_xmit -> ip_tunnel_xmit ->
>   ip_output -> __dev_queue_xmit -> sch_direct_xmit [lock underlay]
> 
> Set dev->lltx = true so HARD_TX_LOCK() skips the spinlock for ERSPAN.
> This is safe as erspan_xmit() has no shared mutable state: o_seqno is
> atomic, TX stats are per-CPU u64_stats, dst_cache is per-CPU, and
> o_flags is no longer modified in the xmit path since commit 9958e69b9893
> ("gre: fix ERSPAN o_flags race/corruption in xmit and fill_info").
> GRETAP, the sibling device with identical xmit structure, already sets
> lltx.

In v3 I asked that the commit message:

1. State that the overlay and underlay devices should be of the same
type (both erspan or both ip6erspan) for the splat to happen.

2. Mention the IPv6 fix. Currently you only describe the IPv4 path:
erspan_xmit() -> ip_tunnel_xmit()

https://lore.kernel.org/netdev/20260803151146.GA766007@shredder/

> 
> Reported-by: syzbot+9bda1b9fbb7fbdf9b62b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9bda1b9fbb7fbdf9b62b
> Fixes: 84e54fe0a5ea ("gre: introduce native tunnel support for ERSPAN")
> Fixes: 5a963eb61b7c ("ip6_gre: Add ERSPAN native tunnel support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> ---
> v4:
>   - refine commit message
> 
> v3:
>   - add fix for IPv6
> 
> v2:
>   - change subject prefix to [PATCH net]
> 
>  net/ipv4/ip_gre.c  | 2 ++
>  net/ipv6/ip6_gre.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 82309efd417e..0058cb20e36a 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1367,6 +1367,8 @@ static int erspan_tunnel_init(struct net_device *dev)
>  	dev->features		|= GRE_FEATURES;
>  	dev->hw_features	|= GRE_FEATURES;
>  	dev->priv_flags		|= IFF_LIVE_ADDR_CHANGE;
> +	/* Skip TX lock: xmit re-enters stack, risking ABBA with underlay */

Nit: I would drop this comment. It doesn't add anything and we don't
have it above similar assignments in the file.

> +	dev->lltx = true;
>  	netif_keep_dst(dev);
>  
>  	return ip_tunnel_init(dev);
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 8ebda0b6a78b..578fb3475f8c 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1871,6 +1871,8 @@ static int ip6erspan_tap_init(struct net_device *dev)
>  		dev->mtu -= 8;
>  
>  	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> +	/* Skip TX lock: xmit re-enters stack, risking ABBA with underlay */

Same.

> +	dev->lltx = true;
>  	ip6erspan_tnl_link_config(tunnel, 1);
>  
>  	netdev_hold(dev, &tunnel->dev_tracker, GFP_KERNEL);
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock
  2026-09-16  8:20 ` Ido Schimmel
@ 2026-09-20  2:48   ` Zhou, Yun
  2026-09-20  3:00     ` Zhou, Yun
  0 siblings, 1 reply; 5+ messages in thread
From: Zhou, Yun @ 2026-09-20  2:48 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: dsahern, davem, edumazet, kuba, pabeni, horms, netdev, linux-kernel

On 9/16/26 16:20, Ido Schimmel wrote:
> On Wed, Sep 16, 2026 at 02:13:14PM +0800, Yun Zhou wrote:
>> erspan_xmit() re-enters the network stack via ip_tunnel_xmit(), causing
>> nested acquisition of _xmit_lock on the underlay device while already
>> holding the ERSPAN device's _xmit_lock, creating an ABBA deadlock:
>>
>>    sch_direct_xmit [lock erspan] -> erspan_xmit -> ip_tunnel_xmit ->
>>    ip_output -> __dev_queue_xmit -> sch_direct_xmit [lock underlay]
>>
>> Set dev->lltx = true so HARD_TX_LOCK() skips the spinlock for ERSPAN.
>> This is safe as erspan_xmit() has no shared mutable state: o_seqno is
>> atomic, TX stats are per-CPU u64_stats, dst_cache is per-CPU, and
>> o_flags is no longer modified in the xmit path since commit 9958e69b9893
>> ("gre: fix ERSPAN o_flags race/corruption in xmit and fill_info").
>> GRETAP, the sibling device with identical xmit structure, already sets
>> lltx.
> 
> In v3 I asked that the commit message:
> 
> 1. State that the overlay and underlay devices should be of the same
> type (both erspan or both ip6erspan) for the splat to happen.
> 
The devices do not need to be the same type. The actual syzbot report 
[1] is between IPv4 erspan and vti6 - two completely different tunnel types:

   -> #1: erspan_xmit (net/ipv4/ip_gre.c)   holding &qdisc_xmit_lock_key#2
   -> #0: vti6_tnl_xmit (net/ipv6/ip6_vti.c) holding &qdisc_xmit_lock_key#3

I can reproduce this reliably with erspan <-> vti6 cross-routed in a 
netns (no same-type stacking involved).

Additionally, I think it makes sense to set lltx=true for vti6 as well.

[1] https://syzkaller.appspot.com/text?tag=CrashReport&x=15b29287180000

Thanks,
Yun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock
  2026-09-20  2:48   ` Zhou, Yun
@ 2026-09-20  3:00     ` Zhou, Yun
  0 siblings, 0 replies; 5+ messages in thread
From: Zhou, Yun @ 2026-09-20  3:00 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: dsahern, davem, edumazet, kuba, pabeni, horms, netdev, linux-kernel



On 9/20/26 10:48, Zhou, Yun wrote:
> On 9/16/26 16:20, Ido Schimmel wrote:
>> On Wed, Sep 16, 2026 at 02:13:14PM +0800, Yun Zhou wrote:
>>> erspan_xmit() re-enters the network stack via ip_tunnel_xmit(), causing
>>> nested acquisition of _xmit_lock on the underlay device while already
>>> holding the ERSPAN device's _xmit_lock, creating an ABBA deadlock:
>>>
>>>    sch_direct_xmit [lock erspan] -> erspan_xmit -> ip_tunnel_xmit ->
>>>    ip_output -> __dev_queue_xmit -> sch_direct_xmit [lock underlay]
>>>
>>> Set dev->lltx = true so HARD_TX_LOCK() skips the spinlock for ERSPAN.
>>> This is safe as erspan_xmit() has no shared mutable state: o_seqno is
>>> atomic, TX stats are per-CPU u64_stats, dst_cache is per-CPU, and
>>> o_flags is no longer modified in the xmit path since commit 9958e69b9893
>>> ("gre: fix ERSPAN o_flags race/corruption in xmit and fill_info").
>>> GRETAP, the sibling device with identical xmit structure, already sets
>>> lltx.
>>
>> In v3 I asked that the commit message:
>>
>> 1. State that the overlay and underlay devices should be of the same
>> type (both erspan or both ip6erspan) for the splat to happen.
>>
> The devices do not need to be the same type. The actual syzbot report 
> [1] is between IPv4 erspan and vti6 - two completely different tunnel 
> types:
> 

More precisely, any pair of 'locked + re-entering' tunnel devices whose
underlying paths intersect could trigger this issue — for example,
gre{6} (which previously didn't have lltx set correctly), and currently
erspan{6}, vti6, etc. Same-type pairing is just one possible
combination, not a requirement.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock
  2026-09-16  6:13 [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock Yun Zhou
  2026-09-16  8:20 ` Ido Schimmel
@ 2026-09-20  6:34 ` netdev-bot+sashiko
  1 sibling, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-20  6:34 UTC (permalink / raw)
  To: yun.zhou
  Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, netdev,
	linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch enables `dev->lltx = true` for both IPv4 ERSPAN
  (`erspan_tunnel_init()`, net/ipv4/ip_gre.c:1371) and IPv6 ERSPAN…

--

net: erspan: set lltx to avoid sch_direct_xmit deadlock

This sets dev->lltx = true for the IPv4 and IPv6 ERSPAN tunnel devices so
that HARD_TX_LOCK() skips the txq _xmit_lock, avoiding the nested
_xmit_lock acquisition when erspan_xmit() re-enters the stack through the
underlay device.  The change is tagged for stable with Fixes tags pointing
at 84e54fe0a5ea and 5a963eb61b7c.

Closes: https://syzkaller.appspot.com/bug?extid=9bda1b9fbb7fbdf9b62b

> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 82309efd417e0..0058cb20e36a1 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1367,6 +1367,8 @@ static int erspan_tunnel_init(struct net_device *dev)
>  	dev->features		|= GRE_FEATURES;
>  	dev->hw_features	|= GRE_FEATURES;
>  	dev->priv_flags		|= IFF_LIVE_ADDR_CHANGE;
> +	/* Skip TX lock: xmit re-enters stack, risking ABBA with underlay */
> +	dev->lltx = true;
>  	netif_keep_dst(dev);
>  
>  	return ip_tunnel_init(dev);

[Severity: Medium]
The commit message justifies the change with:

  "This is safe as erspan_xmit() has no shared mutable state: o_seqno is
   atomic, TX stats are per-CPU u64_stats, dst_cache is per-CPU, and
   o_flags is no longer modified in the xmit path since commit 9958e69b9893"

That statement holds in this tree, but the patch also carries Cc:
stable@vger.kernel.org with Fixes tags on 84e54fe0a5ea and 5a963eb61b7c.
Commit 9958e69b9893 ("gre: fix ERSPAN o_flags race/corruption in xmit and
fill_info") has neither a Fixes tag nor Cc: stable, so it is not queued for
any stable tree.

Both hunks here only touch the *_init() functions, so they apply cleanly to
any tree that has the dev->lltx bitfield (6.12 and later) even when
9958e69b9893 is absent.  In such a tree erspan_xmit() still does the
read-modify-write on the shared per-tunnel word:

net/ipv4/ip_gre.c:erspan_xmit() (pre-9958e69b9893) {
	...
	__clear_bit(IP_TUNNEL_SEQ_BIT, tunnel->parms.o_flags);
	...
	__clear_bit(IP_TUNNEL_KEY_BIT, tunnel->parms.o_flags);
	__gre_xmit(skb, dev, &tunnel->parms.iph, proto);
	...
}

Once lltx is set, is anything left serializing two concurrent
erspan_xmit() calls on the same single-queue device, given that the txq
_xmit_lock was the only thing keeping those __clear_bit() sequences from
overlapping?  The same question applies to the race against
erspan_fill_info(), which in those trees sets IP_TUNNEL_KEY_BIT back into
tunnel->parms.o_flags under RTNL only.

If a lost update resurrects IP_TUNNEL_KEY_BIT or IP_TUNNEL_SEQ_BIT, can
gre_build_header() then emit optional GRE fields that tunnel->tun_hlen
never accounted for, i.e. exactly the header corruption described in
9958e69b9893?

Would it make sense to name 9958e69b9893 as an explicit stable
prerequisite in a note below the --- line, or to fold the equivalent
o_flags localization into this series, so stable maintainers do not apply
the lltx change on its own?

> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 8ebda0b6a78b2..578fb3475f8c2 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1871,6 +1871,8 @@ static int ip6erspan_tap_init(struct net_device *dev)
>  		dev->mtu -= 8;
>  
>  	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> +	/* Skip TX lock: xmit re-enters stack, risking ABBA with underlay */
> +	dev->lltx = true;
>  	ip6erspan_tnl_link_config(tunnel, 1);

The same concern applies on the IPv6 side: in trees without 9958e69b9893,
ip6erspan_tunnel_xmit() still runs

	__clear_bit(IP_TUNNEL_KEY_BIT, t->parms.o_flags);

on the shared parms word, and ip6gre_fill_info() reads it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916061314.936440-1-yun.zhou%40windriver.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-20  6:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16  6:13 [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock Yun Zhou
2026-09-16  8:20 ` Ido Schimmel
2026-09-20  2:48   ` Zhou, Yun
2026-09-20  3:00     ` Zhou, Yun
2026-09-20  6:34 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®