netdev - Re: [BUG] net: ipv4: The sent udp broadcast message may be converted to an arp request message

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: <YgIhGhh75mR5uLaS@shredder>
Date:   Tue, 8 Feb 2022 09:51:54 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     "wanghai (M)" <wanghai38@...wei.com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, edumazet@...gle.com,
        yoshfuji@...ux-ipv6.org, dsahern@...nel.org,
        netdev@...r.kernel.org, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] net: ipv4: The sent udp broadcast message may be converted
 to an arp request message

On Mon, Feb 07, 2022 at 10:09:49PM +0800, wanghai (M) wrote:
> Hello,
> 
> I found a bug, but I don't know how to fix it. Anyone have some good ideas?
> 
> This bug will cause udp broadcast messages not to be sent, but instead send
> non-expected arp request messages.
> 
> Deleting the ip while sending udp broadcast messages and then configuring
> the ip again will probably trigger the bug.
> 
> The following is the timing diagram of the bug, cpu0 sends a broadcast
> message and cpu1 deletes the routing table at the appropriate time.
> 
> cpu0                                        cpu1
> send()
>   udp_sendmsg()
>     ip_route_output_flow()
>     |  fib_lookup()
>     udp_send_skb()
>       ip_send_skb()
>         ip_finish_output2()
> 
>                                             ifconfig eth0:2 down
>                                               fib_del_ifaddr
>                                                 fib_table_delete // delete
> fib table
> 
>           ip_neigh_for_gw()
>           |  ip_neigh_gw4()
>           |    __ipv4_neigh_lookup_noref()
>           |    __neigh_create()
>           |      tbl->constructor(n) --> arp_constructor()
>           |        neigh->type = inet_addr_type_dev_table(); // no route,
> neigh->type = RTN_UNICAST
>           neigh_output() // unicast, send an arp request and create an
> exception arp entry
> 
> After the above operation, an abnormal arp entry will be generated. If
> the ip is configured again(ifconfig eth0:2 12.0.208.0), the abnormal arp
> entry will still exist, and the udp broadcast message will be converted
> to an arp request message when it is sent.

Can you try the below? Not really happy with it, but don't have a better
idea at the moment. It seemed better to handle it from the control path
than augmenting the data path with more checks

diff --git a/include/net/arp.h b/include/net/arp.h
index 031374ac2f22..9e6a1961b64c 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -64,6 +64,7 @@ void arp_send(int type, int ptype, __be32 dest_ip,
 	      const unsigned char *dest_hw,
 	      const unsigned char *src_hw, const unsigned char *th);
 int arp_mc_map(__be32 addr, u8 *haddr, struct net_device *dev, int dir);
+int arp_invalidate(struct net_device *dev, __be32 ip);
 void arp_ifdown(struct net_device *dev);
 
 struct sk_buff *arp_create(int type, int ptype, __be32 dest_ip,
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 4db0325f6e1a..b81665ce2b57 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1116,7 +1116,7 @@ static int arp_req_get(struct arpreq *r, struct net_device *dev)
 	return err;
 }
 
-static int arp_invalidate(struct net_device *dev, __be32 ip)
+int arp_invalidate(struct net_device *dev, __be32 ip)
 {
 	struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
 	int err = -ENXIO;
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 4d61ddd8a0ec..2d7085232cb5 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -1112,9 +1112,11 @@ void fib_add_ifaddr(struct in_ifaddr *ifa)
 		return;
 
 	/* Add broadcast address, if it is explicitly assigned. */
-	if (ifa->ifa_broadcast && ifa->ifa_broadcast != htonl(0xFFFFFFFF))
+	if (ifa->ifa_broadcast && ifa->ifa_broadcast != htonl(0xFFFFFFFF)) {
 		fib_magic(RTM_NEWROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32,
 			  prim, 0);
+		arp_invalidate(dev, ifa->ifa_broadcast);
+	}
 
 	if (!ipv4_is_zeronet(prefix) && !(ifa->ifa_flags & IFA_F_SECONDARY) &&
 	    (prefix != addr || ifa->ifa_prefixlen < 32)) {
@@ -1128,6 +1130,7 @@ void fib_add_ifaddr(struct in_ifaddr *ifa)
 		if (ifa->ifa_prefixlen < 31) {
 			fib_magic(RTM_NEWROUTE, RTN_BROADCAST, prefix | ~mask,
 				  32, prim, 0);
+			arp_invalidate(dev, prefix | ~mask);
 		}
 	}
 }