Commit fd897679 authored by Paolo Abeni's avatar Paolo Abeni Committed by Jakub Kicinski
Browse files

mptcp: be careful on MPTCP-level ack. 



We can enter the main mptcp_recvmsg() loop even when
no subflows are connected. As note by Eric, that would
result in a divide by zero oops on ack generation.

Address the issue by checking the subflow status before
sending the ack.

Additionally protect mptcp_recvmsg() against invocation
with weird socket states.

v1 -> v2:
 - removed unneeded inline keyword - Jakub

Reported-and-suggested-by: default avatarEric Dumazet <eric.dumazet@gmail.com>
Fixes: ea4ca586 ("mptcp: refine MPTCP-level ack scheduling")
Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
Link: https://lore.kernel.org/r/5370c0ae03449239e3d1674ddcfb090cf6f20abe.1606253206.git.pabeni@redhat.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent bfd04232

net/mptcp/protocol.c

+49 −18
Original line number Diff line number Diff line
@@ -419,31 +419,57 @@ static bool mptcp_subflow_active(struct mptcp_subflow_context *subflow) 
	return ((1 << ssk->sk_state) & (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT));

}



static void mptcp_send_ack(struct mptcp_sock *msk, bool force)

static bool tcp_can_send_ack(const struct sock *ssk)

{

	return !((1 << inet_sk_state_load(ssk)) &

	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE));

}



static void mptcp_send_ack(struct mptcp_sock *msk)

{

	struct mptcp_subflow_context *subflow;

	struct sock *pick = NULL;



	mptcp_for_each_subflow(msk, subflow) {

		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);



		if (force) {

		lock_sock(ssk);

		if (tcp_can_send_ack(ssk))

			tcp_send_ack(ssk);

		release_sock(ssk);

			continue;

	}

}



		/* if the hintes ssk is still active, use it */

		pick = ssk;

		if (ssk == msk->ack_hint)

			break;

static bool mptcp_subflow_cleanup_rbuf(struct sock *ssk)

{

	int ret;



	lock_sock(ssk);

	ret = tcp_can_send_ack(ssk);

	if (ret)

		tcp_cleanup_rbuf(ssk, 1);

	release_sock(ssk);

	return ret;

}



static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)

{

	struct mptcp_subflow_context *subflow;



	/* if the hinted ssk is still active, try to use it */

	if (likely(msk->ack_hint)) {

		mptcp_for_each_subflow(msk, subflow) {

			struct sock *ssk = mptcp_subflow_tcp_sock(subflow);



			if (msk->ack_hint == ssk &&

			    mptcp_subflow_cleanup_rbuf(ssk))

				return;

		}

	if (!force && pick) {

		lock_sock(pick);

		tcp_cleanup_rbuf(pick, 1);

		release_sock(pick);

	}



	/* otherwise pick the first active subflow */

	mptcp_for_each_subflow(msk, subflow)

		if (mptcp_subflow_cleanup_rbuf(mptcp_subflow_tcp_sock(subflow)))

			return;

}



static bool mptcp_check_data_fin(struct sock *sk)
@@ -494,7 +520,7 @@ static bool mptcp_check_data_fin(struct sock *sk) 


		ret = true;

		mptcp_set_timeout(sk, NULL);

		mptcp_send_ack(msk, true);

		mptcp_send_ack(msk);

		mptcp_close_wake_up(sk);

	}

	return ret;
@@ -1579,6 +1605,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, 
		return -EOPNOTSUPP;



	lock_sock(sk);

	if (unlikely(sk->sk_state == TCP_LISTEN)) {

		copied = -ENOTCONN;

		goto out_err;

	}



	timeo = sock_rcvtimeo(sk, nonblock);



	len = min_t(size_t, len, INT_MAX);
@@ -1604,7 +1635,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, 
		/* be sure to advertise window change */

		old_space = READ_ONCE(msk->old_wspace);

		if ((tcp_space(sk) - old_space) >= old_space)

			mptcp_send_ack(msk, false);

			mptcp_cleanup_rbuf(msk);



		/* only the master socket status is relevant here. The exit

		 * conditions mirror closely tcp_recvmsg()

CRA Git | Maintained and supported by SUSTech CRA and CCSE