Re: [PATCH] ipv4: return early for possible invalid uaddr

在 2021/8/10 上午6:32, Jakub Kicinski 写道:

On Sun,  8 Aug 2021 01:19:38 +0800 Wen Yang wrote:

quoted

The inet_dgram_connect() first calls inet_autobind() to select an
ephemeral port, then checks uaddr in udp_pre_connect() or
__ip4_datagram_connect(), but the port is not released until the socket
is closed.

We should return early for invalid uaddr to improve performance and
simplify the code a bit,

The performance improvement would be if the benchmark is calling
connect with invalid arguments? That seems like an extremely rare
scenario in real life.

Thanks for your comments.

On the one hand, it is the performance impact, but we also found that it
may cause DoS: simulate a scenario where udp connect is frequently
performed (illegal addrlen, and the socket is not closed), the local
ports will be exhausted quickly.

quoted

and also switch from a mix of tabs and spaces to just tabs.

Please never mix unrelated whitespace cleanup into patches making real
code changes.

OK.

quoted

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 5464818..97b6fc4 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c

@@ -569,6 +569,11 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
  	if (uaddr->sa_family == AF_UNSPEC)
  		return sk->sk_prot->disconnect(sk, flags);
  
+	if (uaddr->sa_family != AF_INET)
+		return -EAFNOSUPPORT;

And what about IPv6 which also calls this function?

Sorry that currently only ipv4 has been modified, we will continue to 
improve, and the v2 patch will be submitted later, thank you.


-- 
Best wishes，
Wen

quoted

+	if (addr_len < sizeof(struct sockaddr_in))
+		return -EINVAL;
+
  	if (BPF_CGROUP_PRE_CONNECT_ENABLED(sk)) {
  		err = sk->sk_prot->pre_connect(sk, uaddr, addr_len);
  		if (err)