All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
@ 2026-01-28  9:05 Jiayuan Chen
  2026-02-03 10:41 ` Paolo Abeni
  0 siblings, 1 reply; 6+ messages in thread
From: Jiayuan Chen @ 2026-01-28  9:05 UTC (permalink / raw)
  To: netdev
  Cc: Jiayuan Chen, syzbot+e738404dcd14b620923c, Jiayuan Chen,
	David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Herbert Xu, linux-kernel

From: Jiayuan Chen <jiayuan.chen@shopee.com>

icmp_route_lookup() performs multiple route lookups to find a suitable
route for sending ICMP error messages, with special handling for XFRM
(IPsec) policies.

The lookup sequence is:
1. First, lookup output route for ICMP reply (dst = original src)
2. Pass through xfrm_lookup() for policy check
3. If blocked (-EPERM) or dst is not local, enter "reverse path"
4. In reverse path, call xfrm_decode_session_reverse() to get fl4_dec
   which reverses the original packet's flow (saddr<->daddr swapped)
5. If fl4_dec.saddr is local (we are the original destination), use
   __ip_route_output_key() for output route lookup
6. If fl4_dec.saddr is NOT local (we are a forwarding node), use
   ip_route_input() to simulate the reverse packet's input path
7. Finally, pass rt2 through xfrm_lookup() with XFRM_LOOKUP_ICMP flag

The bug occurs in step 6: ip_route_input() is called with fl4_dec.daddr
(original packet's source) as destination. If this address becomes local
between the initial check and ip_route_input() call (e.g., due to
concurrent "ip addr add"), ip_route_input() returns a LOCAL route with
dst.output set to ip_rt_bug.

This route is then used for ICMP output, causing dst_output() to call
ip_rt_bug(), triggering a WARN_ON:

 ------------[ cut here ]------------
 WARNING: net/ipv4/route.c:1275 at ip_rt_bug+0x21/0x30, CPU#1
 Call Trace:
  <TASK>
  ip_push_pending_frames+0x202/0x240
  icmp_push_reply+0x30d/0x430
  __icmp_send+0x1149/0x24f0
  ip_options_compile+0xa2/0xd0
  ip_rcv_finish_core+0x829/0x1950
  ip_rcv+0x2d7/0x420
  __netif_receive_skb_one_core+0x185/0x1f0
  netif_receive_skb+0x90/0x450
  tun_get_user+0x3413/0x3fb0
  tun_chr_write_iter+0xe4/0x220
  ...

Fix this by checking rt2->rt_type after ip_route_input(). If it's
RTN_LOCAL, the route cannot be used for output, so treat it as an error.

The reproducer requires kernel modification to widen the race window,
making it unsuitable as a selftest. It is available at:

  https://gist.github.com/mrpre/eae853b72ac6a750f5d45d64ddac1e81

Reported-by: syzbot+e738404dcd14b620923c@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/000000000000b1060905eada8881@google.com/T/
Fixes: 8b7817f3a959 ("[IPSEC]: Add ICMP host relookup support")
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/ipv4/icmp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 19c9c838967f..dc9dcc799824 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -559,6 +559,23 @@ static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
 		/* steal dst entry from skb_in, don't drop refcnt */
 		skb_dstref_steal(skb_in);
 		skb_dstref_restore(skb_in, orefdst);
+
+		/*
+		 * At this point, fl4_dec.daddr should NOT be local (we
+		 * checked fl4_dec.saddr above). However, a race condition
+		 * may occur if the address is added to the interface
+		 * concurrently. In that case, ip_route_input() returns a
+		 * LOCAL route with dst.output=ip_rt_bug, which must not
+		 * be used for output.
+		 */
+		if (!err && rt2 && rt2->rt_type == RTN_LOCAL) {
+			net_warn_ratelimited("%s: detected local route for %pI4 "
+					     "during ICMP error handling (src %pI4), "
+					     "possible address race\n",
+					     __func__, &fl4_dec.daddr, &fl4_dec.saddr);
+			dst_release(&rt2->dst);
+			err = -EINVAL;
+		}
 	}
 
 	if (err)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
@ 2026-01-28 23:56 kernel test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-01-28 23:56 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20260128090523.356953-1-jiayuan.chen@linux.dev>
References: <20260128090523.356953-1-jiayuan.chen@linux.dev>
TO: Jiayuan Chen <jiayuan.chen@linux.dev>
TO: netdev@vger.kernel.org
CC: Jiayuan Chen <jiayuan.chen@shopee.com>
CC: syzbot+e738404dcd14b620923c@syzkaller.appspotmail.com
CC: David Ahern <dsahern@kernel.org>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Simon Horman <horms@kernel.org>
CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: linux-kernel@vger.kernel.org

Hi Jiayuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiayuan-Chen/icmp-fix-ip_rt_bug-race-in-icmp_route_lookup-reverse-path/20260128-170826
base:   net-next/main
patch link:    https://lore.kernel.org/r/20260128090523.356953-1-jiayuan.chen%40linux.dev
patch subject: [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
:::::: branch date: 15 hours ago
:::::: commit date: 15 hours ago
config: i386-randconfig-141-20260129 (https://download.01.org/0day-ci/archive/20260129/202601290724.4TPPHx86-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
smatch version: v0.5.0-8994-gd50c5a4c

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202601290724.4TPPHx86-lkp@intel.com/

smatch warnings:
net/ipv4/icmp.c:604 icmp_route_lookup() warn: passing zero to 'ERR_PTR'

vim +/ERR_PTR +604 net/ipv4/icmp.c

e1e84eb58eb494 Mathieu Desnoyers   2020-10-12  485  
913c83a610bb7d Guillaume Nault     2024-10-01  486  static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
77968b78242ee2 David S. Miller     2011-05-08  487  					struct sk_buff *skb_in,
913c83a610bb7d Guillaume Nault     2024-10-01  488  					const struct iphdr *iph, __be32 saddr,
913c83a610bb7d Guillaume Nault     2024-10-01  489  					dscp_t dscp, u32 mark, int type,
913c83a610bb7d Guillaume Nault     2024-10-01  490  					int code, struct icmp_bxm *param)
f6d460cf0ed16d David S. Miller     2011-03-01  491  {
e1e84eb58eb494 Mathieu Desnoyers   2020-10-12  492  	struct net_device *route_lookup_dev;
05d6d492097c55 Eric Dumazet        2024-04-29  493  	struct dst_entry *dst, *dst2;
f6d460cf0ed16d David S. Miller     2011-03-01  494  	struct rtable *rt, *rt2;
415b3334a21aa6 David S. Miller     2011-07-22  495  	struct flowi4 fl4_dec;
f6d460cf0ed16d David S. Miller     2011-03-01  496  	int err;
f6d460cf0ed16d David S. Miller     2011-03-01  497  
77968b78242ee2 David S. Miller     2011-05-08  498  	memset(fl4, 0, sizeof(*fl4));
c86af46b9c7af2 Gustavo A. R. Silva 2026-01-05  499  	fl4->daddr = (param->replyopts.opt.srr ?
c86af46b9c7af2 Gustavo A. R. Silva 2026-01-05  500  		      param->replyopts.opt.faddr : iph->saddr);
77968b78242ee2 David S. Miller     2011-05-08  501  	fl4->saddr = saddr;
e110861f86094c Lorenzo Colitti     2014-05-13  502  	fl4->flowi4_mark = mark;
e2d118a1cb5e60 Lorenzo Colitti     2016-11-04  503  	fl4->flowi4_uid = sock_net_uid(net, NULL);
1bec9d0c0046fe Guillaume Nault     2025-08-25  504  	fl4->flowi4_dscp = dscp;
77968b78242ee2 David S. Miller     2011-05-08  505  	fl4->flowi4_proto = IPPROTO_ICMP;
77968b78242ee2 David S. Miller     2011-05-08  506  	fl4->fl4_icmp_type = type;
77968b78242ee2 David S. Miller     2011-05-08  507  	fl4->fl4_icmp_code = code;
e1e84eb58eb494 Mathieu Desnoyers   2020-10-12  508  	route_lookup_dev = icmp_get_route_lookup_dev(skb_in);
e1e84eb58eb494 Mathieu Desnoyers   2020-10-12  509  	fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
613d09b30f8b58 David Ahern         2015-08-13  510  
3df98d79215ace Paul Moore          2020-09-27  511  	security_skb_classify_flow(skb_in, flowi4_to_flowi_common(fl4));
3abd1ade6765e8 David Ahern         2017-05-25  512  	rt = ip_route_output_key_hash(net, fl4, skb_in);
b23dd4fe42b455 David S. Miller     2011-03-02  513  	if (IS_ERR(rt))
b23dd4fe42b455 David S. Miller     2011-03-02  514  		return rt;
f6d460cf0ed16d David S. Miller     2011-03-01  515  
f6d460cf0ed16d David S. Miller     2011-03-01  516  	/* No need to clone since we're just using its address. */
f6d460cf0ed16d David S. Miller     2011-03-01  517  	rt2 = rt;
f6d460cf0ed16d David S. Miller     2011-03-01  518  
05d6d492097c55 Eric Dumazet        2024-04-29  519  	dst = xfrm_lookup(net, &rt->dst,
77968b78242ee2 David S. Miller     2011-05-08  520  			  flowi4_to_flowi(fl4), NULL, 0);
05d6d492097c55 Eric Dumazet        2024-04-29  521  	rt = dst_rtable(dst);
05d6d492097c55 Eric Dumazet        2024-04-29  522  	if (!IS_ERR(dst)) {
f6d460cf0ed16d David S. Miller     2011-03-01  523  		if (rt != rt2)
f6d460cf0ed16d David S. Miller     2011-03-01  524  			return rt;
c44daa7e3c7322 Dong Chenchen       2024-11-27  525  		if (inet_addr_type_dev_table(net, route_lookup_dev,
c44daa7e3c7322 Dong Chenchen       2024-11-27  526  					     fl4->daddr) == RTN_LOCAL)
c44daa7e3c7322 Dong Chenchen       2024-11-27  527  			return rt;
05d6d492097c55 Eric Dumazet        2024-04-29  528  	} else if (PTR_ERR(dst) == -EPERM) {
f6d460cf0ed16d David S. Miller     2011-03-01  529  		rt = NULL;
05d6d492097c55 Eric Dumazet        2024-04-29  530  	} else {
452edd598f6052 David S. Miller     2011-03-02  531  		return rt;
05d6d492097c55 Eric Dumazet        2024-04-29  532  	}
2b1dc6285c3f6e Florian Westphal    2023-10-04  533  	err = xfrm_decode_session_reverse(net, skb_in, flowi4_to_flowi(&fl4_dec), AF_INET);
f6d460cf0ed16d David S. Miller     2011-03-01  534  	if (err)
f6d460cf0ed16d David S. Miller     2011-03-01  535  		goto relookup_failed;
f6d460cf0ed16d David S. Miller     2011-03-01  536  
e1e84eb58eb494 Mathieu Desnoyers   2020-10-12  537  	if (inet_addr_type_dev_table(net, route_lookup_dev,
30bbaa19500559 David Ahern         2015-08-13  538  				     fl4_dec.saddr) == RTN_LOCAL) {
415b3334a21aa6 David S. Miller     2011-07-22  539  		rt2 = __ip_route_output_key(net, &fl4_dec);
b23dd4fe42b455 David S. Miller     2011-03-02  540  		if (IS_ERR(rt2))
b23dd4fe42b455 David S. Miller     2011-03-02  541  			err = PTR_ERR(rt2);
f6d460cf0ed16d David S. Miller     2011-03-01  542  	} else {
9d6ec938019c6b David S. Miller     2011-03-12  543  		struct flowi4 fl4_2 = {};
f6d460cf0ed16d David S. Miller     2011-03-01  544  		unsigned long orefdst;
f6d460cf0ed16d David S. Miller     2011-03-01  545  
415b3334a21aa6 David S. Miller     2011-07-22  546  		fl4_2.daddr = fl4_dec.saddr;
9d6ec938019c6b David S. Miller     2011-03-12  547  		rt2 = ip_route_output_key(net, &fl4_2);
b23dd4fe42b455 David S. Miller     2011-03-02  548  		if (IS_ERR(rt2)) {
b23dd4fe42b455 David S. Miller     2011-03-02  549  			err = PTR_ERR(rt2);
f6d460cf0ed16d David S. Miller     2011-03-01  550  			goto relookup_failed;
b23dd4fe42b455 David S. Miller     2011-03-02  551  		}
f6d460cf0ed16d David S. Miller     2011-03-01  552  		/* Ugh! */
e97e6a1830ddb5 Stanislav Fomichev  2025-08-18  553  		orefdst = skb_dstref_steal(skb_in);
415b3334a21aa6 David S. Miller     2011-07-22  554  		err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
50038bf38e6577 Menglong Dong       2024-11-07  555  				     dscp, rt2->dst.dev) ? -EINVAL : 0;
f6d460cf0ed16d David S. Miller     2011-03-01  556  
f6d460cf0ed16d David S. Miller     2011-03-01  557  		dst_release(&rt2->dst);
f6d460cf0ed16d David S. Miller     2011-03-01  558  		rt2 = skb_rtable(skb_in);
e97e6a1830ddb5 Stanislav Fomichev  2025-08-18  559  		/* steal dst entry from skb_in, don't drop refcnt */
e97e6a1830ddb5 Stanislav Fomichev  2025-08-18  560  		skb_dstref_steal(skb_in);
e97e6a1830ddb5 Stanislav Fomichev  2025-08-18  561  		skb_dstref_restore(skb_in, orefdst);
fb56f93cae5c6b Jiayuan Chen        2026-01-28  562  
fb56f93cae5c6b Jiayuan Chen        2026-01-28  563  		/*
fb56f93cae5c6b Jiayuan Chen        2026-01-28  564  		 * At this point, fl4_dec.daddr should NOT be local (we
fb56f93cae5c6b Jiayuan Chen        2026-01-28  565  		 * checked fl4_dec.saddr above). However, a race condition
fb56f93cae5c6b Jiayuan Chen        2026-01-28  566  		 * may occur if the address is added to the interface
fb56f93cae5c6b Jiayuan Chen        2026-01-28  567  		 * concurrently. In that case, ip_route_input() returns a
fb56f93cae5c6b Jiayuan Chen        2026-01-28  568  		 * LOCAL route with dst.output=ip_rt_bug, which must not
fb56f93cae5c6b Jiayuan Chen        2026-01-28  569  		 * be used for output.
fb56f93cae5c6b Jiayuan Chen        2026-01-28  570  		 */
fb56f93cae5c6b Jiayuan Chen        2026-01-28  571  		if (!err && rt2 && rt2->rt_type == RTN_LOCAL) {
fb56f93cae5c6b Jiayuan Chen        2026-01-28  572  			net_warn_ratelimited("%s: detected local route for %pI4 "
fb56f93cae5c6b Jiayuan Chen        2026-01-28  573  					     "during ICMP error handling (src %pI4), "
fb56f93cae5c6b Jiayuan Chen        2026-01-28  574  					     "possible address race\n",
fb56f93cae5c6b Jiayuan Chen        2026-01-28  575  					     __func__, &fl4_dec.daddr, &fl4_dec.saddr);
fb56f93cae5c6b Jiayuan Chen        2026-01-28  576  			dst_release(&rt2->dst);
fb56f93cae5c6b Jiayuan Chen        2026-01-28  577  			err = -EINVAL;
fb56f93cae5c6b Jiayuan Chen        2026-01-28  578  		}
f6d460cf0ed16d David S. Miller     2011-03-01  579  	}
f6d460cf0ed16d David S. Miller     2011-03-01  580  
f6d460cf0ed16d David S. Miller     2011-03-01  581  	if (err)
f6d460cf0ed16d David S. Miller     2011-03-01  582  		goto relookup_failed;
f6d460cf0ed16d David S. Miller     2011-03-01  583  
05d6d492097c55 Eric Dumazet        2024-04-29  584  	dst2 = xfrm_lookup(net, &rt2->dst, flowi4_to_flowi(&fl4_dec), NULL,
9d6ec938019c6b David S. Miller     2011-03-12  585  			   XFRM_LOOKUP_ICMP);
05d6d492097c55 Eric Dumazet        2024-04-29  586  	rt2 = dst_rtable(dst2);
05d6d492097c55 Eric Dumazet        2024-04-29  587  	if (!IS_ERR(dst2)) {
f6d460cf0ed16d David S. Miller     2011-03-01  588  		dst_release(&rt->dst);
415b3334a21aa6 David S. Miller     2011-07-22  589  		memcpy(fl4, &fl4_dec, sizeof(*fl4));
f6d460cf0ed16d David S. Miller     2011-03-01  590  		rt = rt2;
05d6d492097c55 Eric Dumazet        2024-04-29  591  	} else if (PTR_ERR(dst2) == -EPERM) {
452edd598f6052 David S. Miller     2011-03-02  592  		if (rt)
452edd598f6052 David S. Miller     2011-03-02  593  			dst_release(&rt->dst);
452edd598f6052 David S. Miller     2011-03-02  594  		return rt2;
452edd598f6052 David S. Miller     2011-03-02  595  	} else {
05d6d492097c55 Eric Dumazet        2024-04-29  596  		err = PTR_ERR(dst2);
452edd598f6052 David S. Miller     2011-03-02  597  		goto relookup_failed;
f6d460cf0ed16d David S. Miller     2011-03-01  598  	}
f6d460cf0ed16d David S. Miller     2011-03-01  599  	return rt;
f6d460cf0ed16d David S. Miller     2011-03-01  600  
f6d460cf0ed16d David S. Miller     2011-03-01  601  relookup_failed:
f6d460cf0ed16d David S. Miller     2011-03-01  602  	if (rt)
f6d460cf0ed16d David S. Miller     2011-03-01  603  		return rt;
f6d460cf0ed16d David S. Miller     2011-03-01 @604  	return ERR_PTR(err);
f6d460cf0ed16d David S. Miller     2011-03-01  605  }
^1da177e4c3f41 Linus Torvalds      2005-04-16  606  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
  2026-01-28  9:05 [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path Jiayuan Chen
@ 2026-02-03 10:41 ` Paolo Abeni
  2026-02-03 16:24   ` David Ahern
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-02-03 10:41 UTC (permalink / raw)
  To: Jiayuan Chen, netdev
  Cc: Jiayuan Chen, syzbot+e738404dcd14b620923c, David S. Miller,
	David Ahern, Eric Dumazet, Jakub Kicinski, Simon Horman,
	Herbert Xu, linux-kernel

On 1/28/26 10:05 AM, Jiayuan Chen wrote:
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 19c9c838967f..dc9dcc799824 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -559,6 +559,23 @@ static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
>  		/* steal dst entry from skb_in, don't drop refcnt */
>  		skb_dstref_steal(skb_in);
>  		skb_dstref_restore(skb_in, orefdst);
> +
> +		/*
> +		 * At this point, fl4_dec.daddr should NOT be local (we
> +		 * checked fl4_dec.saddr above). However, a race condition
> +		 * may occur if the address is added to the interface
> +		 * concurrently. In that case, ip_route_input() returns a
> +		 * LOCAL route with dst.output=ip_rt_bug, which must not
> +		 * be used for output.
> +		 */
> +		if (!err && rt2 && rt2->rt_type == RTN_LOCAL) {
> +			net_warn_ratelimited("%s: detected local route for %pI4 "
> +					     "during ICMP error handling (src %pI4), "
> +					     "possible address race\n",
> +					     __func__, &fl4_dec.daddr, &fl4_dec.saddr);

The fix looks correct to me, but this patch should target the 'net' tree
and the above warning message is a bit off: the text string should not
be broken to fit the 80 chars limit - it need to be greepable - it's
probably better to not include the function name.

/P


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
  2026-02-03 10:41 ` Paolo Abeni
