[MPTCP] [PATCH net] tcp: fix syn cookied MPTCP request socket leak

[MPTCP] [PATCH net] tcp: fix syn cookied MPTCP request socket leak

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

If a syn-cookies request socket don't pass MPTCP-level
validation done in syn_recv_sock(), we need to release
it immediately, or it will be leaked.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/89
Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
Reported-and-tested-by: Geliang Tang <geliangtang(a)gmail.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
 net/ipv4/syncookies.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index f0794f0232ba..e03756631541 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -214,7 +214,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
 		sock_rps_save_rxhash(child, skb);
 
 		if (rsk_drop_req(req)) {
-			refcount_set(&req->rsk_refcnt, 2);
+			reqsk_put(req);
 			return child;
 		}
 
-- 
2.26.2

[PATCH net] tcp: fix syn cookied MPTCP request socket leak

[Paolo Abeni]

If a syn-cookies request socket don't pass MPTCP-level
validation done in syn_recv_sock(), we need to release
it immediately, or it will be leaked.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/89
Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
Reported-and-tested-by: Geliang Tang <geliangtang@gmail.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/ipv4/syncookies.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index f0794f0232ba..e03756631541 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -214,7 +214,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
 		sock_rps_save_rxhash(child, skb);
 
 		if (rsk_drop_req(req)) {
-			refcount_set(&req->rsk_refcnt, 2);
+			reqsk_put(req);
 			return child;
 		}
 
-- 
2.26.2

[MPTCP] Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak

[David Miller]

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

From: Paolo Abeni <pabeni(a)redhat.com>
Date: Fri,  2 Oct 2020 12:39:44 +0200

> If a syn-cookies request socket don't pass MPTCP-level
> validation done in syn_recv_sock(), we need to release
> it immediately, or it will be leaked.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/89
> Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
> Reported-and-tested-by: Geliang Tang <geliangtang(a)gmail.com>
> Reviewed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>

Applied, thank you.

Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak

[David Miller]

From: Paolo Abeni <pabeni@redhat.com>
Date: Fri,  2 Oct 2020 12:39:44 +0200

> If a syn-cookies request socket don't pass MPTCP-level
> validation done in syn_recv_sock(), we need to release
> it immediately, or it will be leaked.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/89
> Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
> Reported-and-tested-by: Geliang Tang <geliangtang@gmail.com>
> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied, thank you.

[MPTCP] [PATCH net] tcp: fix syn cookied MPTCP request socket leak

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

If a syn-cookies request socket don't pass MPTCP-level
validation done in syn_recv_sock(), we need to release
it immediatelly, or it will be leaked.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/89
Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
 net/ipv4/syncookies.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
---
I don't really know what I'm doing here ;) 
But apparently it fixes the leak in my tests

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index c375c126f436..5c8390876cf8 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -209,15 +209,15 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
 	child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
 						 NULL, &own_req);
 	if (child) {
-		refcount_set(&req->rsk_refcnt, 1);
-		tcp_sk(child)->tsoffset = tsoff;
-		sock_rps_save_rxhash(child, skb);
-
 		if (rsk_drop_req(req)) {
-			refcount_set(&req->rsk_refcnt, 2);
+			reqsk_free(req);
 			return child;
 		}
 
+		refcount_set(&req->rsk_refcnt, 1);
+		tcp_sk(child)->tsoffset = tsoff;
+		sock_rps_save_rxhash(child, skb);
+
 		if (inet_csk_reqsk_queue_add(sk, req, child))
 			return child;
 
-- 
2.26.2