Re: ipv4: icmp: icmp_route_lookup() relookups pick wrong netdev with policy routing + strict rp_filter
From: Muhammad Ziad
Date: Thu Jul 16 2026 - 06:44:23 EST
Thank you for the fix, Eric. Applying the other selectors from skb_in
makes sense to me, but I'm not sure if instead we should copy them
from fl4_dec?
The mark param e.g. is gated by IP4_REPLY_MARK() on fwmark_reflect,
which could be different from skb_in->mark.
On Thu, Jul 16, 2026 at 3:51 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Wed, Jul 15, 2026 at 5:43 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> >
> > 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);
>
> I have tested the following fix, PTAL, thanks.
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 23e921d313b36b00d8ae5e14846527220c9db32b..2259f643cb0d43ea54819dd831c79807413e0d69
> 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -548,10 +548,16 @@ 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 = {};
> + 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);