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

["Zhou, Yun" <yun.zhou@windriver.com>]

On 9/20/26 14:34, netdev-bot+sashiko@kernel.org wrote:
> 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?
> 
Okay, I will add the prerequisite in v5.