lkml.org 
[lkml]   [2021]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v2 2/2] net: return early for possible invalid uaddr
From
Date


在 2021/8/13 上午1:35, Wen Yang 写道:
>
>
> 在 2021/8/12 上午12:11, Eric Dumazet 写道:
>>
>>
>> On 8/11/21 5:24 PM, 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. This could cause performance issues or even exhaust ephemeral
>>> ports if a malicious user makes a large number of UDP connections with
>>> invalid uaddr and/or addr_len.
>>>
>>
>> This is a big patch.
>>
>> Can the malicious user still use a large number of UDP sockets,
>> with valid uaddr/add_len and consequently exhaust ephemeral ports ?
>>
>> If yes, it does not seem your patch is helping.
>>
>
> Thank you for your comments.
> However, we could make these optimizations:
>
> 1, If the user passed in some invalid parameters, we should return as
> soon as possible. We shouldn't assume that these parameters are valid
> first, then do some real work (such as select an ephemeral port), and
> then finally check that they are indeed valid or not.
>
> 2. Unify the code for checking parameters in udp_pre_connect() and
> __ip4_datagram_connect() to make the code clearer.
>
>> If no, have you tried instead to undo the autobind, if the connect
>> fails ?
>>
>
> Thanks. Undo the autobind is useful if the connect fails.
> We will add this logic and submit the v3 patch later.
>

Hello, there is no undo autobind for udp. If this logic is added, the
patch will be bigger; maybe we can release this ephemeral port through
unhash()?

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8a8dba7..43947d8 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -563,6 +563,7 @@ int inet_dgram_connect(struct socket *sock, struct
sockaddr *uaddr,
int addr_len, int flags)
{
struct sock *sk = sock->sk;
+ bool autobind;
int err;

if (addr_len < sizeof(uaddr->sa_family))
@@ -581,9 +582,17 @@ int inet_dgram_connect(struct socket *sock, struct
sockaddr *uaddr,
return err;
}

- if (data_race(!inet_sk(sk)->inet_num) && inet_autobind(sk))
+ autobind = data_race(!inet_sk(sk)->inet_num);
+ if (autobind && inet_autobind(sk))
return -EAGAIN;
- return sk->sk_prot->connect(sk, uaddr, addr_len);
+
+ err = sk->sk_prot->connect(sk, uaddr, addr_len);
+ if (err && autobind) {
+ if (sk->sk_prot->unhash)
+ sk->sk_prot->unhash(sk);
+ }
+
+ return err;
}
EXPORT_SYMBOL(inet_dgram_connect);

Could you kindly give some suggestions?
In addition, the previous v2 patch detects errors before bind and
returns earlier, which should be reasonable.


--
Best wishes,
Wen




\
 
 \ /
  Last update: 2021-08-17 06:09    [W:1.572 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site