@ 2026-02-03 16:24   ` David Ahern
  2026-02-04  2:27     ` Jiayuan Chen
  0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2026-02-03 16:24 UTC (permalink / raw)
  To: Paolo Abeni, Jiayuan Chen, netdev
  Cc: Jiayuan Chen, syzbot+e738404dcd14b620923c, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman, Herbert Xu,
	linux-kernel

On 2/3/26 3:41 AM, Paolo Abeni wrote:
> On 1/28/26 10:05 AM, Jiayuan Chen wrote:
>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> index 19c9c838967f..dc9dcc799824 100644
>> --- a/net/ipv4/icmp.c
>> +++ b/net/ipv4/icmp.c
>> @@ -559,6 +559,23 @@ static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
>>  		/* steal dst entry from skb_in, don't drop refcnt */
>>  		skb_dstref_steal(skb_in);
>>  		skb_dstref_restore(skb_in, orefdst);
>> +
>> +		/*
>> +		 * At this point, fl4_dec.daddr should NOT be local (we
>> +		 * checked fl4_dec.saddr above). However, a race condition
>> +		 * may occur if the address is added to the interface
>> +		 * concurrently. In that case, ip_route_input() returns a
>> +		 * LOCAL route with dst.output=ip_rt_bug, which must not
>> +		 * be used for output.
>> +		 */
>> +		if (!err && rt2 && rt2->rt_type == RTN_LOCAL) {
>> +			net_warn_ratelimited("%s: detected local route for %pI4 "
>> +					     "during ICMP error handling (src %pI4), "
>> +					     "possible address race\n",
>> +					     __func__, &fl4_dec.daddr, &fl4_dec.saddr);
> 
> The fix looks correct to me, but this patch should target the 'net' tree
> and the above warning message is a bit off: the text string should not
> be broken to fit the 80 chars limit - it need to be greepable - it's
> probably better to not include the function name.
> 
> /P
> 

Does the message even provide value? There is nothing a user can do
about it.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
  2026-02-03 16:24   ` David Ahern
@ 2026-02-04  2:27     ` Jiayuan Chen
  2026-02-04 16:29       ` David Ahern
  0 siblings, 1 reply; 6+ messages in thread
From: Jiayuan Chen @ 2026-02-04  2:27 UTC (permalink / raw)
  To: David Ahern, Paolo Abeni, netdev
  Cc: Jiayuan Chen, syzbot+e738404dcd14b620923c, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman, Herbert Xu,
	linux-kernel

February 4, 2026 at 24:24, "David Ahern" <dsahern@kernel.org mailto:dsahern@kernel.org?to=%22David%20Ahern%22%20%3Cdsahern%40kernel.org%3E > wrote:


> 
> On 2/3/26 3:41 AM, Paolo Abeni wrote:
> 
> > 
> > On 1/28/26 10:05 AM, Jiayuan Chen wrote:
> > 
> > > 
> > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > >  index 19c9c838967f..dc9dcc799824 100644
> > >  --- a/net/ipv4/icmp.c
> > >  +++ b/net/ipv4/icmp.c
> > >  @@ -559,6 +559,23 @@ static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
> > >  /* steal dst entry from skb_in, don't drop refcnt */
> > >  skb_dstref_steal(skb_in);
> > >  skb_dstref_restore(skb_in, orefdst);
> > >  +
> > >  + /*
> > >  + * At this point, fl4_dec.daddr should NOT be local (we
> > >  + * checked fl4_dec.saddr above). However, a race condition
> > >  + * may occur if the address is added to the interface
> > >  + * concurrently. In that case, ip_route_input() returns a
> > >  + * LOCAL route with dst.output=ip_rt_bug, which must not
> > >  + * be used for output.
> > >  + */
> > >  + if (!err && rt2 && rt2->rt_type == RTN_LOCAL) {
> > >  + net_warn_ratelimited("%s: detected local route for %pI4 "
> > >  + "during ICMP error handling (src %pI4), "
> > >  + "possible address race\n",
> > >  + __func__, &fl4_dec.daddr, &fl4_dec.saddr);
> > > 
> >  
> >  The fix looks correct to me, but this patch should target the 'net' tree
> >  and the above warning message is a bit off: the text string should not
> >  be broken to fit the 80 chars limit - it need to be greepable - it's
> >  probably better to not include the function name.
> >  
> >  /P
> > 
> Does the message even provide value? There is nothing a user can do
> about it.
>

Hi Paolo, David,

Thanks for the review.

Paolo, I'll fix it in newer version.

Regarding David's question about whether the message provides value:
I think it could still be useful for debugging. If a user's configuration
change causes ICMP packets to be silently dropped, there's currently no counter
or indication of what happened. Adding a dedicated counter for this rare race
condition seems overkill, so a rate-limited warning at least gives some visibility
into the failure.

That said, I'm open to dropping the message entirely if you both think silent handling
is preferred. Let me know and I'll send a new version either way.

Thanks,
Jiayuan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path
  2026-02-04  2:27     ` Jiayuan Chen
@ 2026-02-04 16:29       ` David Ahern
  0 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2026-02-04 16:29 UTC (permalink / raw)
  To: Jiayuan Chen, Paolo Abeni, netdev
  Cc: Jiayuan Chen, syzbot+e738404dcd14b620923c, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Simon Horman, Herbert Xu,
	linux-kernel

On 2/3/26 7:27 PM, Jiayuan Chen wrote:
> 
> Thanks for the review.
> 
> Paolo, I'll fix it in newer version.
> 
> Regarding David's question about whether the message provides value:
> I think it could still be useful for debugging. If a user's configuration
> change causes ICMP packets to be silently dropped, there's currently no counter
> or indication of what happened. Adding a dedicated counter for this rare race
> condition seems overkill, so a rate-limited warning at least gives some visibility
> into the failure.
> 
> That said, I'm open to dropping the message entirely if you both think silent handling
> is preferred. Let me know and I'll send a new version either way.
> 

no strong opinions.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-04 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28  9:05 [PATCH net-next v1] icmp: fix ip_rt_bug race in icmp_route_lookup reverse path Jiayuan Chen
2026-02-03 10:41 ` Paolo Abeni
2026-02-03 16:24   ` David Ahern
2026-02-04  2:27     ` Jiayuan Chen
2026-02-04 16:29       ` David Ahern
  -- strict thread matches above, loose matches on Subject: below --
2026-01-28 23:56 kernel test robot

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.