* Re: [PATCH net] ip6_gre: fix changelink conflict checks for L2 tunnels
2026-10-28 11:57 [PATCH net] ip6_gre: fix changelink conflict checks for L2 tunnels Xia Yan
@ 2026-10-30 15:16 ` Ido Schimmel
2026-10-31 2:44 ` Xia Yan
2026-11-01 12:13 ` netdev-bot+sashiko
1 sibling, 1 reply; 4+ messages in thread
From: Ido Schimmel @ 2026-10-30 15:16 UTC (permalink / raw)
To: Xia Yan
Cc: netdev, dsahern, davem, edumazet, kuba, pabeni, horms, kuniyu,
xeb, linux-kernel, stable, yana06070
On Wed, Oct 28, 2026 at 07:57:10PM +0800, Xia Yan wrote:
> ip6gre_changelink_common() calls ip6gre_tunnel_locate(), which only
> looks for ARPHRD_IP6GRE devices. The same changelink helper also handles
> ip6gretap and ip6erspan devices.
>
> This lets an L2 tunnel change its parameters to match another L2 tunnel,
> since the duplicate check looks for an L3 device instead. Incoming
> packets can then be delivered to the wrong interface. An L3 tunnel with
> matching parameters can also make a valid L2 update fail with EEXIST.
>
> Use ip6gre_tunnel_find() with dev->type, as the newlink paths do.
>
> Found while comparing the duplicate checks in newlink and changelink.
> Confirmed with ip link commands. For example, with an IPv6 underlay
> named under already configured:
>
> ip link add tap1 type ip6gretap local fd26:106::1 \
> remote fd26:106::2 key 100 dev under
> ip link add tap2 type ip6gretap local fd26:106::1 \
> remote fd26:106::2 key 200 dev under
> ip link set tap1 up
> ip link set tap2 up
> ip link set tap2 type ip6gretap key 100
>
> Before the fix, the last command succeeds and received frames move from
> tap1 to tap2. After the fix, it fails with EEXIST and tap2 is unchanged.
>
> Tested with standalone namespace tests in UML, with three runs each for
> ip6gretap and ip6erspan, and with L3 GRE IPv4/IPv6 traffic and normal
> tunnel updates. Full x86_64 W=1 builds with adjusted configurations
> completed before and after with no new diagnostics.
>
> Fixes: c12b395a4664 ("gre: Support GRE over IPv6")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
Diff looks OK, but this is not a regression [1][2][3]. Broken since 2012
and nobody complained. Please remove the Fixes and stable tags and
target net-next when it re-opens [4].
[1] https://lore.kernel.org/netdev/CAHk-=wiSnTE9vBZ=5_v+3EEkdazCCbBM5YABRzRHUAeRdyd4Xw@mail.gmail.com/
[2] https://lkml.org/lkml/2018/6/24/113
[3] https://lwn.net/Articles/1074171/
[4] https://netdev.bots.linux.dev/net-next.html
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] ip6_gre: fix changelink conflict checks for L2 tunnels
2026-10-28 11:57 [PATCH net] ip6_gre: fix changelink conflict checks for L2 tunnels Xia Yan
2026-10-30 15:16 ` Ido Schimmel
@ 2026-11-01 12:13 ` netdev-bot+sashiko
1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-11-01 12:13 UTC (permalink / raw)
To: yanxia
Cc: netdev, dsahern, idosch, davem, edumazet, kuba, pabeni, horms,
kuniyu, xeb, linux-kernel, stable, yana06070
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 (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] Changelink on a non-collect_md L2 tunnel (ip6gretap/ip6erspan)
can now fail with -EEXIST because an L2 collect_md ("external") tunnel…
Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
ip6gre_changelink_common() calls ip6_tnl_encap_setup(nt, &ipencap)…
--
ip6_gre: fix changelink conflict checks for L2 tunnels
This patch changes the duplicate check in ip6gre_changelink_common().
It used ip6gre_tunnel_locate(net, p_p, 0), which only matches ARPHRD_IP6GRE
devices. It now uses ip6gre_tunnel_find(net, p_p, dev->type), so
ip6gretap and ip6erspan devices are checked against other L2 tunnels.
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index b5d6ed8f0c369..df490798355de 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -2055,7 +2055,7 @@ ip6gre_changelink_common(struct net_device *dev, struct nlattr *tb[],
>
> ip6gre_netlink_parms(data, p_p);
>
> - t = ip6gre_tunnel_locate(net, p_p, 0);
> + t = ip6gre_tunnel_find(net, p_p, dev->type);
>
> if (t) {
> if (t->dev != dev)
[Severity: Medium]
Can this reject a valid changelink on a non-collect_md ip6gretap or
ip6erspan device when an "external" L2 tunnel has the same lookup key?
ip6gre_newlink() and ip6erspan_newlink() skip ip6gre_tunnel_find() for
collect_md devices. They still link those devices into the hash buckets:
net/ipv6/ip6_gre.c:ip6gre_newlink() {
...
if (nt->parms.collect_md) {
if (rtnl_dereference(ign->collect_md_tun))
err = -EEXIST;
} else {
if (ip6gre_tunnel_find(net, &nt->parms, dev->type))
err = -EEXIST;
}
...
ip6gre_tunnel_link(net_generic(net, ip6gre_net_id), nt);
...
}
ip6gre_tunnel_find() does not check collect_md and stops at the first
match:
hlist_for_each_entry(t, head, hash_node) {
if (ipv6_addr_equal(local, &t->parms.laddr) &&
ipv6_addr_equal(remote, &t->parms.raddr) &&
key == t->parms.i_key &&
link == t->parms.link &&
type == t->dev->type)
break;
}
ip6gre_tunnel_link() uses hlist_add_head_rcu(), so the first match is the
tunnel that was linked most recently.
For example, create a wildcard ip6gretap device tap0 with no remote. Then
create "md0 type ip6gretap external". Both devices end up in the same
wildcard bucket with all-zero parameters and type ARPHRD_ETHER.
A later changelink on tap0 that only changes ttl would go:
ip6gre_changelink()
ip6gre_changelink_common()
ip6gre_tunnel_find(net, p_p, ARPHRD_ETHER) -> returns md0
t->dev != dev -> ERR_PTR(-EEXIST)
Before this patch the lookup used ARPHRD_IP6GRE, so md0 could not match
and the update went through. The same happens for ip6erspan, and between
ip6gretap and ip6erspan, since both use ARPHRD_ETHER.
The commit message says changelink now does what the newlink paths do.
Newlink never runs this lookup for collect_md devices, though. Should
changelink handle collect_md tunnels in these buckets in some way, for
example by skipping entries with t->parms.collect_md set?
[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch, but
the new EEXIST cases here make it reachable in more situations.
ip6gre_changelink_common() applies the encap attributes to the live tunnel
before the duplicate check and the unregistering check run:
net/ipv6/ip6_gre.c:ip6gre_changelink_common() {
...
if (ip6gre_netlink_encap_parms(data, &ipencap)) {
int err = ip6_tnl_encap_setup(nt, &ipencap);
if (err < 0)
return ERR_PTR(err);
}
ip6gre_netlink_parms(data, p_p);
t = ip6gre_tunnel_find(net, p_p, dev->type);
...
}
ip6_tnl_encap_setup() clears t->encap, then rewrites t->encap,
t->encap_hlen and t->hlen.
Suppose the lookup then returns another tunnel (-EEXIST), or the tunnel
is unregistering (-ENODEV). rtnl_changelink() returns that error with no
rollback:
err = ops->changelink(dev, tb, data, extack);
if (err < 0)
return err;
Could a rejected request that has IFLA_GRE_ENCAP_* attributes (for
example ENCAP_TYPE=NONE on a FOU/GUE tunnel) plus conflicting addresses
or key still change the tunnel's encapsulation? Would it make sense to
run the conflict checks before ip6_tnl_encap_setup()?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028115710.27806-1-yanxia%40cmss.chinamobile.com
^ permalink raw reply [flat|nested] 4+ messages in thread