netdev - Re: [PATCH net] net: Fix icmp host relookup triggering ip_rt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9b9c7edf-fa68-465e-a960-0fe07773ba82@huawei.com>
Date: Wed, 13 Nov 2024 22:13:29 +0800
From: "dongchenchen (A)" <dongchenchen2@...wei.com>
To: Herbert Xu <herbert@...dor.apana.org.au>, Steffen Klassert
	<steffen.klassert@...unet.com>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
	<pabeni@...hat.com>, <horms@...nel.org>, <netdev@...r.kernel.org>,
	<yuehaibing@...wei.com>, <zhangchangzhong@...wei.com>
Subject: Re: [PATCH net] net: Fix icmp host relookup triggering ip_rt_bug

>> If skb_in is outbound, fl4_dec.saddr is not nolocal. It may be no input
>> route from B to A for
>>
>> first communication.
> You're right.  So the problem here is that for the case of A
> being local, we should not be taking the ip_route_input code
> path (this is intended for forwarded packets).
>
> In fact if A is local, and we're sending an ICMP message to A,
> then perhaps we could skip the IPsec lookup completely and just
> do normal routing?
>
> Steffen, what do you think?
>
> So I think it boils down to two choices:
>
> 1) We could simply drop IPsec processing if we notice that A
> (fl.fl4_dst) is local;

Hi, Herbert! Thanks for your suggestions very much.

maybe we can fix it as below:
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index e1384e7331d8..5f63c4dde4e9 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -517,7 +517,9 @@ static struct rtable *icmp_route_lookup(struct net *net,
                           flowi4_to_flowi(fl4), NULL, 0);
         rt = dst_rtable(dst);
         if (!IS_ERR(dst)) {
-               if (rt != rt2)
+               unsigned int addr_type = inet_addr_type_dev_table(net, 
route_lookup_dev, fl4->daddr);
+               if (rt != rt2 || addr_type == RTN_LOCAL)
                         return rt;
         } else if (PTR_ERR(dst) == -EPERM) {
                 rt = NULL;
> 2) Or we could take the __ip_route_output_key code path and
> continue to do the xfrm lookup when A is local.
If only skip input route lookup as below, xfrm_lookup check may return 
ENOENT
for no xfrm pol or dst nofrm flag.

@@ -543,6 +544,10 @@ static struct rtable *icmp_route_lookup(struct net 
*net, err = PTR_ERR(rt2); goto relookup_failed; } + + unsigned int 
addr_type = inet_addr_type_dev_table(net, route_lookup_dev, fl4->daddr); 
+ if (addr_type == RTN_LOCAL) + goto relookup; /* Ugh! */ orefdst = 
skb_in->_skb_refdst; /* save old refdst */

xfrm_lookup
xfrm_lookup_with_ifid
       if (!if_id && ((dst_orig->flags & DST_NOXFRM) ||
!net->xfrm.policy_count[XFRM_POLICY_OUT]))
         goto nopol;

Thanks a lot!
Dong Chenchen

> Thanks,