Re: [PATCH net v4] net: erspan: set lltx to avoid sch_direct_xmit deadlock

[netdev-bot+sashiko@kernel.org]

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