Netdev List
 help / color / mirror / Atom feed
* [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present
@ 2026-06-17 21:57 Michael Bommarito
  2026-06-29  9:50 ` Paolo Abeni
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Bommarito @ 2026-06-17 21:57 UTC (permalink / raw)
  To: Matthieu Baerts, Mat Martineau
  Cc: Geliang Tang, Paolo Abeni, Eric Dumazet, Jakub Kicinski, mptcp,
	netdev, linux-kernel

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present
  2026-06-17 21:57 [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present Michael Bommarito
@ 2026-06-29  9:50 ` Paolo Abeni
  2026-06-29 11:00   ` Michael Bommarito
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2026-06-29  9:50 UTC (permalink / raw)
  To: Michael Bommarito, Matthieu Baerts, Mat Martineau
  Cc: Geliang Tang, Eric Dumazet, Jakub Kicinski, mptcp, netdev,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present
  2026-06-29  9:50 ` Paolo Abeni
@ 2026-06-29 11:00   ` Michael Bommarito
  2026-06-29 13:14     ` Paolo Abeni
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Bommarito @ 2026-06-29 11:00 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Eric Dumazet,
	Jakub Kicinski, mptcp, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present
  2026-06-29 11:00   ` Michael Bommarito
@ 2026-06-29 13:14     ` Paolo Abeni
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2026-06-29 13:14 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Eric Dumazet,
	Jakub Kicinski, mptcp, netdev, linux-kernel

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-29 13:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 21:57 [PATCH] mptcp: only honor zero-length DATA_FIN when a mapping is present Michael Bommarito
2026-06-29  9:50 ` Paolo Abeni
2026-06-29 11:00   ` Michael Bommarito
2026-06-29 13:14     ` Paolo Abeni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox