From: Dan Carpenter <error27@gmail.com>
To: Jiayuan Chen <jiayuan.chen@shopee.com>
Cc: Simon Horman <horms@kernel.org>, netdev@vger.kernel.org
Subject: [bug report] xfrm: fix ip_rt_bug race in icmp_route_lookup reverse path
Date: Fri, 10 Apr 2026 13:16:49 +0300 [thread overview]
Message-ID: <adjOEUXsHTZoB43e@stanley.mountain> (raw)
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
reply other threads:[~2026-04-10 10:16 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=adjOEUXsHTZoB43e@stanley.mountain \
--to=error27@gmail.com \
--cc=horms@kernel.org \
--cc=jiayuan.chen@shopee.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.