mptcp: handle tcp fallback when using syn cookies
authorFlorian Westphal <fw@strlen.de>
Wed, 29 Jan 2020 14:54:46 +0000 (15:54 +0100)
committerDavid S. Miller <davem@davemloft.net>
Wed, 29 Jan 2020 16:45:20 +0000 (17:45 +0100)
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 <edumazet@google.com>
Fixes: cec37a6e41aae7bf ("mptcp: Handle MP_CAPABLE options for outgoing connections")
Reported-by: Christoph Paasch <cpaasch@apple.com>
Tested-by: Christoph Paasch <cpaasch@apple.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/linux/tcp.h
net/ipv4/syncookies.c
net/ipv4/tcp_input.c
net/ipv6/syncookies.c
net/mptcp/subflow.c

index 1cf73e6..3dc9640 100644 (file)
@@ -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;
index 345b2b0..9a4f6b1 100644 (file)
@@ -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;
 
index e8b840a..e325b45 100644 (file)
@@ -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;
 
index 30915f6..13235a0 100644 (file)
@@ -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;
 
index 205dca1..c90c0e6 100644 (file)
@@ -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;
        }