Re: ipv4: icmp: icmp_route_lookup() relookups pick wrong netdev with policy routing + strict rp_filter
From: Eric Dumazet <edumazet@google.com>
Date: 2026-07-16 01:51:05
Also in:
lkml
Subsystem:
networking [general], networking [ipv4/ipv6], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Ido Schimmel, Linus Torvalds
On Wed, Jul 15, 2026 at 5:43 PM Eric Dumazet [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Wed, Jul 15, 2026 at 5:30 PM Muhammad Ziad [off-list ref] wrote:quoted
Hello, There appears to be a bug in mainline Linux in ICMP reverse-path relookup logic inside icmp_route_lookup() (called by __icmp_send()) when strict rp_filter setting is in place. When Linux forwards a packet between two interfaces and needs to generate an ICMP error, icmp_route_lookup() performs a "secondary" reverse-path lookup to find a suitable route back towards the original source via ip_route_input(). To simulate the reverse path, the kernel derives the incoming netdev by calling ip_route_output_key() with a decoy flow that has *only* daddr assigned in it: struct flowi4 fl4_2 = {}; fl4_2.daddr = fl4_dec.saddr; rt2 = ip_route_output_key(net, &fl4_2); /* no saddr */ ... ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr, dscp, rt2->dst.dev); This can lead to a mismatch between rt2->dst.dev and the netdev the real reverse packet would use once routing policy rules are in effect. With strict rp_filter, passing the wrong netdev to ip_route_input() causes the relookup to fail and a "martian source" message to be logged, after which icmp_route_lookup() falls back to the earlier output route lookup (relookup_failed). In such a scenario, I would expect the relookup to use a canonical netdev and the kernel to not produce spurious "martian source" log messages as a result. This suggests the decoy flow would possibly need to carry saddr too so that ip_route_output_key() is able to resolve the right netdev. I tested this on: Ubuntu kernel 6.17.0-35-generic. Here is a reproducer script that sets up two net namespaces: a "forwarder" with two routes to the same dst in separate routing tables picked according to saddr, and a "sender" netns behind it which sends a ping with ttl=1 via the forwarder forcing it to generate an ICMP error as a response which leads to the result explained above: #!/bin/bash if [ "${forwarder_ns:-}" != "1" ]; then exec env forwarder_ns=1 unshare -Urn bash "$0" "$@" fi SRC=10.0.1.2 DST=198.51.100.5 # Current netns is the "forwarder". # Create a second namespace for the sender. unshare -n sleep 120 & cpid=$! trap 'kill "$cpid" 2>/dev/null || true' EXIT in_ns() { nsenter -t "$cpid" -n "$@"; } # veth r0(router) <-> s0(src) ip link add s0 type veth peer name r0 ip link set s0 netns "$cpid" ip link set lo up ip link set r0 up ip addr add 10.0.1.1/24 dev r0 ip link add dumA type dummy ip addr add 203.0.113.1/24 dev dumA ip link set dumA up ip link add dumB type dummy ip addr add 192.0.2.1/24 dev dumB ip link set dumB up sysctl -q -w net.ipv4.ip_forward=1 for c in all default r0 dumA dumB; do sysctl -q -w "net.ipv4.conf.$c.rp_filter=1" sysctl -q -w "net.ipv4.conf.$c.log_martians=1" done # Destination reachable two ways. # Policy rule diverts traffic FROM src to dumB ip route add 198.51.100.0/24 dev dumA ip route add 198.51.100.0/24 dev dumB table 100 ip rule add from "$SRC" lookup 100 # sender namespace setup. in_ns ip link set lo up in_ns ip link set s0 up in_ns ip addr add 10.0.1.2/24 dev s0 in_ns ip route add default via 10.0.1.1 # This will trigger a "martian source" log. in_ns ping -q -c1 -W2 -t1 "$DST" &>/dev/null Happy to test patches or provide additional traces, if needed. Thank you, Mohamed GhazyThanks for the report. It seems we are lacking more than saddr setting :/ Could you testdiff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 23e921d313b36b00d8ae5e14846527220c9db32b..6277e1bf85f304678ff167e8fd2b9239f155ce42100644--- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c@@ -548,10 +548,17 @@ static struct rtable *icmp_route_lookup(structnet *net, struct flowi4 *fl4, if (IS_ERR(rt2)) err = PTR_ERR(rt2); } else { - struct flowi4 fl4_2 = {}; + /* TODO: populate + .flowi4_dscp = dscp, + .flowi4_mark = mark, + .flowi4_uid = sock_net_uid(net, NULL), + */ + struct flowi4 fl4_2 = { + .daddr = fl4_dec.saddr, + .saddr = fl4_dec.daddr, + }; unsigned long orefdst; - fl4_2.daddr = fl4_dec.saddr; rt2 = ip_route_output_key(net, &fl4_2); if (IS_ERR(rt2)) { err = PTR_ERR(rt2);
I have tested the following fix, PTAL, thanks.
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 23e921d313b36b00d8ae5e14846527220c9db32b..2259f643cb0d43ea54819dd831c79807413e0d69100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c@@ -548,10 +548,16 @@ static struct rtable *icmp_route_lookup(structnet *net, struct flowi4 *fl4,
if (IS_ERR(rt2))
err = PTR_ERR(rt2);
} else {
- struct flowi4 fl4_2 = {};
+ struct flowi4 fl4_2 = {
+ .daddr = fl4_dec.saddr,
+ .saddr = fl4_dec.daddr,
+ .flowi4_dscp = dscp,
+ .flowi4_proto = IPPROTO_ICMP,
+ .flowi4_mark = mark,
+ .flowi4_uid = sock_net_uid(net, NULL),
+ };
unsigned long orefdst;
- fl4_2.daddr = fl4_dec.saddr;
rt2 = ip_route_output_key(net, &fl4_2);
if (IS_ERR(rt2)) {
err = PTR_ERR(rt2);