net/mptcp/options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mptcp_get_options() clears only the status group of struct
mptcp_options_received; data_seq, subflow_seq and data_len are filled in
by mptcp_parse_option() exclusively inside the DSS mapping block, which
runs only when the DSS M (mapping present) bit is set.
A peer can send a DSS option with the DATA_FIN flag set but the mapping
bit clear. The parser then records mp_opt->data_fin while leaving
data_len and data_seq uninitialized. For a zero-length segment
mptcp_incoming_options() evaluates
if (mp_opt.data_fin && mp_opt.data_len == 1 &&
mptcp_update_rcv_data_fin(msk, mp_opt.data_seq, mp_opt.dsn64))
which reads the uninitialized data_len and data_seq; KMSAN reports an
uninit-value in mptcp_incoming_options(). The stale data_seq can also be
fed into the receive-side DATA_FIN sequence tracking.
Record the DATA_FIN flag only when the DSS option carries a mapping, so
data_fin is never set without data_seq and data_len also being present.
data_fin is part of the status group that mptcp_get_options() clears up
front, so on the no-map path it stays zero and the zero-length DATA_FIN
branch is simply skipped. A DATA_FIN is always transmitted together with
a mapping (mptcp_write_data_fin() sets use_map along with data_seq and
data_len), so legitimate DATA_FIN handling is unaffected.
Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2: adopt Paolo Abeni's suggested approach - do not set mp_opt->data_fin
at all unless a mapping is present, rather than gating the consumer in
mptcp_incoming_options() (v1). data_fin then defaults to the value
mptcp_get_options() already clears it to (0) on the no-map path, so
the uninitialized data_len/data_seq are never read.
net/mptcp/options.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index dff3fd5d3b559..6d003b24b969f 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -157,7 +157,6 @@ static void mptcp_parse_option(const struct sk_buff *skb,
ptr++;
flags = (*ptr++) & MPTCP_DSS_FLAG_MASK;
- mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
mp_opt->dsn64 = (flags & MPTCP_DSS_DSN64) != 0;
mp_opt->use_map = (flags & MPTCP_DSS_HAS_MAP) != 0;
mp_opt->ack64 = (flags & MPTCP_DSS_ACK64) != 0;
@@ -178,6 +177,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
}
if (mp_opt->use_map) {
+ mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
if (mp_opt->dsn64)
expected_opsize += TCPOLEN_MPTCP_DSS_MAP64;
else
--
2.53.0July 8, 2026 at 1:17 AM, "Michael Bommarito" <michael.bommarito@gmail.com mailto:michael.bommarito@gmail.com?to=%22Michael%20Bommarito%22%20%3Cmichael.bommarito%40gmail.com%3E > wrote:
Hi Michael
Thanks for the patch.
>
> mptcp_get_options() clears only the status group of struct
> mptcp_options_received; data_seq, subflow_seq and data_len are filled in
> by mptcp_parse_option() exclusively inside the DSS mapping block, which
> runs only when the DSS M (mapping present) bit is set.
>
> A peer can send a DSS option with the DATA_FIN flag set but the mapping
> bit clear. The parser then records mp_opt->data_fin while leaving
> data_len and data_seq uninitialized. For a zero-length segment
> mptcp_incoming_options() evaluates
>
> if (mp_opt.data_fin && mp_opt.data_len == 1 &&
> mptcp_update_rcv_data_fin(msk, mp_opt.data_seq, mp_opt.dsn64))
>
> which reads the uninitialized data_len and data_seq; KMSAN reports an
> uninit-value in mptcp_incoming_options(). The stale data_seq can also be
> fed into the receive-side DATA_FIN sequence tracking.
>
> Record the DATA_FIN flag only when the DSS option carries a mapping, so
> data_fin is never set without data_seq and data_len also being present.
> data_fin is part of the status group that mptcp_get_options() clears up
> front, so on the no-map path it stays zero and the zero-length DATA_FIN
> branch is simply skipped. A DATA_FIN is always transmitted together with
> a mapping (mptcp_write_data_fin() sets use_map along with data_seq and
> data_len), so legitimate DATA_FIN handling is unaffected.
>
> Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> v2: adopt Paolo Abeni's suggested approach - do not set mp_opt->data_fin
> at all unless a mapping is present, rather than gating the consumer in
> mptcp_incoming_options() (v1). data_fin then defaults to the value
> mptcp_get_options() already clears it to (0) on the no-map path, so
> the uninitialized data_len/data_seq are never read.
>
> net/mptcp/options.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index dff3fd5d3b559..6d003b24b969f 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -157,7 +157,6 @@ static void mptcp_parse_option(const struct sk_buff *skb,
> ptr++;
>
> flags = (*ptr++) & MPTCP_DSS_FLAG_MASK;
> - mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
> mp_opt->dsn64 = (flags & MPTCP_DSS_DSN64) != 0;
> mp_opt->use_map = (flags & MPTCP_DSS_HAS_MAP) != 0;
> mp_opt->ack64 = (flags & MPTCP_DSS_ACK64) != 0;
> @@ -178,6 +177,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
> }
>
> if (mp_opt->use_map) {
> + mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
> if (mp_opt->dsn64)
> expected_opsize += TCPOLEN_MPTCP_DSS_MAP64;
> else
One minor thing I noticed while reviewing: Sashiko previously pointed out
in a comment that the 'pr_debug' logging 'mp_opt->data_fin' might be
problematic because it's printed before data_fin is actually assigned
(since data_fin is now set only inside the if (mp_opt->use_map) block).
I think it would be cleaner to move the pr_debug call after the data_fin
assignment, so the logged value is always meaningful and consistent with
the actual parsed state, like:
'''
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index b924209a9b74..f58eb24f4d35 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -162,11 +162,6 @@ static void mptcp_parse_option(const struct sk_buff *skb,
mp_opt->ack64 = (flags & MPTCP_DSS_ACK64) != 0;
mp_opt->use_ack = (flags & MPTCP_DSS_HAS_ACK);
- pr_debug("data_fin=%d dsn64=%d use_map=%d ack64=%d use_ack=%d\n",
- mp_opt->data_fin, mp_opt->dsn64,
- mp_opt->use_map, mp_opt->ack64,
- mp_opt->use_ack);
-
expected_opsize = TCPOLEN_MPTCP_DSS_BASE;
if (mp_opt->use_ack) {
@@ -184,6 +179,11 @@ static void mptcp_parse_option(const struct sk_buff *skb,
expected_opsize += TCPOLEN_MPTCP_DSS_MAP32;
}
+ pr_debug("data_fin=%d dsn64=%d use_map=%d ack64=%d use_ack=%d\n",
+ mp_opt->data_fin, mp_opt->dsn64,
+ mp_opt->use_map, mp_opt->ack64,
+ mp_opt->use_ack);
+
/* Always parse any csum presence combination, we will enforce
* RFC 8684 Section 3.3.0 checks later in subflow_data_ready
*/
'''
This change isn't critical to the fix itself, so feel free to let Paolo/Matt and
other Maintainers decide if it should go in now, later, or not at all.
A couple of additional notes for future submissions (just friendly reminders):
- The subject-prefix convention for MPTCP patches is typically [PATCH mptcp-net]
for bug fixes targeting the net tree, or [PATCH mptcp-next] for new features.
Since this is a fix, something like [PATCH mptcp-net v2] would be more appropriate.
- It's also helpful to include a Link: to the previous version (v1) in the changelog,
so reviewers can easily track the discussion history.
Looking forward to seeing more of your work on the MPTCP mailing list!
Thanks,
Gang
> --
> 2.53.0
>
Hi Michael, Gang,
On 09/07/2026 07:43, gang.yan@linux.dev wrote:
> July 8, 2026 at 1:17 AM, "Michael Bommarito" <michael.bommarito@gmail.com> wrote:
Thank you for this patch and the review!
>> mptcp_get_options() clears only the status group of struct
>> mptcp_options_received; data_seq, subflow_seq and data_len are filled in
>> by mptcp_parse_option() exclusively inside the DSS mapping block, which
>> runs only when the DSS M (mapping present) bit is set.
>>
>> A peer can send a DSS option with the DATA_FIN flag set but the mapping
>> bit clear. The parser then records mp_opt->data_fin while leaving
>> data_len and data_seq uninitialized. For a zero-length segment
>> mptcp_incoming_options() evaluates
By chance, do you have a reproducer? Maybe using Packetdrill with MPTCP
support? [1]
If not, how did you discover this issue?
[1] https://github.com/multipath-tcp/packetdrill/
>> if (mp_opt.data_fin && mp_opt.data_len == 1 &&
>> mptcp_update_rcv_data_fin(msk, mp_opt.data_seq, mp_opt.dsn64))
>>
>> which reads the uninitialized data_len and data_seq; KMSAN reports an
>> uninit-value in mptcp_incoming_options(). The stale data_seq can also be
>> fed into the receive-side DATA_FIN sequence tracking.
>>
>> Record the DATA_FIN flag only when the DSS option carries a mapping, so
>> data_fin is never set without data_seq and data_len also being present.
>> data_fin is part of the status group that mptcp_get_options() clears up
>> front, so on the no-map path it stays zero and the zero-length DATA_FIN
>> branch is simply skipped. A DATA_FIN is always transmitted together with
>> a mapping (mptcp_write_data_fin() sets use_map along with data_seq and
>> data_len), so legitimate DATA_FIN handling is unaffected.
>>
>> Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
>> Suggested-by: Paolo Abeni <pabeni@redhat.com>
>> Cc: stable@vger.kernel.org
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
>> ---
>> v2: adopt Paolo Abeni's suggested approach - do not set mp_opt->data_fin
>> at all unless a mapping is present, rather than gating the consumer in
>> mptcp_incoming_options() (v1). data_fin then defaults to the value
>> mptcp_get_options() already clears it to (0) on the no-map path, so
>> the uninitialized data_len/data_seq are never read.
>>
>> net/mptcp/options.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index dff3fd5d3b559..6d003b24b969f 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
>> @@ -157,7 +157,6 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>> ptr++;
>>
>> flags = (*ptr++) & MPTCP_DSS_FLAG_MASK;
>> - mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
>> mp_opt->dsn64 = (flags & MPTCP_DSS_DSN64) != 0;
>> mp_opt->use_map = (flags & MPTCP_DSS_HAS_MAP) != 0;
>> mp_opt->ack64 = (flags & MPTCP_DSS_ACK64) != 0;
>> @@ -178,6 +177,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>> }
>>
>> if (mp_opt->use_map) {
>> + mp_opt->data_fin = (flags & MPTCP_DSS_DATA_FIN) != 0;
>> if (mp_opt->dsn64)
>> expected_opsize += TCPOLEN_MPTCP_DSS_MAP64;
>> else
>
> One minor thing I noticed while reviewing: Sashiko previously pointed out
> in a comment that the 'pr_debug' logging 'mp_opt->data_fin' might be
> problematic because it's printed before data_fin is actually assigned
> (since data_fin is now set only inside the if (mp_opt->use_map) block).
>
> I think it would be cleaner to move the pr_debug call after the data_fin
> assignment, so the logged value is always meaningful and consistent with
> the actual parsed state, like:
(...)
> This change isn't critical to the fix itself, so feel free to let Paolo/Matt and
> other Maintainers decide if it should go in now, later, or not at all.
Probably best to fix that now, to avoid getting confused when looking at
debug info. Do you mind doing that in a v3, please?
pw-bot: changes-requested
> A couple of additional notes for future submissions (just friendly reminders):
>
> - The subject-prefix convention for MPTCP patches is typically [PATCH mptcp-net]
> for bug fixes targeting the net tree, or [PATCH mptcp-next] for new features.
> Since this is a fix, something like [PATCH mptcp-net v2] would be more appropriate.
For a "one-off" or urgent fix for MPTCP, that's OK to send patches to
the netdev ML directly. In this case, please use the "net" prefix, see:
https://docs.kernel.org/process/maintainer-netdev.html
For other kind of patches specific to MPTCP, please send them only to
the MPTCP ML with the 'mptcp-net' or 'mptcp-next' prefixes. Just to be
clear: here you can send the v3 to the same people and lists -- you
missed Simon apparently, did you not use get_maintainer.pl script? --
with the 'net' prefix ([PATCH net v3]).
> - It's also helpful to include a Link: to the previous version (v1) in the changelog,
> so reviewers can easily track the discussion history.
If I may add, please also send new version in a separate thread, not as
a reply, not to confuse various tools, as mentioned in the same doc.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
On Thu, Jul 9, 2026 at 8:27 AM Matthieu Baerts <matttbe@kernel.org> wrote: > By chance, do you have a reproducer? Maybe using Packetdrill with MPTCP > support? [1] > If not, how did you discover this issue? Yup, I'll send to you offlist. It's a custom Python/TUN/qemu setup, not Packetdrill, but I'm guessing it could be ported or extended for testing/fuzzing. I discovered it with my homegrown agentic stack, which is a Rube Goldberg machine full of various tools (like cocci, semgrep) and knowledge graphs (built with libclang). The agent logs suggest that it came from a family of rules that surfaces suspicious struct init candidates, which I can share too. > If I may add, please also send new version in a separate thread, not as > a reply, not to confuse various tools, as mentioned in the same doc. Sure, thanks to both you and Gang for the feedback. I'll send a v3 based on your suggestions later today. Thanks, Mike
On 09/07/2026 15:12, Michael Bommarito wrote: > On Thu, Jul 9, 2026 at 8:27 AM Matthieu Baerts <matttbe@kernel.org> wrote: > >> By chance, do you have a reproducer? Maybe using Packetdrill with MPTCP >> support? [1] >> If not, how did you discover this issue? > > Yup, I'll send to you offlist. It's a custom Python/TUN/qemu setup, > not Packetdrill, but I'm guessing it could be ported or extended for > testing/fuzzing. Interesting. Yes, it could be useful for various protocols. > I discovered it with my homegrown agentic stack, which is a Rube > Goldberg machine full of various tools (like cocci, semgrep) and > knowledge graphs (built with libclang). The agent logs suggest that > it came from a family of rules that surfaces suspicious struct init > candidates, which I can share too. Is it also based on crafted traffic? It could also be useful! >> If I may add, please also send new version in a separate thread, not as >> a reply, not to confuse various tools, as mentioned in the same doc. > > Sure, thanks to both you and Gang for the feedback. I'll send a v3 > based on your suggestions later today. Thanks! Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Michael,
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! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/28886089429
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/cdeb2105da80
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1123264
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.