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

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

[Muhammad Ziad]

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

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@gmail.com> 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);

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:43 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Jul 15, 2026 at 5:30 PM Muhammad Ziad <muhzi100@gmail.com> 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);

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

[Muhammad Ziad]

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@google.com> wrote:
>
> On Wed, Jul 15, 2026 at 5:43 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Jul 15, 2026 at 5:30 PM Muhammad Ziad <muhzi100@gmail.com> 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);

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

[Eric Dumazet]

On Thu, Jul 16, 2026 at 12:40 PM Muhammad Ziad <muhzi100@gmail.com> wrote:
>
> 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.
>

Yes, sashiko had some remarks.

I have been playing with:

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 23e921d313b36b00d8ae5e14846527220c9db32b..b0eb4f8ff9867499dc3a96c92a414440b2d3a115
100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -548,10 +548,12 @@ 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 = fl4_dec;
                unsigned long orefdst;

                fl4_2.daddr = fl4_dec.saddr;
+               fl4_2.saddr = fl4_dec.daddr;
+               fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
                rt2 = ip_route_output_key(net, &fl4_2);
                if (IS_ERR(rt2)) {
                        err = PTR_ERR(rt2);


>
> On Thu, Jul 16, 2026 at 3:51 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Jul 15, 2026 at 5:43 PM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Wed, Jul 15, 2026 at 5:30 PM Muhammad Ziad <muhzi100@gmail.com> 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);

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

[Eric Dumazet]

On Thu, Jul 16, 2026 at 1:22 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Jul 16, 2026 at 12:40 PM Muhammad Ziad <muhzi100@gmail.com> wrote:
> >
> > 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.
> >
>
> Yes, sashiko had some remarks.
>
> I have been playing with:
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 23e921d313b36b00d8ae5e14846527220c9db32b..b0eb4f8ff9867499dc3a96c92a414440b2d3a115
> 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -548,10 +548,12 @@ 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 = fl4_dec;
>                 unsigned long orefdst;
>
>                 fl4_2.daddr = fl4_dec.saddr;
> +               fl4_2.saddr = fl4_dec.daddr;
> +               fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
>                 rt2 = ip_route_output_key(net, &fl4_2);
>                 if (IS_ERR(rt2)) {
>                         err = PTR_ERR(rt2);
>
>

Or even better:

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 23e921d313b36b00d8ae5e14846527220c9db32b..aaaa0e347702c4412ff515a9d0589c79c69c62d1
100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -548,10 +548,13 @@ 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 = fl4_dec;
                unsigned long orefdst;

-               fl4_2.daddr = fl4_dec.saddr;
+               swap(fl4_2.daddr, fl4_2.saddr);
+               swap(fl4_2.fl4_sport, fl4_2.fl4_dport);
+
+               fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
                rt2 = ip_route_output_key(net, &fl4_2);
                if (IS_ERR(rt2)) {
                        err = PTR_ERR(rt2);

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

[Muhammad Ziad]

Seems ANYSRC flag is required for ip_route_output_key() to not fail
with an error:

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 23e921d313b3..c2428fa21b07 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -548,10 +548,15 @@ 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 = fl4_dec;
                unsigned long orefdst;

-               fl4_2.daddr = fl4_dec.saddr;
+               swap(fl4_2.daddr, fl4_2.saddr);
+               swap(fl4_2.fl4_sport, fl4_2.fl4_dport);
+
+               fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
+               fl4_2.flowi4_flags |= FLOWI_FLAG_ANYSRC;
+
                rt2 = ip_route_output_key(net, &fl4_2);
                if (IS_ERR(rt2)) {
                        err = PTR_ERR(rt2);


On Thu, Jul 16, 2026 at 2:29 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Jul 16, 2026 at 1:22 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Jul 16, 2026 at 12:40 PM Muhammad Ziad <muhzi100@gmail.com> wrote:
> > >
> > > 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.
> > >
> >
> > Yes, sashiko had some remarks.
> >
> > I have been playing with:
> >
> > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > index 23e921d313b36b00d8ae5e14846527220c9db32b..b0eb4f8ff9867499dc3a96c92a414440b2d3a115
> > 100644
> > --- a/net/ipv4/icmp.c
> > +++ b/net/ipv4/icmp.c
> > @@ -548,10 +548,12 @@ 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 = fl4_dec;
> >                 unsigned long orefdst;
> >
> >                 fl4_2.daddr = fl4_dec.saddr;
> > +               fl4_2.saddr = fl4_dec.daddr;
> > +               fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
> >                 rt2 = ip_route_output_key(net, &fl4_2);
> >                 if (IS_ERR(rt2)) {
> >                         err = PTR_ERR(rt2);
> >
> >
>
> Or even better:
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 23e921d313b36b00d8ae5e14846527220c9db32b..aaaa0e347702c4412ff515a9d0589c79c69c62d1
> 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -548,10 +548,13 @@ 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 = fl4_dec;
>                 unsigned long orefdst;
>
> -               fl4_2.daddr = fl4_dec.saddr;
> +               swap(fl4_2.daddr, fl4_2.saddr);
> +               swap(fl4_2.fl4_sport, fl4_2.fl4_dport);
> +
> +               fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
>                 rt2 = ip_route_output_key(net, &fl4_2);
>                 if (IS_ERR(rt2)) {
>                         err = PTR_ERR(rt2);