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@xxxxxxxxxx> wrote:
>
> On Thu, Jul 16, 2026 at 1:22 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> >
> > On Thu, Jul 16, 2026 at 12:40 PM Muhammad Ziad <muhzi100@xxxxxxxxx> 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);