Commit 581c8cbf authored by Gang Yan's avatar Gang Yan Committed by Jakub Kicinski
Browse files

mptcp: annotate data-races around subflow->fully_established 

We introduce the same handling for potential data races with the
'fully_established' flag in subflow as previously done for
msk->fully_established.

Additionally, we make a crucial change: convert the subflow's
'fully_established' from 'bit_field' to 'bool' type. This is
necessary because methods for avoiding data races don't work well
with 'bit_field'. Specifically, the 'READ_ONCE' needs to know
the size of the variable being accessed, which is not supported in
'bit_field'. Also, 'test_bit' expect the address of 'bit_field'.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/516


Signed-off-by: default avatarGang Yan <yangang@kylinos.cn>
Reviewed-by: default avatarMatthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: default avatarMatthieu Baerts (NGI0) <matttbe@kernel.org>
Link: https://patch.msgid.link/20241021-net-next-mptcp-misc-6-13-v1-2-1ef02746504a@kernel.org


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

net/mptcp/diag.c

+1 −1
Original line number Diff line number Diff line
@@ -47,7 +47,7 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb) 
		flags |= MPTCP_SUBFLOW_FLAG_BKUP_REM;

	if (sf->request_bkup)

		flags |= MPTCP_SUBFLOW_FLAG_BKUP_LOC;

	if (sf->fully_established)

	if (READ_ONCE(sf->fully_established))

		flags |= MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED;

	if (sf->conn_finished)

		flags |= MPTCP_SUBFLOW_FLAG_CONNECTED;

net/mptcp/options.c

+2 −2
Original line number Diff line number Diff line
@@ -461,7 +461,7 @@ static bool mptcp_established_options_mp(struct sock *sk, struct sk_buff *skb, 
		return false;



	/* MPC/MPJ needed only on 3rd ack packet, DATA_FIN and TCP shutdown take precedence */

	if (subflow->fully_established || snd_data_fin_enable ||

	if (READ_ONCE(subflow->fully_established) || snd_data_fin_enable ||

	    subflow->snd_isn != TCP_SKB_CB(skb)->seq ||

	    sk->sk_state != TCP_ESTABLISHED)

		return false;
@@ -930,7 +930,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk, 
	/* here we can process OoO, in-window pkts, only in-sequence 4th ack

	 * will make the subflow fully established

	 */

	if (likely(subflow->fully_established)) {

	if (likely(READ_ONCE(subflow->fully_established))) {

		/* on passive sockets, check for 3rd ack retransmission

		 * note that msk is always set by subflow_syn_recv_sock()

		 * for mp_join subflows

net/mptcp/protocol.c

+1 −1
Original line number Diff line number Diff line
@@ -3511,7 +3511,7 @@ static void schedule_3rdack_retransmission(struct sock *ssk) 
	struct tcp_sock *tp = tcp_sk(ssk);

	unsigned long timeout;



	if (mptcp_subflow_ctx(ssk)->fully_established)

	if (READ_ONCE(mptcp_subflow_ctx(ssk)->fully_established))

		return;



	/* reschedule with a timeout above RTT, as we must look only for drop */

net/mptcp/protocol.h

+3 −3
Original line number Diff line number Diff line
@@ -513,7 +513,6 @@ struct mptcp_subflow_context { 
		request_bkup : 1,

		mp_capable : 1,	    /* remote is MPTCP capable */

		mp_join : 1,	    /* remote is JOINing */

		fully_established : 1,	    /* path validated */

		pm_notified : 1,    /* PM hook called for established status */

		conn_finished : 1,

		map_valid : 1,
@@ -532,10 +531,11 @@ struct mptcp_subflow_context { 
		is_mptfo : 1,	    /* subflow is doing TFO */

		close_event_done : 1,       /* has done the post-closed part */

		mpc_drop : 1,	    /* the MPC option has been dropped in a rtx */

		__unused : 8;

		__unused : 9;

	bool	data_avail;

	bool	scheduled;

	bool	pm_listener;	    /* a listener managed by the kernel PM? */

	bool	fully_established;  /* path validated */

	u32	remote_nonce;

	u64	thmac;

	u32	local_nonce;
@@ -780,7 +780,7 @@ static inline bool __tcp_can_send(const struct sock *ssk) 
static inline bool __mptcp_subflow_active(struct mptcp_subflow_context *subflow)

{

	/* can't send if JOIN hasn't completed yet (i.e. is usable for mptcp) */

	if (subflow->request_join && !subflow->fully_established)

	if (subflow->request_join && !READ_ONCE(subflow->fully_established))

		return false;



	return __tcp_can_send(mptcp_subflow_tcp_sock(subflow));

net/mptcp/subflow.c

+2 −2
Original line number Diff line number Diff line
@@ -800,7 +800,7 @@ void __mptcp_subflow_fully_established(struct mptcp_sock *msk, 
				       const struct mptcp_options_received *mp_opt)

{

	subflow_set_remote_key(msk, subflow, mp_opt);

	subflow->fully_established = 1;

	WRITE_ONCE(subflow->fully_established, true);

	WRITE_ONCE(msk->fully_established, true);



	if (subflow->is_mptfo)
@@ -2062,7 +2062,7 @@ static void subflow_ulp_clone(const struct request_sock *req, 
	} else if (subflow_req->mp_join) {

		new_ctx->ssn_offset = subflow_req->ssn_offset;

		new_ctx->mp_join = 1;

		new_ctx->fully_established = 1;

		WRITE_ONCE(new_ctx->fully_established, true);

		new_ctx->remote_key_valid = 1;

		new_ctx->backup = subflow_req->backup;

		new_ctx->request_bkup = subflow_req->request_bkup;