* [MPTCP] Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak
@ 2020-09-14 9:34 Geliang Tang
0 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2020-09-14 9:34 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]
Hi Paolo,
Thanks for your patch. I tested it. It fixed the memory leak problem.
But I got this
selftest fail output:
11 single subflow with syn cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
12 multiple subflows with syn cookies syn[fail] got 1 JOIN[s] syn expected 2
- synack[fail] got 1 JOIN[s] synack expected 2
- ack[fail] got 1 JOIN[s] ack expected 2
Server ns stats
MPTcpExtMPCapableSYNRX 1 0.0
MPTcpExtMPCapableACKRX 1 0.0
MPTcpExtMPJoinSynRx 1 0.0
MPTcpExtMPJoinAckRx 1 0.0
Client ns stats
MPTcpExtMPJoinSynAckRx 1 0.0
13 subflows limited by server w cookies syn[fail] got 1 JOIN[s] syn expected 2
- synack[fail] got 1 JOIN[s] synack expected 2
- ack[ ok ]
Server ns stats
MPTcpExtMPCapableSYNRX 1 0.0
MPTcpExtMPCapableACKRX 1 0.0
MPTcpExtMPJoinSynRx 1 0.0
MPTcpExtMPJoinAckRx 1 0.0
Client ns stats
MPTcpExtMPJoinSynAckRx 1 0.0
14 signal address with syn cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
15 subflow and signal w cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
16 subflows and signal w. cookies syn[fail] got 1 JOIN[s] syn expected 3
- synack[fail] got 1 JOIN[s] synack expected 3
- ack[fail] got 1 JOIN[s] ack expected 3
Server ns stats
MPTcpExtMPCapableSYNRX 1 0.0
MPTcpExtMPCapableACKRX 1 0.0
MPTcpExtMPJoinSynRx 1 0.0
MPTcpExtMPJoinAckRx 1 0.0
Client ns stats
MPTcpExtMPJoinSynAckRx 1 0.0
-Geliang
Paolo Abeni <pabeni(a)redhat.com> 于2020年9月11日周五 下午9:38写道:
>
> 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
> _______________________________________________
> mptcp mailing list -- mptcp(a)lists.01.org
> To unsubscribe send an email to mptcp-leave(a)lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak
@ 2020-10-02 22:35 David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-10-02 22:35 UTC (permalink / raw)
To: mptcp
[-- 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak
@ 2020-09-15 3:03 Geliang Tang
0 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2020-09-15 3:03 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 2531 bytes --]
Hi Paolo,
Paolo Abeni <pabeni(a)redhat.com> 于2020年9月14日周一 下午9:54写道:
>
> On Mon, 2020-09-14 at 17:34 +0800, Geliang Tang wrote:
> > Hi Paolo,
> >
> > Thanks for your patch. I tested it. It fixed the memory leak problem.
> > But I got this
> > selftest fail output:
> >
> > 11 single subflow with syn cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
> > 12 multiple subflows with syn cookies syn[fail] got 1 JOIN[s] syn expected 2
> > - synack[fail] got 1 JOIN[s] synack expected 2
> > - ack[fail] got 1 JOIN[s] ack expected 2
> > Server ns stats
> > MPTcpExtMPCapableSYNRX 1 0.0
> > MPTcpExtMPCapableACKRX 1 0.0
> > MPTcpExtMPJoinSynRx 1 0.0
> > MPTcpExtMPJoinAckRx 1 0.0
> > Client ns stats
> > MPTcpExtMPJoinSynAckRx 1 0.0
> > 13 subflows limited by server w cookies syn[fail] got 1 JOIN[s] syn expected 2
> > - synack[fail] got 1 JOIN[s] synack expected 2
> > - ack[ ok ]
> > Server ns stats
> > MPTcpExtMPCapableSYNRX 1 0.0
> > MPTcpExtMPCapableACKRX 1 0.0
> > MPTcpExtMPJoinSynRx 1 0.0
> > MPTcpExtMPJoinAckRx 1 0.0
> > Client ns stats
> > MPTcpExtMPJoinSynAckRx 1 0.0
> > 14 signal address with syn cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
> > 15 subflow and signal w cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
> > 16 subflows and signal w. cookies syn[fail] got 1 JOIN[s] syn expected 3
> > - synack[fail] got 1 JOIN[s] synack expected 3
> > - ack[fail] got 1 JOIN[s] ack expected 3
> > Server ns stats
> > MPTcpExtMPCapableSYNRX 1 0.0
> > MPTcpExtMPCapableACKRX 1 0.0
> > MPTcpExtMPJoinSynRx 1 0.0
> > MPTcpExtMPJoinAckRx 1 0.0
> > Client ns stats
> > MPTcpExtMPJoinSynAckRx 1 0.0
>
> I sometime see similar errors, but they are quite unfrequent, and looks
> tied to the self-test fragility: we use some hard-coded sleep to allow
> for MPJ/ADD_ADDR handshake. If the VM running the test is my change
> slowed down a bit, self-tests will fail.
>
> Is the above failure reproducible for you?
Yes, I got this self-test failure every time both on -net and linux-next.
-Geliang
>
> Thanks,
>
> Paolo
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak
@ 2020-09-14 13:54 Paolo Abeni
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2020-09-14 13:54 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]
On Mon, 2020-09-14 at 17:34 +0800, Geliang Tang wrote:
> Hi Paolo,
>
> Thanks for your patch. I tested it. It fixed the memory leak problem.
> But I got this
> selftest fail output:
>
> 11 single subflow with syn cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
> 12 multiple subflows with syn cookies syn[fail] got 1 JOIN[s] syn expected 2
> - synack[fail] got 1 JOIN[s] synack expected 2
> - ack[fail] got 1 JOIN[s] ack expected 2
> Server ns stats
> MPTcpExtMPCapableSYNRX 1 0.0
> MPTcpExtMPCapableACKRX 1 0.0
> MPTcpExtMPJoinSynRx 1 0.0
> MPTcpExtMPJoinAckRx 1 0.0
> Client ns stats
> MPTcpExtMPJoinSynAckRx 1 0.0
> 13 subflows limited by server w cookies syn[fail] got 1 JOIN[s] syn expected 2
> - synack[fail] got 1 JOIN[s] synack expected 2
> - ack[ ok ]
> Server ns stats
> MPTcpExtMPCapableSYNRX 1 0.0
> MPTcpExtMPCapableACKRX 1 0.0
> MPTcpExtMPJoinSynRx 1 0.0
> MPTcpExtMPJoinAckRx 1 0.0
> Client ns stats
> MPTcpExtMPJoinSynAckRx 1 0.0
> 14 signal address with syn cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
> 15 subflow and signal w cookies syn[ ok ] - synack[ ok ] - ack[ ok ]
> 16 subflows and signal w. cookies syn[fail] got 1 JOIN[s] syn expected 3
> - synack[fail] got 1 JOIN[s] synack expected 3
> - ack[fail] got 1 JOIN[s] ack expected 3
> Server ns stats
> MPTcpExtMPCapableSYNRX 1 0.0
> MPTcpExtMPCapableACKRX 1 0.0
> MPTcpExtMPJoinSynRx 1 0.0
> MPTcpExtMPJoinAckRx 1 0.0
> Client ns stats
> MPTcpExtMPJoinSynAckRx 1 0.0
I sometime see similar errors, but they are quite unfrequent, and looks
tied to the self-test fragility: we use some hard-coded sleep to allow
for MPJ/ADD_ADDR handshake. If the VM running the test is my change
slowed down a bit, self-tests will fail.
Is the above failure reproducible for you?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-02 22:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 9:34 [MPTCP] Re: [PATCH net] tcp: fix syn cookied MPTCP request socket leak Geliang Tang
2020-09-14 13:54 Paolo Abeni
2020-09-15 3:03 Geliang Tang
2020-10-02 22:35 David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.