Skip to contentSkip to conversation

net: ipv6: Fix UDP early demux lookup with udp_l3mdev_accept=0

PATCH Superseded Subash Abhinov Kasiviswanathan 3 participants 5 messages Apr 17, 2017 – Apr 18, 2017

Subsystems (3)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 net: ipv6: Fix UDP early demux lookup with udp_l3mdev_accept=0

David Ahern reported that 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast") breaks udp_l3mdev_accept=0 since early demux for IPv6 UDP was doing a generic socket lookup which does not require an exact match. Fix this by making UDPv6 early demux match connected sockets only.

v1->v2: Take reference to socket after match as suggested by Eric

Fixes 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast")
Cc Eric Dumazet <[email protected]>
---
 net/ipv6/udp.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
net/ipv6/udp.c +14 -9
index b793ed1..5a4504b 100644
@@ -46,6 +46,7 @@
 #include <net/tcp_states.h>
 #include <net/ip6_checksum.h>
 #include <net/xfrm.h>
+#include <net/inet_hashtables.h>
 #include <net/inet6_hashtables.h>
 #include <net/busy_poll.h>
 #include <net/sock_reuseport.h>
@@ -864,21 +865,25 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	return 0;
 }
 
+
 static struct sock *__udp6_lib_demux_lookup(struct net *net,
 			__be16 loc_port, const struct in6_addr *loc_addr,
 			__be16 rmt_port, const struct in6_addr *rmt_addr,
 			int dif)
 {
+	unsigned short hnum = ntohs(loc_port);
+	unsigned int hash2 = udp6_portaddr_hash(net, loc_addr, hnum);
+	unsigned int slot2 = hash2 & udp_table.mask;
+	struct udp_hslot *hslot2 = &udp_table.hash2[slot2];
+	const __portpair ports = INET_COMBINED_PORTS(rmt_port, hnum);
 	struct sock *sk;
 
-	rcu_read_lock();
-	sk = __udp6_lib_lookup(net, rmt_addr, rmt_port, loc_addr, loc_port,
-			       dif, &udp_table, NULL);
-	if (sk && !atomic_inc_not_zero(&sk->sk_refcnt))
-		sk = NULL;
-	rcu_read_unlock();
-
-	return sk;
+	udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) {
+		if (INET6_MATCH(sk, net, rmt_addr, loc_addr, ports, dif))
+			return sk;
+		break;
+	}
+	return NULL;
 }
 
 static void udp_v6_early_demux(struct sk_buff *skb)
@@ -903,7 +908,7 @@ static void udp_v6_early_demux(struct sk_buff *skb)
 	else
 		return;
 
-	if (!sk)
+	if (!sk || !atomic_inc_not_zero_hint(&sk->sk_refcnt, 2))
 		return;
 
 	skb->sk = sk;
Signature · 1 line
1.9.1
#1 lore raw
Quoted 12 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Subash Abhinov Kasiviswanathan
Sent: Tuesday, April 18, 2017 7:25 AM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Subash Abhinov Kasiviswanathan; Eric Dumazet
Subject: [PATCH net-next v2] net: ipv6: Fix UDP early demux lookup with udp_l3mdev_accept=0

net/ipv6/udp.c int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, · line 884

Quoted 5 lines
net/ipv6/udp.c +4 -1
-	return sk;
+	udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) {
+		if (INET6_MATCH(sk, net, rmt_addr, loc_addr, ports, dif))
+			return sk;
+		break;

I think break here should remove ?

Quoted 5 lines
net/ipv6/udp.c +2 -1
+	}
+	return NULL;
 }
--

1.9.1

#2 lore raw
Quoted 2 lines

net/ipv6/udp.c int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, · line 884

Quoted 1 line
net/ipv6/udp.c +1 -0
+		break;

I think break here should remove ?

Hi Yuan

This is similar to __udp4_lib_demux_lookup where we need to check if the first socket is an exact match or break since chains maybe long.

#3 lore raw
Quoted 9 lines · Subash Abhinov Kasiviswanathan
Quoted 2 lines
Quoted 1 line

+ break;

I think break here should remove ?

Hi Yuan

This is similar to __udp4_lib_demux_lookup where we need to check if the first
socket is an exact match or break since chains maybe long.

I suggest adding the same comment as __udp4_lib_demux_lookup; it does look odd.

Acked-by Tested-by on v2 1/1
#4 lore raw
In reply to Subash Abhinov Kasiviswanathan
Quoted 8 lines · Subash Abhinov Kasiviswanathan

David Ahern reported that 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast") breaks udp_l3mdev_accept=0 since early demux for IPv6 UDP was doing a generic socket lookup which does not require an exact match. Fix this by making UDPv6 early demux match connected sockets only.

v1->v2: Take reference to socket after match as suggested by Eric

Quoted 7 lines
Fixes 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast")
Cc Eric Dumazet <[email protected]>
---
 net/ipv6/udp.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Besides adding the comment before the break, this looks fine to me.

If the only change for v3 is the comment before the break, please keep the above with the new patch.