Re: ipv4: icmp: icmp_route_lookup() relookups pick wrong netdev with policy routing + strict rp_filter

[Eric Dumazet]

On Wed, Jul 15, 2026 at 5:30 PM Muhammad Ziad <muhzi100@xxxxxxxxx> wrote:
>
> 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 Ghazy

Thanks for the report.

It seems we are lacking more than saddr setting :/

Could you test

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 23e921d313b36b00d8ae5e14846527220c9db32b..6277e1bf85f304678ff167e8fd2b9239f155ce42
100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -548,10 +548,17 @@ static struct rtable *icmp_route_lookup(struct
net *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);