net/mptcp/sockopt.c | 27 ++++++++++++++++++++++++--- net/mptcp/subflow.c | 4 +++- 2 files changed, 27 insertions(+), 4 deletions(-)
mptcpi_write_seq, mptcpi_snd_una and mptcpi_rcv_nxt report the raw
64-bit data sequence numbers, seeded from the connection's IDSN/IASN.
Since the IDSN/IASN come from mptcp_crypto_key_sha(), these fields
carry an effectively random offset and are not useful to userspace as
absolute values: a caller has to snapshot two getsockopt(MPTCP_INFO)
calls and subtract to get anything meaningful, which is exactly what
tools/testing/selftests/net/mptcp/mptcp_sockopt.c already does.
mptcp_info also reports mptcpi_bytes_sent, mptcpi_bytes_received and
mptcpi_bytes_acked, which give the same information as a plain byte
count starting at 0. The snapshot-and-diff workaround for the seq
fields is redundant once those are available.
Normalize mptcpi_write_seq, mptcpi_snd_una and mptcpi_rcv_nxt in
mptcp_diag_fill_info() by subtracting the local and remote initial
sequence numbers, read from msk->first's subflow context rather than
caching them on mptcp_sock, to avoid growing every mptcp_sock for a
diag-only need.
msk->first only changes when the whole msk is already in TCP_CLOSE,
confirmed by testing with a debug print in __mptcp_close_ssk() under
mptcp_join.sh: every NULL transition observed had msk_state ==
TCP_CLOSE, none while the connection was still established.
mptcp_close() holds the same sk lock mptcp_diag_fill_info() acquires
via lock_sock_fast(), so no caller of MPTCP_INFO (direct getsockopt,
MPTCP_FULL_INFO, or the netlink diag path in mptcp_diag.c) can
observe that transition mid-flight; if it somehow did, the 0 fallback
just leaves the fields unnormalized on an already-gone socket.
subflow->idsn is set once at handshake time and never modified
afterwards, so it can be read directly. subflow->iasn is incremented
by one in subflow_set_remote_key() to account for the peer's virtual
SYN, and ack_seq carries that same increment, so the increment is
undone here (iasn - 1) to keep rcv_nxt normalized against the same
baseline write_seq and snd_una use.
subflow_set_remote_key() also reordered subflow->remote_key_valid to
be set only after subflow->iasn and the other fields it guards are
fully populated, instead of first. Reading it as a "safe to use iasn
now" flag before this change could observe iasn while it still held
its stale/zero value, which mptcp_diag_fill_info() then depended on.
Note: this changes the semantics of mptcpi_write_seq, mptcpi_snd_una
and mptcpi_rcv_nxt. They no longer report the raw on-the-wire MPTCP
data sequence numbers, but values relative to the connection's
initial sequence numbers. This is intentional. Tools that correlate
these fields directly against captured DSS sequence numbers (e.g.
via tcpdump) will need to account for the offset; mptcpi_bytes_sent,
mptcpi_bytes_received and mptcpi_bytes_acked remain unaffected and
are the recommended fields for tracking absolute transferred-byte
counts.
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
Changes since v3:
- Reordered subflow->remote_key_valid to be set after iasn is fully
populated in subflow_set_remote_key(), instead of before: reading
remote_key_valid as a readiness flag while iasn could still be
stale/uninitialized was a data race (reported by Sashiko).
- Added a comment in mptcp_diag_fill_info() documenting why the
msk->first NULL fallback to 0 cannot be observed by any external
caller, given the shared lock_sock/lock_sock_fast serialization
with mptcp_close() (reported by Sashiko, confirmed with Matt).
- Documented in the commit message that this intentionally changes
the UAPI semantics of the three fields, per Matt's request
(reported by Sashiko as a compatibility concern).
Changes since v2:
- Dropped the msk->local_idsn/msk->remote_idsn fields entirely.
Read idsn/iasn from msk->first's subflow context in
mptcp_diag_fill_info() instead, per Matt's suggestion, to avoid
growing mptcp_sock for a diag-only need. Verified msk->first only
goes NULL as part of whole-msk teardown, not during a live
multi-subflow connection.
Changes since v1:
- Cached msk->remote_idsn before subflow->iasn++ instead of after:
the increment accounts for the peer's virtual SYN, and caching
remote_idsn post-increment left mptcpi_rcv_nxt starting at 0 while
mptcpi_write_seq/mptcpi_snd_una started at 1 for the same
connection (reported by Sashiko).
v1: https://lore.kernel.org/all/20260825113355.3573376-1-kalpan.jani@mpiricsoftware.com/
v2: https://lore.kernel.org/all/20260827041058.2833707-1-kalpan.jani@mpiricsoftware.com/
v3: https://lore.kernel.org/all/20260902102125.2035540-1-kalpan.jani@mpiricsoftware.com/
---
net/mptcp/sockopt.c | 27 ++++++++++++++++++++++++---
net/mptcp/subflow.c | 4 +++-
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 922f6ae5c80cb..b9df9aebf9e68 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1047,6 +1047,7 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
{
struct sock *sk = (struct sock *)msk;
+ u64 local_idsn = 0, remote_idsn = 0;
u32 flags = 0;
bool slow;
u32 now;
@@ -1084,9 +1085,29 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
info->mptcpi_flags = flags;
slow = lock_sock_fast(sk);
+ /* msk->first is only ever NULL once the whole msk is already in
+ * TCP_CLOSE (see __mptcp_close_ssk()); mptcp_close() holds the same
+ * sk lock this function acquires via lock_sock_fast(), so no caller
+ * can observe that transition mid-flight. If it does happen, the 0
+ * fallback below just leaves write_seq/snd_una/rcv_nxt unnormalized,
+ * which is harmless since the socket is already gone.
+ */
+
+ if (msk->first) {
+ struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(msk->first);
+
+ local_idsn = subflow->idsn;
+ /* subflow->iasn is incremented once in subflow_set_remote_key()
+ * to account for the peer's virtual SYN; undo that here so
+ * rcv_nxt normalizes against the same baseline write_seq and
+ * snd_una use.
+ */
+ remote_idsn = subflow->remote_key_valid ? subflow->iasn - 1 : 0;
+ }
+
info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
info->mptcpi_token = msk->token;
- info->mptcpi_write_seq = msk->write_seq;
+ info->mptcpi_write_seq = msk->write_seq - local_idsn;
info->mptcpi_retransmits = inet_csk(sk)->icsk_retransmits;
info->mptcpi_bytes_sent = msk->bytes_sent;
info->mptcpi_bytes_received = msk->bytes_received;
@@ -1100,8 +1121,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
mptcp_data_lock(sk);
info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
- info->mptcpi_snd_una = msk->snd_una;
- info->mptcpi_rcv_nxt = msk->ack_seq;
+ info->mptcpi_snd_una = msk->snd_una - local_idsn;
+ info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;
info->mptcpi_bytes_acked = msk->bytes_acked;
mptcp_data_unlock(sk);
}
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 2d7ccb01d2342..a3313a3db5a76 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -482,7 +482,6 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
if (subflow->remote_key_valid)
return;
- subflow->remote_key_valid = 1;
subflow->remote_key = mp_opt->sndr_key;
mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow->iasn);
subflow->iasn++;
@@ -494,6 +493,9 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
WRITE_ONCE(msk->ack_seq, subflow->iasn);
WRITE_ONCE(msk->can_ack, true);
atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
+
+ /* publish last, once iasn and the fields above are fully populated */
+ subflow->remote_key_valid = 1;
}
static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
--
2.43.0
Hi Kalpan,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf:
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/33721505735
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d7b6ca0c46b5
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1156691
If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:
$ cd [kernel source code]
$ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
--pull always mptcp/mptcp-upstream-virtme-docker:latest \
auto-normal
For more details:
https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)
Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Changing the semantics of UAPI fields `mptcpi_write_seq`, `mptcpi_snd_una`, and `mptcpi_rcv_nxt` from absolute MPTCP sequence numbers to relative zero-based offsets breaks userspace tools.
- [Medium] Unsynchronized access to the 64-bit `subflow->iasn` and bitfield `subflow->remote_key_valid` causes a data race, torn reads, and memory reordering bugs.
- [Medium] A cross-lock Time-of-Check to Time-of-Use (TOCTOU) race between reading `remote_key_valid` and `msk->ack_seq` causes the diagnostic tool to leak the raw, unnormalized sequence number.
--
commit b74b908d7d9e7114635fdc56f70e62e97e495292
Author: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
mptcp: normalize seq numbers reported in mptcp_info
This commit attempts to normalize sequence numbers reported in mptcp_info by
subtracting the initial sequence numbers from the raw values. It calculates
these offsets using the local and remote IDSN from the subflow context.
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index fcf6feb2a9eb5..5bcad54e909d1 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
[ ... ]
> @@ -1008,9 +1009,29 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
> info->mptcpi_flags = flags;
>
> slow = lock_sock_fast(sk);
> + /* msk->first is only ever NULL once the whole msk is already in
> + * TCP_CLOSE (see __mptcp_close_ssk()); mptcp_close() holds the same
> + * sk lock this function acquires via lock_sock_fast(), so no caller
> + * can observe that transition mid-flight. If it does happen, the 0
> + * fallback below just leaves write_seq/snd_una/rcv_nxt unnormalized,
> + * which is harmless since the socket is already gone.
> + */
> +
> + if (msk->first) {
> + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(msk->first);
> +
> + local_idsn = subflow->idsn;
> + /* subflow->iasn is incremented once in subflow_set_remote_key()
> + * to account for the peer's virtual SYN; undo that here so
> + * rcv_nxt normalizes against the same baseline write_seq and
> + * snd_una use.
> + */
> + remote_idsn = subflow->remote_key_valid ? subflow->iasn - 1 : 0;
[Severity: Medium]
Could this result in a data race? In mptcp_diag_fill_info(), we are only
holding the socket lock via lock_sock_fast(). However, the writer in
subflow_set_remote_key() holds only the data lock.
On 32-bit systems, a lockless read of the 64-bit subflow->iasn can tear.
Additionally, since there are no memory barriers (e.g. smp_load_acquire() and
smp_store_release()), the CPU could reorder the reads and observe
remote_key_valid == 1 while iasn is still uninitialized.
Could this happen when getsockopt(MPTCP_INFO) is called concurrently with the
softirq processing the remote key via mptcp_propagate_state()?
> + }
> +
> info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
> info->mptcpi_token = msk->token;
> - info->mptcpi_write_seq = msk->write_seq;
> + info->mptcpi_write_seq = msk->write_seq - local_idsn;
[Severity: Critical]
Does this regression break userspace ABI?
By changing the semantics of mptcpi_write_seq, mptcpi_snd_una, and
mptcpi_rcv_nxt from absolute sequence numbers to relative zero-based offsets,
existing tools relying on absolute sequence numbers (like tcpdump correlation)
might break.
Furthermore, because the kernel does not expose the local_idsn and remote_idsn
offsets in the mptcp_info struct, userspace has no way to reconstruct the
original absolute values.
> info->mptcpi_retransmits = inet_csk(sk)->icsk_retransmits;
> info->mptcpi_bytes_sent = msk->bytes_sent;
> info->mptcpi_bytes_received = msk->bytes_received;
[ ... ]
> @@ -1024,8 +1045,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>
> mptcp_data_lock(sk);
> info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
> - info->mptcpi_snd_una = msk->snd_una;
> - info->mptcpi_rcv_nxt = msk->ack_seq;
> + info->mptcpi_snd_una = msk->snd_una - local_idsn;
> + info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;
[Severity: Medium]
Could this regression leak the raw sequence number due to a Time-of-Check to
Time-of-Use race?
If mptcp_diag_fill_info() samples remote_key_valid as 0 earlier under
lock_sock_fast(), remote_idsn will be captured as 0. If
subflow_set_remote_key() then executes concurrently before we acquire the
data lock here, it updates msk->ack_seq to the absolute sequence number.
When we compute info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn here, it
will subtract 0 from the raw sequence number, returning the unnormalized
value and causing a discontinuity for userspace.
> info->mptcpi_bytes_acked = msk->bytes_acked;
> mptcp_data_unlock(sk);
> }
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index af81ad5e699d1..794c6dd9e4150 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
[ ... ]
> @@ -493,6 +492,9 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
> WRITE_ONCE(msk->ack_seq, subflow->iasn);
> WRITE_ONCE(msk->can_ack, true);
> atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
> +
> + /* publish last, once iasn and the fields above are fully populated */
> + subflow->remote_key_valid = 1;
[Severity: Medium]
As mentioned earlier in mptcp_diag_fill_info(), writing to this bitfield
under the data lock without memory barriers means that concurrent lockless
readers might observe remote_key_valid == 1 while iasn is still
uninitialized.
> }
>
> static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260903054853.3400545-1-kalpan.jani@mpiricsoftware.com?part=1
© 2016 - 2026 Red Hat, Inc.