From 23639980572d308e4e75f03d4150b9cf7969c718 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Wed, 29 Jan 2020 15:54:46 +0100 Subject: [PATCH] mptcp: handle tcp fallback when using syn cookies We can't deal with syncookie mode yet, the syncookie rx path will create tcp reqsk, i.e. we get OOB access because we treat tcp reqsk as mptcp reqsk one: TCP: SYN flooding on port 20002. Sending cookies. BUG: KASAN: slab-out-of-bounds in subflow_syn_recv_sock+0x451/0x4d0 net/mptcp/subflow.c:191 Read of size 1 at addr ffff8881167bc148 by task syz-executor099/2120 subflow_syn_recv_sock+0x451/0x4d0 net/mptcp/subflow.c:191 tcp_get_cookie_sock+0xcf/0x520 net/ipv4/syncookies.c:209 cookie_v6_check+0x15a5/0x1e90 net/ipv6/syncookies.c:252 tcp_v6_cookie_check net/ipv6/tcp_ipv6.c:1123 [inline] [..] Bug can be reproduced via "sysctl net.ipv4.tcp_syncookies=2". Note that MPTCP should work with syncookies (4th ack would carry needed state), but it appears better to sort that out in -next so do tcp fallback for now. I removed the MPTCP ifdef for tcp_rsk "is_mptcp" member because if (IS_ENABLED()) is easier to read than "#ifdef IS_ENABLED()/#endif" pair. Cc: Eric Dumazet Fixes: ae5c72c1f119e759 ("mptcp: Handle MP_CAPABLE options for outgoing connections") Reported-by: Christoph Paasch Tested-by: Christoph Paasch Signed-off-by: Florian Westphal Signed-off-by: David S. Miller --- include/linux/tcp.h | 2 -- net/ipv4/syncookies.c | 4 ++++ net/ipv4/tcp_input.c | 3 +++ net/ipv6/syncookies.c | 3 +++ net/mptcp/subflow.c | 5 ++++- 5 files changed, 14 insertions(+), 3 deletions(-) diff --git a/include/linux/tcp.h b/include/linux/tcp.h index 1cf73e6f85ca8..3dc964010feff 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -148,9 +148,7 @@ struct tcp_request_sock { const struct tcp_request_sock_ops *af_specific; u64 snt_synack; /* first SYNACK sent time */ bool tfo_listener; -#if IS_ENABLED(CONFIG_MPTCP) bool is_mptcp; -#endif u32 txhash; u32 rcv_isn; u32 snt_isn; diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 345b2b0ff6185..9a4f6b16c9bce 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -349,6 +349,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) req->ts_recent = tcp_opt.saw_tstamp ? tcp_opt.rcv_tsval : 0; treq->snt_synack = 0; treq->tfo_listener = false; + + if (IS_ENABLED(CONFIG_MPTCP)) + treq->is_mptcp = 0; + if (IS_ENABLED(CONFIG_SMC)) ireq->smc_ok = 0; diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index e8b840a4767e1..e325b4506e252 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -6637,6 +6637,9 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops, af_ops->init_req(req, sk, skb); + if (IS_ENABLED(CONFIG_MPTCP) && want_cookie) + tcp_rsk(req)->is_mptcp = 0; + if (security_inet_conn_request(sk, skb, req)) goto drop_and_free; diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c index 30915f6f31e30..13235a012388b 100644 --- a/net/ipv6/syncookies.c +++ b/net/ipv6/syncookies.c @@ -178,6 +178,9 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) treq = tcp_rsk(req); treq->tfo_listener = false; + if (IS_ENABLED(CONFIG_MPTCP)) + treq->is_mptcp = 0; + if (security_inet_conn_request(sk, skb, req)) goto out_free; diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 205dca1c30b78..c90c0e6ffb823 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -186,6 +186,9 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn); + if (tcp_rsk(req)->is_mptcp == 0) + goto create_child; + /* if the sk is MP_CAPABLE, we try to fetch the client key */ subflow_req = mptcp_subflow_rsk(req); if (subflow_req->mp_capable) { @@ -769,7 +772,7 @@ static void subflow_ulp_clone(const struct request_sock *req, struct mptcp_subflow_context *old_ctx = mptcp_subflow_ctx(newsk); struct mptcp_subflow_context *new_ctx; - if (!subflow_req->mp_capable) { + if (!tcp_rsk(req)->is_mptcp || !subflow_req->mp_capable) { subflow_ulp_fallback(newsk, old_ctx); return; } -- 2.39.5