Reduce the differences between TCP and MPTCP for TLS usage

net/mptcp/protocol.c static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset, · line 457

[Severity: Medium]
As reported by Sashiko on v15 [1], can __mptcp_sync_rcv_sequence() compute an off-by-one map_seq and end_seq if mptcp_check_data_fin() runs before mptcp_release_cb()?

When mptcp_worker() holds lock_sock(sk) on a passive TFO socket with SYN data queued in sk->sk_receive_queue (msk->rcvd_dummy_seq is true), the peer's third ACK carrying MP_CAPABLE and a zero-payload DATA_FIN (with data_seq equal to subflow->iasn and data_len equal to 1) can arrive in BH context.

In that case, subflow_set_remote_key() sets msk->ack_seq to subflow->iasn and sets MPTCP_SYNC_SEQ in msk->cb_flags, while mptcp_incoming_options() / get_mapping_status() calls mptcp_update_rcv_data_fin() to record msk->rcv_data_fin_seq = subflow->iasn and msk->rcv_data_fin = 1 without queuing an skb to msk->backlog_list.

Before release_sock(sk) invokes mptcp_release_cb() in net/mptcp/protocol.c, mptcp_worker() calls mptcp_check_data_fin():

net/mptcp/protocol.c:mptcp_worker()

	mptcp_check_send_data_fin(sk);
	mptcp_check_data_fin_ack(sk);
	mptcp_check_data_fin(sk);

which sees msk->ack_seq == rcv_data_fin_seq and advances msk->ack_seq by 1:

