[PATCH v2] mptcp: only set DATA_FIN when a mapping is present

[PATCH v2] mptcp: only set DATA_FIN when a mapping is present

[Michael Bommarito]

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.0

Re: [PATCH v2] mptcp: only set DATA_FIN when a mapping is present

[gang.yan]

July 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
>

Re: [PATCH v2] mptcp: only set DATA_FIN when a mapping is present

[Matthieu Baerts]

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.

Re: [PATCH v2] mptcp: only set DATA_FIN when a mapping is present

[Michael Bommarito]

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

Re: [PATCH v2] mptcp: only set DATA_FIN when a mapping is present

[Matthieu Baerts]

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.