The skb cb offset containing the timestamp presence flag is cleared
before loading such information. Cache such value before MPTCP CB
initialization.
Fixes: 36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/fastopen.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index 82ec15bcfd7f..082c46c0f50e 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -12,6 +12,7 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
struct sock *sk, *ssk;
struct sk_buff *skb;
struct tcp_sock *tp;
+ bool has_rxtstamp;
/* on early fallback the subflow context is deleted by
* subflow_syn_recv_sock()
@@ -40,12 +41,13 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
*/
tp->copied_seq += skb->len;
subflow->ssn_offset += skb->len;
+ has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
/* Only the sequence delta is relevant */
MPTCP_SKB_CB(skb)->map_seq = -skb->len;
MPTCP_SKB_CB(skb)->end_seq = 0;
MPTCP_SKB_CB(skb)->offset = 0;
- MPTCP_SKB_CB(skb)->has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
+ MPTCP_SKB_CB(skb)->has_rxtstamp = has_rxtstamp;
MPTCP_SKB_CB(skb)->cant_coalesce = 1;
mptcp_data_lock(sk);
--
2.53.0
Hi Paolo, On 29/04/2026 13:06, Paolo Abeni wrote: > The skb cb offset containing the timestamp presence flag is cleared > before loading such information. Cache such value before MPTCP CB > initialization. Now in our tree: New patches for t/upstream-net and t/upstream: - 11220629c0b6: mptcp: fix rx timestamp corruption on fastopen - Results: 74d088bb4493..53245e9a2f63 (export-net) - Results: b64555038d9a..fb282c0c802d (export) Tests are now in progress: - export-net: https://github.com/multipath-tcp/mptcp_net-next/commit/19acdc9893d0404966344913456f96a33ee235ae/checks - export: https://github.com/multipath-tcp/mptcp_net-next/commit/a9a1aa04308af2b72dae77e1cb6382a079e4776c/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
On Wed, 29 Apr 2026 13:06:02 +0200, Paolo Abeni <pabeni@redhat.com> wrote: > mptcp: fix rx timestamp corruption on fastopen Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> -- Matthieu Baerts (NGI0) <matttbe@kernel.org>
On Wed, 29 Apr 2026 13:06:02 +0200, Paolo Abeni <pabeni@redhat.com> wrote: > The skb cb offset containing the timestamp presence flag is cleared > before loading such information. Cache such value before MPTCP CB > initialization. Good catch, thank you for the patch! > > > diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c > index 82ec15bcfd7f..082c46c0f50e 100644 > --- a/net/mptcp/fastopen.c > +++ b/net/mptcp/fastopen.c > @@ -40,12 +41,13 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf > */ > tp->copied_seq += skb->len; > subflow->ssn_offset += skb->len; > + has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp; > > /* Only the sequence delta is relevant */ > MPTCP_SKB_CB(skb)->map_seq = -skb->len; From sashiko: Does this assignment result in a zero-extension issue? While not introduced by this patch, since skb->len is an unsigned int, applying the unary - operator results in a large positive unsigned int rather than a negative value. When this is assigned to map_seq, which is a u64, it gets zero-extended instead of sign-extended. Downstream in mptcp_inq_hint(), the calculation: READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq could underflow if msk->ack_seq is 0, resulting in a massive 64-bit value. Could this trigger a fallback where ioctl(FIONREAD) falsely reports INT_MAX bytes available? If a peer sends a TCP Fast Open SYN packet with a payload, server applications querying FIONREAD might attempt a 2GB allocation and crash. Should this be explicitly cast, such as -(u64)skb->len, before negation? It looks like it didn't get that map_seq will be += skb->len later. -- Matthieu Baerts (NGI0) <matttbe@kernel.org>
Hi Paolo, On 30/04/2026 16:29, Matthieu Baerts wrote: > On Wed, 29 Apr 2026 13:06:02 +0200, Paolo Abeni <pabeni@redhat.com> wrote: >> The skb cb offset containing the timestamp presence flag is cleared >> before loading such information. Cache such value before MPTCP CB >> initialization. > > Good catch, thank you for the patch! > >> >> >> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c >> index 82ec15bcfd7f..082c46c0f50e 100644 >> --- a/net/mptcp/fastopen.c >> +++ b/net/mptcp/fastopen.c >> @@ -40,12 +41,13 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf >> */ >> tp->copied_seq += skb->len; >> subflow->ssn_offset += skb->len; >> + has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp; >> >> /* Only the sequence delta is relevant */ >> MPTCP_SKB_CB(skb)->map_seq = -skb->len; > > From sashiko: > > Does this assignment result in a zero-extension issue? > > While not introduced by this patch, since skb->len is an unsigned int, > applying the unary - operator results in a large positive unsigned int rather > than a negative value. > > When this is assigned to map_seq, which is a u64, it gets zero-extended > instead of sign-extended. Downstream in mptcp_inq_hint(), the calculation: > > READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq > > could underflow if msk->ack_seq is 0, resulting in a massive 64-bit value. > > Could this trigger a fallback where ioctl(FIONREAD) falsely reports INT_MAX > bytes available? > > If a peer sends a TCP Fast Open SYN packet with a payload, server > applications querying FIONREAD might attempt a 2GB allocation and crash. > > Should this be explicitly cast, such as -(u64)skb->len, before negation? > > It looks like it didn't get that map_seq will be += skb->len later. Wrong explanation: if it is += skb->len later, it will not be back to 0 without the cast. @Paolo: do you remember why it is set to -(u32)skb->len here? Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Paolo,
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): Unstable: 1 failed test(s): packetdrill_dss ⚠️
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/25106678391
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/cebd25803b46
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1087424
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)
© 2016 - 2026 Red Hat, Inc.