public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [bug report] xfrm: fix ip_rt_bug race in icmp_route_lookup reverse path
@ 2026-04-10 10:16 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2026-04-10 10:16 UTC (permalink / raw)
  To: Jiayuan Chen; +Cc: Simon Horman, netdev

Hello Jiayuan Chen,

Commit 81b84de32bb2 ("xfrm: fix ip_rt_bug race in icmp_route_lookup
reverse path") from Feb 6, 2026 (linux-next), leads to the following
Smatch static checker warning:

	net/ipv4/icmp.c:587 icmp_route_lookup()
	error: we previously assumed 'rt2' could be null (see line 576)

net/ipv4/icmp.c
    491 static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
    492                                         struct sk_buff *skb_in,
    493                                         const struct iphdr *iph, __be32 saddr,
    494                                         dscp_t dscp, u32 mark, int type,
    495                                         int code, struct icmp_bxm *param)
    496 {
    497         struct net_device *route_lookup_dev;
    498         struct dst_entry *dst, *dst2;
    499         struct rtable *rt, *rt2;
    500         struct flowi4 fl4_dec;
    501         int err;
    502 
    503         memset(fl4, 0, sizeof(*fl4));
    504         fl4->daddr = (param->replyopts.opt.srr ?
    505                       param->replyopts.opt.faddr : iph->saddr);
    506         fl4->saddr = saddr;
    507         fl4->flowi4_mark = mark;
    508         fl4->flowi4_uid = sock_net_uid(net, NULL);
    509         fl4->flowi4_dscp = dscp;
    510         fl4->flowi4_proto = IPPROTO_ICMP;
    511         fl4->fl4_icmp_type = type;
    512         fl4->fl4_icmp_code = code;
    513         route_lookup_dev = icmp_get_route_lookup_dev(skb_in);
    514         fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
    515 
    516         security_skb_classify_flow(skb_in, flowi4_to_flowi_common(fl4));
    517         rt = ip_route_output_key_hash(net, fl4, skb_in);
    518         if (IS_ERR(rt))
    519                 return rt;
    520 
    521         /* No need to clone since we're just using its address. */
    522         rt2 = rt;
    523 
    524         dst = xfrm_lookup(net, &rt->dst,
    525                           flowi4_to_flowi(fl4), NULL, 0);
    526         rt = dst_rtable(dst);
    527         if (!IS_ERR(dst)) {
    528                 if (rt != rt2)
    529                         return rt;
    530                 if (inet_addr_type_dev_table(net, route_lookup_dev,
    531                                              fl4->daddr) == RTN_LOCAL)
    532                         return rt;
    533         } else if (PTR_ERR(dst) == -EPERM) {
    534                 rt = NULL;
    535         } else {
    536                 return rt;
    537         }
    538         err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
    539         if (err)
    540                 goto relookup_failed;
    541 
    542         if (inet_addr_type_dev_table(net, route_lookup_dev,
    543                                      fl4_dec.saddr) == RTN_LOCAL) {
    544                 rt2 = __ip_route_output_key(net, &fl4_dec);
    545                 if (IS_ERR(rt2))
    546                         err = PTR_ERR(rt2);
    547         } else {
    548                 struct flowi4 fl4_2 = {};
    549                 unsigned long orefdst;
    550 
    551                 fl4_2.daddr = fl4_dec.saddr;
    552                 rt2 = ip_route_output_key(net, &fl4_2);
    553                 if (IS_ERR(rt2)) {
    554                         err = PTR_ERR(rt2);
    555                         goto relookup_failed;
    556                 }
    557                 /* Ugh! */
    558                 orefdst = skb_dstref_steal(skb_in);
    559                 err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
    560                                      dscp, rt2->dst.dev) ? -EINVAL : 0;
    561 
    562                 dst_release(&rt2->dst);
    563                 rt2 = skb_rtable(skb_in);
    564                 /* steal dst entry from skb_in, don't drop refcnt */
    565                 skb_dstref_steal(skb_in);
    566                 skb_dstref_restore(skb_in, orefdst);
    567 
    568                 /*
    569                  * At this point, fl4_dec.daddr should NOT be local (we
    570                  * checked fl4_dec.saddr above). However, a race condition
    571                  * may occur if the address is added to the interface
    572                  * concurrently. In that case, ip_route_input() returns a
    573                  * LOCAL route with dst.output=ip_rt_bug, which must not
    574                  * be used for output.
    575                  */
    576                 if (!err && rt2 && rt2->rt_type == RTN_LOCAL) {
                                    ^^^
Can rt2 really be NULL here?

    577                         net_warn_ratelimited("detected local route for %pI4 during ICMP sending, src %pI4\n",
    578                                              &fl4_dec.daddr, &fl4_dec.saddr);
    579                         dst_release(&rt2->dst);
    580                         err = -EINVAL;
    581                 }
    582         }
    583 
    584         if (err)
    585                 goto relookup_failed;
    586 
--> 587         dst2 = xfrm_lookup(net, &rt2->dst, flowi4_to_flowi(&fl4_dec), NULL,
                                        ^^^^^^^^^
Because, if so, then we are screwed here.


    588                            XFRM_LOOKUP_ICMP);
    589         rt2 = dst_rtable(dst2);
    590         if (!IS_ERR(dst2)) {
    591                 dst_release(&rt->dst);
    592                 rt = rt2;
    593         } else if (PTR_ERR(dst2) == -EPERM) {
    594                 if (rt)
    595                         dst_release(&rt->dst);
    596                 return rt2;
    597         } else {
    598                 err = PTR_ERR(dst2);
    599                 goto relookup_failed;
    600         }
    601         return rt;
    602 
    603 relookup_failed:
    604         if (rt)
    605                 return rt;
    606         return ERR_PTR(err);
    607 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-04-10 10:16 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10 10:16 [bug report] xfrm: fix ip_rt_bug race in icmp_route_lookup reverse path Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox