Commit ae2dd716 authored by Florian Westphal's avatar Florian Westphal Committed by David S. Miller
Browse files

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 <edumazet@google.com>
Fixes: cec37a6e ("mptcp: Handle MP_CAPABLE options for outgoing connections")
Reported-by: default avatarChristoph Paasch <cpaasch@apple.com>
Tested-by: default avatarChristoph Paasch <cpaasch@apple.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent b2c5b614

include/linux/tcp.h

+0 −2
Original line number Diff line number Diff line
@@ -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;

net/ipv4/syncookies.c

+4 −0
Original line number Diff line number Diff line
@@ -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;

net/ipv4/tcp_input.c

+3 −0
Original line number Diff line number Diff line
@@ -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;

net/ipv6/syncookies.c

+3 −0
Original line number Diff line number Diff line
@@ -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;

net/mptcp/subflow.c

+4 −1
Original line number Diff line number Diff line
@@ -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;

	}