mptcp: only honor zero-length DATA_FIN when a mapping is present

[PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present

Posted by Michael Bommarito 2 weeks, 3 days ago

mptcp_get_options() initializes only the status group of struct
mptcp_options_received; data_seq, subflow_seq and data_len are set by
mptcp_parse_option() only inside the DSS mapping block, which runs when
the DSS M (mapping present) bit is set.

A peer can send a DSS option with DATA_FIN set but the mapping bit clear.
The parser then sets mp_opt.data_fin while leaving data_len and data_seq
uninitialized, and for a zero-length segment mptcp_incoming_options()
reads them; KMSAN reports an uninit-value in mptcp_incoming_options().

Impact: a remote peer that has completed the MPTCP handshake makes
mptcp_incoming_options() read uninitialized data_len and data_seq (KMSAN
uninit-value) by sending a DSS option with DATA_FIN set and the mapping
bit clear.

A DATA_FIN is always sent with a mapping (mptcp_write_data_fin()), so
gating this path on the mapping bit drops only the malformed no-map case
and leaves valid DATA_FIN handling unchanged.

Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
The stale data_seq then reaches mptcp_update_rcv_data_fin(); no further
consequence is demonstrated, so this is a robustness fix. The read site
for a zero-length segment is:

	if (mp_opt.data_fin && mp_opt.data_len == 1 &&
	    mptcp_update_rcv_data_fin(msk, mp_opt.data_seq, mp_opt.dsn64))

Reproduced under KMSAN: a no-map DSS DATA_FIN crafted on the wire and
injected over a TUN device into a real MPTCP listener produces twelve
"uninit-value in mptcp_incoming_options" reports on stock and none on the patched build (the unrelated mm-side
KMSAN boot noise present on both trees is not affected); a well-formed mapped DATA_FIN
(control) drives the same branch with no report. Harness on request.

 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..e40efa26a6694 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1227,7 +1227,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 	 * present, needs to be updated here before the skb is freed.
 	 */
 	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
-		if (mp_opt.data_fin && mp_opt.data_len == 1 &&
+		if (mp_opt.use_map && mp_opt.data_fin && mp_opt.data_len == 1 &&
 		    mptcp_update_rcv_data_fin(msk, mp_opt.data_seq, mp_opt.dsn64))
 			mptcp_schedule_work((struct sock *)msk);
 
-- 
2.53.0

Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present

Posted by Paolo Abeni 5 days, 19 hours ago

On 6/17/26 11:57 PM, Michael Bommarito wrote:
> mptcp_get_options() initializes only the status group of struct
> mptcp_options_received; data_seq, subflow_seq and data_len are set by
> mptcp_parse_option() only inside the DSS mapping block, which runs when
> the DSS M (mapping present) bit is set.
> 
> A peer can send a DSS option with DATA_FIN set but the mapping bit clear.
> The parser then sets mp_opt.data_fin while leaving data_len and data_seq
> uninitialized, and for a zero-length segment mptcp_incoming_options()
> reads them; KMSAN reports an uninit-value in mptcp_incoming_options().
> 
> Impact: a remote peer that has completed the MPTCP handshake makes
> mptcp_incoming_options() read uninitialized data_len and data_seq (KMSAN
> uninit-value) by sending a DSS option with DATA_FIN set and the mapping
> bit clear.
> 
> A DATA_FIN is always sent with a mapping (mptcp_write_data_fin()), so
> gating this path on the mapping bit drops only the malformed no-map case
> and leaves valid DATA_FIN handling unchanged.
> 
> Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>

Isn't this fixed by commit 5e939544f9d2 ("mptcp: fix uninit-value in
mptcp_established_options") ?

/P

Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present

Posted by Michael Bommarito 5 days, 17 hours ago

On Mon, Jun 29, 2026 at 5:50 AM Paolo Abeni <pabeni@redhat.com> wrote:
> Isn't this fixed by commit 5e939544f9d2 ("mptcp: fix uninit-value in
> mptcp_established_options") ?

I did the reproduction ~10 days ago on linus's latest, so definitely
still reproducing.  I think 5e939544f9d2 was on the TX side and this
is about the RX option path, so they don't overlap on flows either.

Thanks,
Mike

Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present

Posted by Paolo Abeni 5 days, 15 hours ago

On 6/29/26 1:00 PM, Michael Bommarito wrote:
> On Mon, Jun 29, 2026 at 5:50 AM Paolo Abeni <pabeni@redhat.com> wrote:
>> Isn't this fixed by commit 5e939544f9d2 ("mptcp: fix uninit-value in
>> mptcp_established_options") ?
> 
> I did the reproduction ~10 days ago on linus's latest, so definitely
> still reproducing.  I think 5e939544f9d2 was on the TX side and this
> is about the RX option path, so they don't overlap on flows either.


Right.

AFAICS the RFC is a little vague about enforcing meaningful flags
combination.

I think it would be better to avoid another conditional while processing
incoming ack. Does the following solve the issue? 

Thanks!

---
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 0ca60314d667..b924209a9b74 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

Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present

Posted by MPTCP CI 2 weeks, 3 days ago

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/27723068868

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/41f92d8643fc
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1113137


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)