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

[Wen Yang <wenyang@linux.alibaba.com>]

在 2021/8/10 上午6:32, Jakub Kicinski 写道:
> On Sun,  8 Aug 2021 01:19:38 +0800 Wen Yang wrote:
>> 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.

>> 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.

>> 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


>> +	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)