net/ipv4/fib_semantics.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
fib_encap_match() builds the requested lwtunnel state and compares it
against the nexthop of a candidate route. When lwtunnel_build_state()
failed it left result at 0, which is interpreted as "the nexthop
matches", so fib_nh_match() continues to compare only oif and gateway.
So if there comes along a RTM_DELROUTE which carries an encapsulation
the kernel rejects, it could delete a different route with a different
encapsulation.
Report a mismatch instead. This also covers LWTUNNEL_ENCAP_NONE, which
lwtunnel_build_state() rejects with -EINVAL, so the separate check for
it can go away. It used to claim a match for an encapsulation type the
kernel refuses to build.
Fixes: 571e722676fe ("ipv4: support for fib route lwtunnel encap attributes")
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
v1 (https://lore.kernel.org/netdev/20260918152836.1173368-1-g.goller@proxmox.com/):
* removed the LWTUNNEL_ENCAP_NONE check as it is in lwtunnel_build_state
already (thanks @Ido)
net/ipv4/fib_semantics.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 50e96f86ca59..001da9fc86cc 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -911,17 +911,14 @@ static int fib_encap_match(struct net *net, u16 encap_type,
struct netlink_ext_ack *extack)
{
struct lwtunnel_state *lwtstate;
- int ret, result = 0;
+ int result;
- if (encap_type == LWTUNNEL_ENCAP_NONE)
- return 0;
+ if (lwtunnel_build_state(net, encap_type, encap, AF_INET, cfg,
+ &lwtstate, extack))
+ return 1;
- ret = lwtunnel_build_state(net, encap_type, encap, AF_INET,
- cfg, &lwtstate, extack);
- if (!ret) {
- result = lwtunnel_cmp_encap(lwtstate, nh->fib_nh_lws);
- lwtstate_free(lwtstate);
- }
+ result = lwtunnel_cmp_encap(lwtstate, nh->fib_nh_lws);
+ lwtstate_free(lwtstate);
return result;
}
--
2.47.3
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · 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).
- [High] Incomplete fix: the encapsulation selector of an RTM_DELROUTE is
still ignored on two reachable paths, so the exact outcome the commit…
--
ipv4: fib: treat an unbuildable encapsulation as a nexthop mismatch
fib_encap_match() used to leave its result at 0 when lwtunnel_build_state()
failed, which fib_nh_match() reads as "the nexthop matches". The patch
returns a mismatch in that case and drops the now redundant
LWTUNNEL_ENCAP_NONE test, since lwtunnel_build_state() rejects that type
itself.
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 50e96f86ca59a..001da9fc86cc3 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -911,17 +911,14 @@ static int fib_encap_match(struct net *net, u16 encap_type,
> struct netlink_ext_ack *extack)
> {
> struct lwtunnel_state *lwtstate;
> - int ret, result = 0;
> + int result;
>
> - if (encap_type == LWTUNNEL_ENCAP_NONE)
> - return 0;
> + if (lwtunnel_build_state(net, encap_type, encap, AF_INET, cfg,
> + &lwtstate, extack))
> + return 1;
>
> - ret = lwtunnel_build_state(net, encap_type, encap, AF_INET,
> - cfg, &lwtstate, extack);
> - if (!ret) {
> - result = lwtunnel_cmp_encap(lwtstate, nh->fib_nh_lws);
> - lwtstate_free(lwtstate);
> - }
> + result = lwtunnel_cmp_encap(lwtstate, nh->fib_nh_lws);
> + lwtstate_free(lwtstate);
>
> return result;
> }
[Severity: High]
The change to fib_encap_match() looks right on its own, but does it actually
cover the case described in the changelog?
The commit message says:
So if there comes along a RTM_DELROUTE which carries an encapsulation
the kernel rejects, it could delete a different route with a different
encapsulation.
fib_encap_match() has a single caller, and that call is nested inside a test
for an output interface or a gateway in fib_nh_match():
net/ipv4/fib_semantics.c:fib_nh_match() {
...
if (cfg->fc_oif || cfg->fc_gw_family) {
struct fib_nh *nh;
nh = fib_info_nh(fi, 0);
if (cfg->fc_encap) {
if (fib_encap_match(net, cfg->fc_encap_type,
cfg->fc_encap, nh, cfg, extack))
return 1;
}
...
}
rtm_to_fib_config() records RTA_ENCAP unconditionally and does not require
RTA_OIF or RTA_GATEWAY next to it:
net/ipv4/fib_frontend.c:rtm_to_fib_config() {
...
case RTA_ENCAP:
cfg->fc_encap = attr;
break;
...
}
So for an RTM_DELROUTE carrying only RTA_DST plus RTA_ENCAP (and no
RTA_OIF, no RTA_GATEWAY, no RTA_MULTIPATH), isn't the whole block above
skipped, leaving fib_nh_match() to fall through to:
net/ipv4/fib_semantics.c:fib_nh_match() {
...
#ifdef CONFIG_IP_ROUTE_MULTIPATH
if (!cfg->fc_mp)
return 0;
...
}
and fib_table_delete() then picks that alias?
net/ipv4/fib_trie.c:fib_table_delete() {
...
fib_nh_match(net, cfg, fi, extack) == 0 &&
fib_metrics_match(cfg, fi)) {
fa_to_delete = fa;
...
}
Two aliases with different lwtunnel encapsulation can coexist, since
nh_comp() distinguishes them via lwtunnel_cmp_encap(), for example:
ip route append 10.0.0.0/24 encap ip id 1 dst A dev eth0
ip route append 10.0.0.0/24 encap ip id 2 dst B dev eth0
A delete request for 10.0.0.0/24 carrying the id 2 encapsulation, or an
RTA_ENCAP with no RTA_ENCAP_TYPE (encap_type == LWTUNNEL_ENCAP_NONE, which
lwtunnel_build_state() rejects with -EINVAL), and no oif or gateway, would
then remove the id 1 route instead. Should the encap comparison be moved
out from under the cfg->fc_oif || cfg->fc_gw_family test so this path is
covered too?
A second path looks similar. The RTA_MULTIPATH loop in fib_nh_match()
compares rtnh_ifindex, RTA_GATEWAY/RTA_VIA and, with
CONFIG_IP_ROUTE_CLASSID, RTA_FLOW, but never the nested RTA_ENCAP and
RTA_ENCAP_TYPE:
net/ipv4/fib_semantics.c:fib_nh_match() {
...
for_nexthops(fi) {
int attrlen;
if (!rtnh_ok(rtnh, remaining))
return -EINVAL;
if (rtnh->rtnh_ifindex && rtnh->rtnh_ifindex != nh->fib_nh_oif)
return 1;
...
}
The add side does consume those attributes in fib_get_nhs():
net/ipv4/fib_semantics.c:fib_get_nhs() {
...
fib_cfg.fc_encap = nla_find(attrs, attrlen, RTA_ENCAP);
...
}
and rtm_to_fib_config() only validates the nested encapsulation type
number, never the payload:
net/ipv4/fib_frontend.c:rtm_to_fib_config() {
...
case RTA_MULTIPATH:
err = lwtunnel_valid_encap_type_attr(nla_data(attr),
nla_len(attr),
extack);
...
}
Does this make ECMP routes that differ only in per-nexthop encapsulation
interchangeable for deletion?
If relocating the check and teaching the multipath loop about RTA_ENCAP is
out of scope here, could the changelog be narrowed to say the fix applies
only to requests that also carry an oif or a gateway, so backporters do not
read it as closing the whole case?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921130736.210845-1-g.goller%40proxmox.com
On Mon, Sep 21, 2026 at 03:07:30PM +0200, Gabriel Goller wrote:
> fib_encap_match() builds the requested lwtunnel state and compares it
> against the nexthop of a candidate route. When lwtunnel_build_state()
> failed it left result at 0, which is interpreted as "the nexthop
> matches", so fib_nh_match() continues to compare only oif and gateway.
>
> So if there comes along a RTM_DELROUTE which carries an encapsulation
> the kernel rejects, it could delete a different route with a different
> encapsulation.
>
> Report a mismatch instead. This also covers LWTUNNEL_ENCAP_NONE, which
> lwtunnel_build_state() rejects with -EINVAL, so the separate check for
> it can go away. It used to claim a match for an encapsulation type the
> kernel refuses to build.
>
> Fixes: 571e722676fe ("ipv4: support for fib route lwtunnel encap attributes")
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
I asked Claude to check if this can result in routes that are no longer
deleted and it came up with the following scenario which I verified:
Before:
# ip link add name dummy1 up type dummy
# ip route add 192.0.2.0/24 encap bpf xmit obj ./lwt_ok.o sec xmit dev dummy1
# ip route flush dev dummy1
# ip route show
After:
# ip link add name dummy1 up type dummy
# ip route add 192.0.2.0/24 encap bpf xmit obj ./lwt_ok.o sec xmit dev dummy1
# ip route flush dev dummy1
Failed to send flush request: No such process
# ip route show
192.0.2.0/24 encap bpf xmit lwt_ok.o:[xmit] dev dummy1 scope link
The flush is dump followed by delete and bpf_fill_encap_info() doesn't
fill enough information for bpf_build_state() to reconstruct the state
and it returns an error.
I don't know if anyone is relying on this behavior, but AFAIK nobody
complained about the issue that this patch is fixing for 11 years and
according to [1] you didn't hit it either. Given the above and the fact
that the modern alternative (nexthop objects) avoids this issue, I'm
tempted to keep the code as-is.
[1] https://lore.kernel.org/netdev/arEQHeBlqKWCu3l5@luna.proxmox.com/
On 21.09.2026 18:11, Ido Schimmel wrote:
> On Mon, Sep 21, 2026 at 03:07:30PM +0200, Gabriel Goller wrote:
> > fib_encap_match() builds the requested lwtunnel state and compares it
> > against the nexthop of a candidate route. When lwtunnel_build_state()
> > failed it left result at 0, which is interpreted as "the nexthop
> > matches", so fib_nh_match() continues to compare only oif and gateway.
> >
> > So if there comes along a RTM_DELROUTE which carries an encapsulation
> > the kernel rejects, it could delete a different route with a different
> > encapsulation.
> >
> > Report a mismatch instead. This also covers LWTUNNEL_ENCAP_NONE, which
> > lwtunnel_build_state() rejects with -EINVAL, so the separate check for
> > it can go away. It used to claim a match for an encapsulation type the
> > kernel refuses to build.
> >
> > Fixes: 571e722676fe ("ipv4: support for fib route lwtunnel encap attributes")
> > Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>
> I asked Claude to check if this can result in routes that are no longer
> deleted and it came up with the following scenario which I verified:
>
> Before:
>
> # ip link add name dummy1 up type dummy
> # ip route add 192.0.2.0/24 encap bpf xmit obj ./lwt_ok.o sec xmit dev dummy1
> # ip route flush dev dummy1
> # ip route show
>
> After:
>
> # ip link add name dummy1 up type dummy
> # ip route add 192.0.2.0/24 encap bpf xmit obj ./lwt_ok.o sec xmit dev dummy1
> # ip route flush dev dummy1
> Failed to send flush request: No such process
> # ip route show
> 192.0.2.0/24 encap bpf xmit lwt_ok.o:[xmit] dev dummy1 scope link
>
> The flush is dump followed by delete and bpf_fill_encap_info() doesn't
> fill enough information for bpf_build_state() to reconstruct the state
> and it returns an error.
>
> I don't know if anyone is relying on this behavior, but AFAIK nobody
> complained about the issue that this patch is fixing for 11 years and
> according to [1] you didn't hit it either. Given the above and the fact
> that the modern alternative (nexthop objects) avoids this issue, I'm
> tempted to keep the code as-is.
>
> [1] https://lore.kernel.org/netdev/arEQHeBlqKWCu3l5@luna.proxmox.com/
Ah, I missed the whole `ip route flush` path. Makes sense, we can drop this
patch. Notably because ipv6 also doesn't check the encap value :)
Thanks
Gabriel
© 2016 - 2026 Red Hat, Inc.