* [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