netdev - [PATCH] tcp: Fix tcp_max_syn_backlog limit on connection requests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Dec 2019 07:38:08 +0000
From:   Ttttabcd <ttttabcd@...tonmail.com>
To:     Netdev <netdev@...r.kernel.org>
Cc:     "edumazet@...gle.com" <edumazet@...gle.com>,
        David Miller <davem@...emloft.net>,
        "kuznet@....inr.ac.ru" <kuznet@....inr.ac.ru>,
        "yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org>
Subject: [PATCH] tcp: Fix tcp_max_syn_backlog limit on connection requests

In the original logic of tcp_conn_request, the backlog parameter of the
listen system call and net.ipv4.tcp_max_syn_backlog are independent of
each other, which causes some confusion in the processing.

The backlog determines the maximum length of request_sock_queue, hereafter
referred to as backlog.

In the original design, if syn_cookies is not turned on, a quarter of
tcp_max_syn_backlog will be reserved for clients that have proven to
exist, mitigating syn attacks.

Suppose now that tcp_max_syn_backlog is 1000, but the backlog is only 200,
then 1000 >> 2 = 250, the backlog is used up by the syn attack, and the
above mechanism will not work.

Is tcp_max_syn_backlog used to limit the
maximum length of request_sock_queue?

Now suppose sycookie is enabled, backlog is 1000, and tcp_max_syn_backlog
is only 200. In this case tcp_max_syn_backlog will be useless.

Because syn_cookies is enabled, the tcp_max_syn_backlog logic will
be ignored, and the length of request_sock_queue will exceed
tcp_max_syn_backlog until the backlog.

I modified the original logic and set the minimum value in backlog and
tcp_max_syn_backlog as the maximum length limit of request_sock_queue.

Now there is only a unified limit.

The maximum length limit variable is "max_syn_backlog".

Use syn_cookies whenever max_syn_backlog is exceeded.

If syn_cookies is not enabled, a quarter of the max_syn_backlog queue is
reserved for hosts that have proven to exist.

In any case, request_sock_queue will not exceed max_syn_backlog.
When syn_cookies is not turned on, a quarter of the queue retention
will not be preempted.

Signed-off-by: AK Deng <ttttabcd@...tonmail.com>
---
 net/ipv4/tcp_input.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 88b987ca9ebb..190edaaa9dce 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6562,14 +6562,18 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 	struct request_sock *req;
 	bool want_cookie = false;
 	struct dst_entry *dst;
+	int max_syn_backlog;
 	struct flowi fl;

+	max_syn_backlog = min(net->ipv4.sysctl_max_syn_backlog,
+			      sk->sk_max_ack_backlog);
+
 	/* TW buckets are converted to open requests without
 	 * limitations, they conserve resources and peer is
 	 * evidently real one.
 	 */
 	if ((net->ipv4.sysctl_tcp_syncookies == 2 ||
-	     inet_csk_reqsk_queue_is_full(sk)) && !isn) {
+	     inet_csk_reqsk_queue_len(sk) >= max_syn_backlog) && !isn) {
 		want_cookie = tcp_syn_flood_action(sk, rsk_ops->slab_name);
 		if (!want_cookie)
 			goto drop;
@@ -6621,8 +6625,8 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 	if (!want_cookie && !isn) {
 		/* Kill the following clause, if you dislike this way. */
 		if (!net->ipv4.sysctl_tcp_syncookies &&
-		    (net->ipv4.sysctl_max_syn_backlog - inet_csk_reqsk_queue_len(sk) <
-		     (net->ipv4.sysctl_max_syn_backlog >> 2)) &&
+		    (max_syn_backlog - inet_csk_reqsk_queue_len(sk) <
+		     (max_syn_backlog >> 2)) &&
 		    !tcp_peer_is_proven(req, dst)) {
 			/* Without syncookies last quarter of
 			 * backlog is filled with destinations,
--
2.24.0