mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v2] ipv4: fib: treat an unbuildable encapsulation as a nexthop mismatch
@ 2026-09-21 13:07 Gabriel Goller
  2026-09-21 15:11 ` Ido Schimmel
  0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Goller @ 2026-09-21 13:07 UTC (permalink / raw)
  To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Roopa Prabhu
  Cc: netdev, linux-kernel

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



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

* Re: [PATCH net v2] ipv4: fib: treat an unbuildable encapsulation as a nexthop mismatch
  2026-09-21 13:07 [PATCH net v2] ipv4: fib: treat an unbuildable encapsulation as a nexthop mismatch Gabriel Goller
@ 2026-09-21 15:11 ` Ido Schimmel
  2026-09-22  9:12   ` Gabriel Goller
  0 siblings, 1 reply; 3+ messages in thread
From: Ido Schimmel @ 2026-09-21 15:11 UTC (permalink / raw)
  To: Gabriel Goller
  Cc: David Ahern, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Roopa Prabhu, netdev, linux-kernel

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/

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

* Re: [PATCH net v2] ipv4: fib: treat an unbuildable encapsulation as a nexthop mismatch
  2026-09-21 15:11 ` Ido Schimmel
@ 2026-09-22  9:12   ` Gabriel Goller
  0 siblings, 0 replies; 3+ messages in thread
From: Gabriel Goller @ 2026-09-22  9:12 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: David Ahern, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Roopa Prabhu, netdev, linux-kernel

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


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

end of thread, other threads:[~2026-09-22  9:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 13:07 [PATCH net v2] ipv4: fib: treat an unbuildable encapsulation as a nexthop mismatch Gabriel Goller
2026-09-21 15:11 ` Ido Schimmel
2026-09-22  9:12   ` Gabriel Goller

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®