From: Wen Yang <wenyang@linux.alibaba.com>
To: Eric Dumazet <eric.dumazet@gmail.com>,
davem@davemloft.net, David Ahern <dsahern@kernel.org>,
Jakub Kicinski <kuba@kernel.org>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] net: return early for possible invalid uaddr
Date: Tue, 17 Aug 2021 12:08:39 +0800 [thread overview]
Message-ID: <e926cd4f-9c87-8fa3-5b55-861ac299a184@linux.alibaba.com> (raw)
In-Reply-To: <6c11b9e7-6aac-65c9-4755-99d41fbdcb4e@linux.alibaba.com>
在 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
prev parent reply other threads:[~2021-08-17 4:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-11 15:24 [PATCH 1/2] ipv4: fix up inetsw_array coding style Wen Yang
2021-08-11 15:24 ` [PATCH v2 2/2] net: return early for possible invalid uaddr Wen Yang
2021-08-11 16:11 ` Eric Dumazet
2021-08-12 17:35 ` Wen Yang
2021-08-17 4:08 ` Wen Yang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e926cd4f-9c87-8fa3-5b55-861ac299a184@linux.alibaba.com \
--to=wenyang@linux.alibaba.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=eric.dumazet@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).