net/mptcp/protocol.c:mptcp_check_data_fin()

	if (mptcp_pending_data_fin(sk, &rcv_data_fin_seq)) {
		WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
		WRITE_ONCE(msk->rcv_data_fin, 0);

When release_sock(sk) subsequently runs mptcp_release_cb() -> __mptcp_sync_rcv_sequence(sk), msk->ack_seq is already subflow->iasn + 1, so both MPTCP_SKB_CB(skb)->map_seq and MPTCP_SKB_CB(skb)->end_seq on the TFO skb will be shifted forward by 1 byte.

[ ... ]

net/mptcp/subflow.c static void subflow_set_remote_key(struct mptcp_sock *msk, · line 543

[Severity: Medium]
As reported by Sashiko on v15 [2], does deferring __mptcp_sync_rcv_sequence() here while immediately updating msk->ack_seq cause mptcp_inq_hint() to return a bogus value when sock_owned_by_user(sk) is true?

In mptcp_fastopen_subflow_synack_set_params() in net/mptcp/fastopen.c, the TFO skb is queued onto sk->sk_receive_queue with MPTCP_SKB_CB(skb)->map_seq set to -skb->len and end_seq set to 0 while msk->ack_seq is 0.

If user space holds lock_sock(sk) in mptcp_ioctl() (for SIOCINQ) or mptcp_recvmsg() when the peer's third ACK arrives in BH context, subflow_set_remote_key() updates msk->ack_seq to subflow->iasn immediately, but defers __mptcp_sync_rcv_sequence(sk) via MPTCP_SYNC_SEQ until release_sock(sk) runs mptcp_release_cb().

When the third ACK carries no payload (or arrives after mptcp_move_skbs() has checked msk->backlog_list), __mptcp_move_skb() is not called before mptcp_inq_hint(sk) runs under lock_sock(sk):

net/mptcp/protocol.c:mptcp_ioctl()

	lock_sock(sk);
	if (mptcp_move_skbs(sk))
		mptcp_cleanup_rbuf(msk, 0);
	*karg = mptcp_inq_hint(sk);
	release_sock(sk);

and similarly in mptcp_recvmsg() (for TCP_CM_INQ) and __mptcp_recvmsg_mskq() -> mptcp_rcv_space_adjust().

In mptcp_inq_hint() in net/mptcp/protocol.c:

	skb = skb_peek(&sk->sk_receive_queue);
	if (skb) {
		u64 hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;

		if (hint_val >= INT_MAX)
			return INT_MAX;

		return (unsigned int)hint_val;
	}

hint_val subtracts the unsynchronized dummy map_seq (-skb->len + offset) from subflow->iasn, evaluating to subflow->iasn + skb->len - offset and returning INT_MAX (or a corrupted value) instead of the unread TFO payload length.

[1] https://lore.kernel.org/r/25f42bae1af43649a5c89ca10cfe0d962941c6f7.1792718041.git.tanggeliang@kylinos.cn
[2] https://lore.kernel.org/r/25f42bae1af43649a5c89ca10cfe0d962941c6f7.1792718041.git.tanggeliang@kylinos.cn

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

net/mptcp/protocol.c static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset, · line 431

[Severity: High]
As reported by Sashiko on v17 [1], can subtracting offset from mptcp_subflow_get_mapped_dsn(subflow) in mptcp_init_skb() and dropping the offset check in __mptcp_try_coalesce() corrupt the MPTCP data stream when an skb has a non-zero subflow offset?

An skb at the head of ssk->sk_receive_queue can reach __mptcp_move_skbs_from_subflow() with offset = tp->copied_seq - TCP_SKB_CB(skb)->seq > 0 while carrying a new DSS mapping M2 starting at tp->copied_seq (skb->data[offset..skb->len)) with mapped_dsn = mptcp_subflow_get_mapped_dsn(subflow):

1. In subflow_check_data_avail() -> get_mapping_status(), a single skb can span the tail of a previous DSS mapping M1 in skb->data[0..offset) and the start of a new DSS mapping M2 in skb->data[offset..skb->len):

   net/mptcp/subflow.c:get_mapping_status() {
       ...
       if (!skb_is_fully_mapped(ssk, skb)) {
           /* will validate the next map after consuming the current one */
           goto validate_csum;
       }
   }

If M1 was already received on another subflow (ack_seq < msk->ack_seq), mptcp_subflow_discard_data() advances tp->copied_seq by offset without eating the skb and resets subflow->map_valid = 0. On the next iteration, get_mapping_status() installs M2 starting offset bytes into the skb.

2. Similarly, tcp_data_queue() or tcp_ofo_queue() can queue a partially overlapping TCP segment with TCP_SKB_CB(skb)->seq < tp->copied_seq < TCP_SKB_CB(skb)->end_seq at a DSS mapping boundary.

Because MPTCP stripes mappings across subflows, M1 and M2 on the same subflow have contiguous SSNs but non-contiguous MPTCP DSNs, so skb->data[0..offset) does not belong to the DSN range [mapped_dsn - offset, mapped_dsn). Setting MPTCP_SKB_CB(skb)->map_seq = mapped_dsn - offset (and map_seq64 = mapped_dsn - offset in the follow-up commit syncing the skb cb layout with tcp) causes several problems when M2 is out of order (mapped_dsn > msk->ack_seq) or queued to msk->backlog_list:

- If mapped_dsn - offset <= msk->ack_seq < mapped_dsn, __mptcp_move_skb() treats the out-of-order skb as in-sequence or partially overlapping, advances msk->ack_seq and msk->bytes_received across the unreceived gap [msk->ack_seq, mapped_dsn), coalesces skb->data[0..skb->len) onto the receive queue tail via mptcp_try_coalesce() if mapped_dsn - offset == MPTCP_SKB_CB(tail)->end_seq, delivers the stale M1 bytes in skb->data[0..offset) to user space, and causes the genuine data for [msk->ack_seq, mapped_dsn) to be dropped later as MPTCP_MIB_DUPDATA.

- If mapped_dsn - offset > msk->ack_seq, mptcp_data_queue_ofo() inserts or coalesces the skb in msk->out_of_order_queue under the fabricated range [mapped_dsn - offset, end_seq), dropping or replacing legitimate out-of-order segments covering [mapped_dsn - offset, mapped_dsn) and delivering skb->data[0..offset) to user space when __mptcp_ofo_queue() dequeues it.

- In __mptcp_add_backlog(), if mapped_dsn - offset == MPTCP_SKB_CB(tail)->end_seq for a prior backlog skb, __mptcp_try_coalesce() coalesces skb->data[0..skb->len) including the stale prefix skb->data[0..offset) into tail.

net/mptcp/protocol.h struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk); · line 895

[Severity: High]
As reported by Sashiko on v15 [2], won't mptcp_iasn() be off by one if a DATA_FIN is processed before the deferred MPTCP_SYNC_SEQ runs in mptcp_release_cb()?

The commit message notes that mptcp_check_data_fin() incrementing ack_seq before MPTCP_SYNC_SEQ runs would require a DATA_FIN to arrive before the MPC third ACK on a passive TFO socket. However, subflow_set_remote_key() sets MPTCP_SYNC_SEQ at the moment the MPC third ACK (or SYN-ACK on an active socket) arrives while sock_owned_by_user(sk) is true, which also establishes the connection and sets msk->ack_seq = subflow->iasn.

If the peer sends a payload with DATA_FIN (or a subflow closes immediately) together with or right after the MPC third ACK while lock_sock(sk) is still held:

1. When data + DATA_FIN is queued to msk->backlog_list while lock_sock(sk) is held, release_sock(sk) invokes mptcp_release_cb(sk), which spools the backlog via __mptcp_move_skbs() in its loop before checking MPTCP_SYNC_SEQ at the end of the function:

   net/mptcp/protocol.c:__mptcp_move_skbs() {
       ...
       __mptcp_ofo_queue(msk);
       if (moved)
           mptcp_check_data_fin((struct sock *)msk);
       return moved;
   }

Inside __mptcp_move_skbs(), __mptcp_move_skb() syncs any TFO skb using mptcp_iasn(msk) == subflow->iasn and queues the new skb at map_seq = subflow->iasn, and then mptcp_check_data_fin() increments msk->ack_seq by 1 without incrementing msk->bytes_received. When mptcp_release_cb() then processes MPTCP_SYNC_SEQ, msk->copied_seq += mptcp_iasn(msk) adds subflow->iasn + 1, advancing msk->copied_seq one byte past the queued skb's map_seq and skipping the first byte of unread payload.

2. Similarly, if a 0-payload DATA_FIN arrives after the third ACK while mptcp_worker() holds lock_sock(sk), mptcp_worker() calls mptcp_check_data_fin(sk) and increments msk->ack_seq to subflow->iasn + 1 before release_sock(sk) runs mptcp_release_cb(sk).

net/mptcp/subflow.c static void subflow_set_remote_key(struct mptcp_sock *msk, · line 545

[Severity: High]
As reported by Sashiko on v15 [2], when subflow_set_remote_key() runs in BH context while sock_owned_by_user(sk) is true, it immediately updates msk->ack_seq to subflow->iasn and sets msk->can_ack = true, but defers updating msk->copied_seq via MPTCP_SYNC_SEQ until mptcp_release_cb(). What happens if the user context holding lock_sock(sk) accesses msk->copied_seq or spools the backlog before release_sock(sk) runs?

Specifically, if the peer's MP_CAPABLE third ACK (on a passive TFO socket) or SYN-ACK (on an active client socket) arrives with or followed by data while user space holds lock_sock(sk):

1. In mptcp_recvmsg(), mptcp_recv_skb(), and mptcp_ioctl(SIOCINQ), mptcp_move_skbs(sk) is called while holding lock_sock(sk). Inside mptcp_move_skbs() -> __mptcp_move_skbs() -> __mptcp_move_skb(), __mptcp_sync_rcv_sequence(sk) rebases any queued TFO skb's map_seq from -(u64)skb->len to mptcp_iasn(msk) - skb->len and clears msk->rcvd_dummy_seq = false, and __mptcp_move_skb() moves the IASN-mapped backlog skbs into sk->sk_receive_queue. However, MPTCP_SYNC_SEQ is still pending, so msk->copied_seq (and peek_seq = msk->copied_seq + copied in mptcp_recvmsg()) remains in the pre-IASN sequence space.

2. In mptcp_recv_skb(), the loop then computes:

   offset = msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq;
   if (offset < skb->len) {
       *off = offset;
       return skb;
   }
   mptcp_eat_recv_skb(sk, skb);

Because msk->copied_seq has not been rebased by subflow->iasn, offset underflows to a value >= skb->len, and mptcp_eat_recv_skb() frees and drops every unread skb in sk->sk_receive_queue without advancing msk->copied_seq or msk->bytes_consumed. Even after the later commit in this series ("mptcp:
defer read_sock cleanup to mptcp_worker") removes mptcp_move_skbs() from mptcp_recv_skb(), mptcp_release_cb() still runs __mptcp_move_skbs() and calls sk->sk_data_ready(sk) in its loop before processing MPTCP_SYNC_SEQ, so kTLS RX synchronously invokes __mptcp_read_sock() -> mptcp_recv_skb() with an unsynchronized msk->copied_seq and drops the queued skbs.

3. In mptcp_recvmsg(), the next iteration of __mptcp_recvmsg_mskq() computes offset = *seq - MPTCP_SKB_CB(skb)->map_seq, which similarly underflows by subflow->iasn; when offset >= skb->len, it treats all unread skbs in sk->sk_receive_queue as already-peeked data, sets *last = skb, and blocks in sk_wait_data() (or copies from a corrupted mid-skb offset if a 32-bit offset wraps below skb->len in the follow-up commit).

4. In mptcp_inq_hint() (called under lock_sock(sk) by mptcp_ioctl(SIOCINQ), TCP_CM_INQ, and mptcp_rcv_space_adjust()), READ_ONCE(msk->ack_seq) - READ_ONCE(msk->copied_seq) subtracts the pre-IASN msk->copied_seq from the post-IASN msk->ack_seq, returning INT_MAX and corrupting receive buffer autotuning.

[1] https://lore.kernel.org/r/9a98772b14e535454de5bafbf113aa72a3ee6f44.1793326853.git.tanggeliang@kylinos.cn
[2] https://lore.kernel.org/r/25f42bae1af43649a5c89ca10cfe0d962941c6f7.1792718041.git.tanggeliang@kylinos.cn

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

net/mptcp/protocol.c static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb) · line 320

[Severity: High]
Can a 64-bit sequence number wrap-around bypass this admission check when msk->ack_seq has advanced past max_seq (msk->rcv_wnd_sent), allowing out-of-window data to be queued and then dequeued via 32-bit truncation in __mptcp_ofo_queue()?

In subflow_set_remote_key(), both msk->ack_seq and msk->rcv_wnd_sent are initialized to subflow->iasn:

net/mptcp/subflow.c:subflow_set_remote_key() {
	...
	WRITE_ONCE(msk->ack_seq, subflow->iasn);
	WRITE_ONCE(msk->can_ack, true);
	atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
}

On an active MPTCP connection before the first outgoing DSS ACK updates msk->rcv_wnd_sent via mptcp_set_rwin(), if in-order data of length N arrives and is accepted by __mptcp_move_skb(), msk->ack_seq advances to subflow->iasn + N while max_seq remains at subflow->iasn.

When msk->ack_seq > max_seq, a segment carrying a 64-bit data_seq in the range [max_seq + 2^63 + 1, msk->ack_seq + 2^63 - 1] (for example, subflow->iasn + 0x8000000000000000ULL + k with 1 <= k < N and k + len > N) satisfies after64(map_seq64, msk->ack_seq) in __mptcp_move_skb(), but in mptcp_data_queue_ofo() both after64(map_seq64, max_seq) and after64(end_seq, max_seq) evaluate to false because max_seq - map_seq64 is positive in signed 64-bit arithmetic.

Once queued into msk->out_of_order_queue, MPTCP_SKB_CB(skb)->map_seq and end_seq are truncated to 32 bits, dropping bit 63 so that map_seq becomes (u32)subflow->iasn + k. When __mptcp_ofo_queue() runs, after(map_seq, ack_seq) is false and after(end_seq, ack_seq) is true, so the out-of-window SKB is moved into sk->sk_receive_queue and advances msk->ack_seq.

Should mptcp_data_queue_ofo() clamp or validate max_seq against msk->ack_seq (or check the distance from msk->ack_seq) before performing the window check?

[ ... ]

net/mptcp/protocol.c static void mptcp_init_skb(struct sock *ssk, struct sk_buff *skb, int offset) · line 436

[Severity: Critical]
As reported by Sashiko on v17 [1], does subtracting offset from map_seq64 and map_seq without trimming the prefix bytes from skb corrupt the MPTCP data stream when an SKB with offset > 0 carries an out-of-order mapping?

In __mptcp_move_skbs_from_subflow(), offset = tp->copied_seq - TCP_SKB_CB(skb)->seq is non-zero when the first offset bytes of the head SKB on ssk->sk_receive_queue were already consumed or discarded (for instance, when a single subflow SKB spans two DSS mappings and mptcp_subflow_discard_data() advances tp->copied_seq past the first mapping). Only the bytes in [offset, skb->len) belong to the active DSS mapping starting at mapped_dsn = mptcp_subflow_get_mapped_dsn(subflow).

When mapped_dsn > msk->ack_seq (out-of-order MPTCP data) and offset > 0, subtracting offset in mptcp_init_skb() shifts map_seq64 and map_seq backwards into the unreceived MPTCP sequence range [mapped_dsn - offset, mapped_dsn):

1. If mapped_dsn - offset <= msk->ack_seq, __mptcp_move_skb() treats the SKB as in-sequence or partially overlapping msk->ack_seq, queues it directly to sk->sk_receive_queue (or coalesces it into the tail SKB via __mptcp_try_coalesce()), and advances msk->ack_seq across the unreceived gap [msk->ack_seq, mapped_dsn), delivering the stale subflow prefix bytes [0, offset) to user space.

2. If mapped_dsn - offset > msk->ack_seq, mptcp_data_queue_ofo() inserts the SKB into out_of_order_queue covering [mapped_dsn - offset, mapped_dsn + len). From there, __mptcp_try_coalesce() can merge the unpulled [0, offset) prefix into a preceding OFO SKB ending at mapped_dsn - offset, subsequent legitimate segments for [mapped_dsn - offset, mapped_dsn) are dropped as duplicate data, and __mptcp_ofo_queue() dequeues the SKB as soon as msk->ack_seq reaches mapped_dsn - offset rather than mapped_dsn.

net/mptcp/protocol.c static struct sk_buff *mptcp_recv_skb(struct sock *sk, u32 *off) · line 4632

[Severity: High]
What happens in mptcp_recvmsg() and mptcp_recv_skb() if mptcp_move_skbs() spools the backlog while MPTCP_SYNC_SEQ is still pending on msk->cb_flags?

When subflow_set_remote_key() runs in softirq while the MPTCP socket is owned by user space (for example, on a passive TFO socket during mptcp_recvmsg() or __mptcp_read_sock()), it sets msk->ack_seq to subflow->iasn immediately, but defers updating msk->copied_seq by setting MPTCP_SYNC_SEQ in msk->cb_flags:

net/mptcp/subflow.c:subflow_set_remote_key() {
	...
	if (!sock_owned_by_user(sk)) {
		msk->copied_seq += subflow->iasn;
		__mptcp_sync_rcv_sequence(sk);
	} else {
		set_bit(MPTCP_SYNC_SEQ, &msk->cb_flags);
	}
}

If data arrives in the 3rd ACK or immediately afterward while user space still holds the socket lock, the incoming SKBs are queued to msk->backlog_list with map_seq64 and map_seq in the subflow->iasn sequence space. Before release_sock() runs mptcp_release_cb(), both mptcp_recvmsg() and mptcp_recv_skb() call mptcp_move_skbs(sk):

1. Inside mptcp_move_skbs() -> __mptcp_move_skbs() -> __mptcp_move_skb(), __mptcp_sync_rcv_sequence(sk) updates any remaining TFO SKB in sk->sk_receive_queue to map_seq = (u32)(mptcp_iasn(msk) - skb->len) and clears msk->rcvd_dummy_seq = false, while queuing the spooled SKBs with IASN-based map_seq values into sk->sk_receive_queue. However, msk->copied_seq is still in the pre-IASN sequence space.

2. In mptcp_recv_skb(), offset = (u32)msk->copied_seq - MPTCP_SKB_CB(skb)->map_seq is then off by -(u32)iasn, so offset >= skb->len holds whenever (u32)(-iasn) >= skb->len and the loop eats and drops every queued SKB via mptcp_eat_recv_skb(). Follow-up commit 68366c7579c4 ("mptcp: defer read_sock cleanup to mptcp_worker") later removes the mptcp_move_skbs() call from mptcp_recv_skb(), but does not address mptcp_recvmsg() or mptcp_release_cb().

3. In mptcp_recvmsg(), after mptcp_move_skbs(sk) returns true, the loop continues into __mptcp_recvmsg_mskq() with *seq still unsynchronized, causing u32 offset = (u32)(*seq) - MPTCP_SKB_CB(skb)->map_seq to skip unread SKBs or copy from a corrupted offset.

4. Even when release_sock() runs mptcp_release_cb(), __mptcp_move_skbs() runs before the test for MPTCP_SYNC_SEQ:

net/mptcp/protocol.c:mptcp_release_cb() {
	for (;;) {
		...
		if (__mptcp_move_skbs(sk, &join_list, &moved))
			...
		if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {
			msk->copied_seq += mptcp_iasn(msk);
			__mptcp_sync_rcv_sequence(sk);
		}
	}
}

If the spooled backlog carries a DATA_FIN, __mptcp_move_skbs() calls mptcp_check_data_fin(), which increments msk->ack_seq by 1 without incrementing msk->bytes_received. When MPTCP_SYNC_SEQ then adds mptcp_iasn(msk) (which is msk->ack_seq - msk->bytes_received + msk->tfo_skb_len), msk->copied_seq is advanced by iasn + 1, skipping 1 byte of unread payload.

Should MPTCP_SYNC_SEQ be synchronized inside __mptcp_sync_rcv_sequence() (or before spooling backlog SKBs in mptcp_move_skbs() and mptcp_release_cb()) so that msk->copied_seq and the receive queue SKBs never end up in different sequence spaces?

[1] https://lore.kernel.org/r/fe11356d3e5637de94fbe89540271d5f4ee381b4.1793326853.git.tanggeliang@kylinos.cn

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

net/mptcp/protocol.c static bool __mptcp_move_skb(struct sock *sk, struct sk_buff *skb) · line 506

[Severity: High]
When __mptcp_move_skbs_from_subflow() passes a non-zero offset to mptcp_init_skb() (for example, when the leading offset bytes of skb on ssk->sk_receive_queue belonged to a prior or duplicate DSS mapping that was already consumed or discarded via subflow_check_data_avail() -> mptcp_subflow_discard_data()), mptcp_init_skb() subtracts offset from mptcp_subflow_get_mapped_dsn(subflow):

	MPTCP_SKB_CB(skb)->map_seq64 = mptcp_subflow_get_mapped_dsn(subflow) -
				       offset;
	MPTCP_SKB_CB(skb)->map_seq = (u32)MPTCP_SKB_CB(skb)->map_seq64;
	MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq + skb->len;

If the new DSS mapping starting at offset has mapped_dsn > msk->ack_seq (out-of-order at the MPTCP level because the range [msk->ack_seq, mapped_dsn) is still in flight on another subflow), doesn't subtracting offset assign the DSN range [mapped_dsn - offset, mapped_dsn) to the already-discarded bytes 0..offset of skb?

Specifically, in __mptcp_move_skb():

1. When mapped_dsn - offset <= msk->ack_seq < mapped_dsn, __mptcp_move_skb() sees map_seq64 <= msk->ack_seq and map_seq64 + skb->len > msk->ack_seq. Instead of queueing skb to msk->out_of_order_queue, it takes the in-sequence or partial-packet insert path, advances msk->ack_seq across the unreceived DSN gap, and causes __mptcp_recvmsg_mskq() to copy the stale subflow bytes at 0..offset to user-space.

2. When mapped_dsn - offset > msk->ack_seq, __mptcp_move_skb() queues skb into msk->out_of_order_queue with map_seq = mapped_dsn - offset, so mptcp_data_queue_ofo() and __mptcp_ofo_queue() treat bytes 0..offset as valid data for [mapped_dsn - offset, mapped_dsn) and drop the real segments arriving on other subflows as duplicate data.

Can this corrupt the MPTCP receive stream when an out-of-order subflow SKB has a non-zero offset?

net/mptcp/subflow.c static void subflow_set_remote_key(struct mptcp_sock *msk, · line 545

[Severity: High]
When subflow_set_remote_key() runs in BH context while sock_owned_by_user(sk) is true, msk->ack_seq is updated immediately to subflow->iasn while updating msk->copied_seq is deferred via MPTCP_SYNC_SEQ until mptcp_release_cb(). Could leaving msk->copied_seq unsynchronized while the user holds the socket lock cause several issues?

First, in net/mptcp/protocol.c:mptcp_recvmsg():

		bytes_read = __mptcp_recvmsg_mskq(sk, msg, len - copied, flags,
						  seq, &tss, &cmsg_flags,
						  &last);
		...
		if (!list_empty(&msk->backlog_list) && mptcp_move_skbs(sk)) {
			if (flags & MSG_PEEK)
				peek_seq = msk->copied_seq + copied;
			continue;
		}

If payload arrives with or right after the MP_CAPABLE packet while the user holds lock_sock(sk) in mptcp_recvmsg(), mptcp_data_ready() queues the IASN-based SKBs onto msk->backlog_list. Before release_sock(sk) and mptcp_release_cb() run, mptcp_recvmsg() calls mptcp_move_skbs(sk) and continues to the next iteration of __mptcp_recvmsg_mskq(). Inside mptcp_move_skbs() -> __mptcp_move_skb(), __mptcp_sync_rcv_sequence(sk) updates any TFO SKB in sk->sk_receive_queue to the IASN sequence space and clears msk->rcvd_dummy_seq, and queues the new IASN-based SKBs onto sk->sk_receive_queue, while MPTCP_SYNC_SEQ is still pending and msk->copied_seq has not been advanced by subflow->iasn.

On the next iteration, __mptcp_recvmsg_mskq() computes:

		u32 offset = (u32)(*seq) - MPTCP_SKB_CB(skb)->map_seq;

using the unsynchronized msk->copied_seq against the IASN-based map_seq. When offset >= skb->len, __mptcp_recvmsg_mskq() treats the unread SKB as already-peeked data, sets *last = skb, and skips it; sk_wait_data(sk, &timeo, last) then blocks waiting for skb_peek_tail(&sk->sk_receive_queue) != last even after release_sock(sk) runs MPTCP_SYNC_SEQ, stalling the reader with unread data in the queue. When offset < skb->len, __mptcp_recvmsg_mskq() copies from a bogus offset inside skb and eats the remainder.

Second, any call to mptcp_inq_hint(sk) while MPTCP_SYNC_SEQ is pending (via mptcp_ioctl() for SIOCINQ, mptcp_recvmsg() for TCP_CM_INQ, or mptcp_rcv_space_adjust()) subtracts the unsynchronized msk->copied_seq from the IASN-based msk->ack_seq and returns INT_MAX.

Third, in net/mptcp/protocol.c:mptcp_release_cb():

	do {
		...
		if (__mptcp_move_skbs(sk))
			need_push = true;
		...
	} while (!list_empty(&msk->backlog_list));

	if (need_push)
		__mptcp_push_pending(sk, 0);

	if (msk->cb_flags) {
		...
		if (__test_and_clear_bit(MPTCP_SYNC_SEQ, &msk->cb_flags)) {
			msk->copied_seq += mptcp_iasn(msk);
			__mptcp_sync_rcv_sequence(sk);
		}
	}

Because __mptcp_move_skbs(sk) runs before MPTCP_SYNC_SEQ is handled, if a spooled backlog SKB carries a DATA_FIN (arriving with or right after the 3rd ACK while the user holds lock_sock(sk)), __mptcp_move_skbs() -> mptcp_check_data_fin() increments msk->ack_seq by 1 without incrementing msk->bytes_received, causing mptcp_iasn(msk) to shift msk->copied_seq one byte past the real IASN.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5