[PATCH v2 mptcp] mptcp: Clear mptcp_out_options in mptcp_{syn,synack,established}_options().

Kuniyuki Iwashima posted 1 patch 1 day, 5 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20260504044051.3725846-1-kuniyu@google.com
net/mptcp/options.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH v2 mptcp] mptcp: Clear mptcp_out_options in mptcp_{syn,synack,established}_options().
Posted by Kuniyuki Iwashima 1 day, 5 hours ago
syzbot reported read of uninit-value in mptcp_write_data_fin().

Since the cited commit, struct mptcp_out_options is not cleared
in __tcp_transmit_skb().

Let's clear it in mptcp_{syn,synack,established}_options().

Another option is to move struct mptcp_out_options to struct
tcp_out_options.cleared, but this memset() is not needed for TCP.

[0]:
BUG: KMSAN: uninit-value in mptcp_write_data_fin net/mptcp/options.c:542 [inline]
BUG: KMSAN: uninit-value in mptcp_established_options_dss net/mptcp/options.c:590 [inline]
BUG: KMSAN: uninit-value in mptcp_established_options+0x112f/0x3530 net/mptcp/options.c:874
 mptcp_write_data_fin net/mptcp/options.c:542 [inline]
 mptcp_established_options_dss net/mptcp/options.c:590 [inline]
 mptcp_established_options+0x112f/0x3530 net/mptcp/options.c:874
 tcp_established_options+0x312/0xcc0 net/ipv4/tcp_output.c:1192
 __tcp_transmit_skb+0x5dc/0x5fe0 net/ipv4/tcp_output.c:1575
 __tcp_send_ack+0x967/0xad0 net/ipv4/tcp_output.c:4499
 tcp_send_ack+0x3d/0x60 net/ipv4/tcp_output.c:4505
 mptcp_subflow_shutdown+0x164/0x690 net/mptcp/protocol.c:3137
 mptcp_check_send_data_fin+0x31b/0x3d0 net/mptcp/protocol.c:3218
 __mptcp_wr_shutdown net/mptcp/protocol.c:3234 [inline]
 __mptcp_close+0x860/0x1360 net/mptcp/protocol.c:3313
 mptcp_close+0x42/0x260 net/mptcp/protocol.c:3367
 inet_release+0x1ee/0x2a0 net/ipv4/af_inet.c:442
 __sock_release net/socket.c:722 [inline]
 sock_close+0xd6/0x2f0 net/socket.c:1514
 __fput+0x60e/0x1010 fs/file_table.c:510
 ____fput+0x25/0x30 fs/file_table.c:538
 task_work_run+0x208/0x2b0 kernel/task_work.c:233
 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
 __exit_to_user_mode_loop kernel/entry/common.c:67 [inline]
 exit_to_user_mode_loop+0x306/0x1b60 kernel/entry/common.c:98
 __exit_to_user_mode_prepare include/linux/irq-entry-common.h:207 [inline]
 syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:238 [inline]
 syscall_exit_to_user_mode include/linux/entry-common.h:318 [inline]
 __do_fast_syscall_32+0x2c7/0x460 arch/x86/entry/syscall_32.c:310
 do_fast_syscall_32+0x37/0x80 arch/x86/entry/syscall_32.c:332
 do_SYSENTER_32+0x1f/0x30 arch/x86/entry/syscall_32.c:370
 entry_SYSENTER_compat_after_hwframe+0x84/0x8e

Local variable opts created at:
 __tcp_transmit_skb+0x4d/0x5fe0 net/ipv4/tcp_output.c:1536
 __tcp_send_ack+0x967/0xad0 net/ipv4/tcp_output.c:4499

CPU: 1 UID: 0 PID: 5855 Comm: syz.3.4 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026

Fixes: cfcceb7a39fc ("tcp: shrink per-packet memset in __tcp_transmit_skb()")
Reported-by: syzbot+ff020673c5e3d94d9478@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/mptcp/69f44505.050a0220.3cbe47.0008.GAE@google.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v2: Clear the entire struct mptcp_out_options
v1: https://lore.kernel.org/mptcp/20260501061939.1808489-1-kuniyu@google.com/
---
 net/mptcp/options.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 8a1c5698983c..02253c1a9f57 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -403,6 +403,8 @@ bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
 
+	memset(opts, 0, sizeof(*opts));
+
 	/* we will use snd_isn to detect first pkt [re]transmission
 	 * in mptcp_established_options_mp()
 	 */
@@ -846,7 +848,7 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
 	bool snd_data_fin;
 	bool ret = false;
 
-	opts->suboptions = 0;
+	memset(opts, 0, sizeof(*opts));
 
 	/* Force later mptcp_write_options(), but do not use any actual
 	 * option space.
@@ -915,6 +917,8 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 {
 	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
 
+	memset(opts, 0, sizeof(*opts));
+
 	if (subflow_req->mp_capable) {
 		opts->suboptions = OPTION_MPTCP_MPC_SYNACK;
 		opts->sndr_key = subflow_req->local_key;
-- 
2.54.0.545.g6539524ca2-goog
Re: [PATCH v2 mptcp] mptcp: Clear mptcp_out_options in mptcp_{syn,synack,established}_options().
Posted by Paolo Abeni 1 day, 1 hour ago
Hi,

On 5/4/26 6:40 AM, Kuniyuki Iwashima wrote:
> syzbot reported read of uninit-value in mptcp_write_data_fin().
> 
> Since the cited commit, struct mptcp_out_options is not cleared
> in __tcp_transmit_skb().
> 
> Let's clear it in mptcp_{syn,synack,established}_options().
> 
> Another option is to move struct mptcp_out_options to struct
> tcp_out_options.cleared, but this memset() is not needed for TCP.
> 
> [0]:
> BUG: KMSAN: uninit-value in mptcp_write_data_fin net/mptcp/options.c:542 [inline]
> BUG: KMSAN: uninit-value in mptcp_established_options_dss net/mptcp/options.c:590 [inline]
> BUG: KMSAN: uninit-value in mptcp_established_options+0x112f/0x3530 net/mptcp/options.c:874
>  mptcp_write_data_fin net/mptcp/options.c:542 [inline]
>  mptcp_established_options_dss net/mptcp/options.c:590 [inline]
>  mptcp_established_options+0x112f/0x3530 net/mptcp/options.c:874
>  tcp_established_options+0x312/0xcc0 net/ipv4/tcp_output.c:1192
>  __tcp_transmit_skb+0x5dc/0x5fe0 net/ipv4/tcp_output.c:1575
>  __tcp_send_ack+0x967/0xad0 net/ipv4/tcp_output.c:4499
>  tcp_send_ack+0x3d/0x60 net/ipv4/tcp_output.c:4505
>  mptcp_subflow_shutdown+0x164/0x690 net/mptcp/protocol.c:3137
>  mptcp_check_send_data_fin+0x31b/0x3d0 net/mptcp/protocol.c:3218
>  __mptcp_wr_shutdown net/mptcp/protocol.c:3234 [inline]
>  __mptcp_close+0x860/0x1360 net/mptcp/protocol.c:3313
>  mptcp_close+0x42/0x260 net/mptcp/protocol.c:3367
>  inet_release+0x1ee/0x2a0 net/ipv4/af_inet.c:442
>  __sock_release net/socket.c:722 [inline]
>  sock_close+0xd6/0x2f0 net/socket.c:1514
>  __fput+0x60e/0x1010 fs/file_table.c:510
>  ____fput+0x25/0x30 fs/file_table.c:538
>  task_work_run+0x208/0x2b0 kernel/task_work.c:233
>  resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
>  __exit_to_user_mode_loop kernel/entry/common.c:67 [inline]
>  exit_to_user_mode_loop+0x306/0x1b60 kernel/entry/common.c:98
>  __exit_to_user_mode_prepare include/linux/irq-entry-common.h:207 [inline]
>  syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:238 [inline]
>  syscall_exit_to_user_mode include/linux/entry-common.h:318 [inline]
>  __do_fast_syscall_32+0x2c7/0x460 arch/x86/entry/syscall_32.c:310
>  do_fast_syscall_32+0x37/0x80 arch/x86/entry/syscall_32.c:332
>  do_SYSENTER_32+0x1f/0x30 arch/x86/entry/syscall_32.c:370
>  entry_SYSENTER_compat_after_hwframe+0x84/0x8e
> 
> Local variable opts created at:
>  __tcp_transmit_skb+0x4d/0x5fe0 net/ipv4/tcp_output.c:1536
>  __tcp_send_ack+0x967/0xad0 net/ipv4/tcp_output.c:4499
> 
> CPU: 1 UID: 0 PID: 5855 Comm: syz.3.4 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
> 
> Fixes: cfcceb7a39fc ("tcp: shrink per-packet memset in __tcp_transmit_skb()")
> Reported-by: syzbot+ff020673c5e3d94d9478@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/mptcp/69f44505.050a0220.3cbe47.0008.GAE@google.com/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> v2: Clear the entire struct mptcp_out_options
> v1: https://lore.kernel.org/mptcp/20260501061939.1808489-1-kuniyu@google.com/
> ---
>  net/mptcp/options.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 8a1c5698983c..02253c1a9f57 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -403,6 +403,8 @@ bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
>  {
>  	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
>  
> +	memset(opts, 0, sizeof(*opts));

I'm sorry for the latency. I think this is a bit overkill. Back than we
tried to avoid memsetting the whole opts, and it should be doable to
avoid it - at least for most packets.

AFAICS the in the relevant code path, the egress skb has no extensions.
Something alike the following (completely untested) should be sufficient:
---
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index f816c26cbd14..63bbac884f18 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -583,6 +583,8 @@ static bool mptcp_established_options_dss(struct
sock *sk, struct sk_buff *skb,
 				map_size += TCPOLEN_MPTCP_DSS_CHECKSUM;

 			opts->ext_copy = *mpext;
+		} else {
+			opts->ext_copy.use_map = 0;
 		}

 		dss_size = map_size;
---

BTW I'm not sure that the sashiko report on V1 is relevant/true/correct:
if such statement would be correct, syzbot would have reported the KMSAN
splat on the bitfield set operation, which happens earlier in the code
path WRT the actual syzbot splat and is hit on every packet.

/P
Re: [PATCH v2 mptcp] mptcp: Clear mptcp_out_options in mptcp_{syn,synack,established}_options().
Posted by Matthieu Baerts 17 hours ago
Hi Paolo, Kuniyuki,

On 04/05/2026 11:12, Paolo Abeni wrote:
> On 5/4/26 6:40 AM, Kuniyuki Iwashima wrote:
>> syzbot reported read of uninit-value in mptcp_write_data_fin().
>>
>> Since the cited commit, struct mptcp_out_options is not cleared
>> in __tcp_transmit_skb().

(...)

>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 8a1c5698983c..02253c1a9f57 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
>> @@ -403,6 +403,8 @@ bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
>>  {
>>  	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
>>  
>> +	memset(opts, 0, sizeof(*opts));
> 
> I'm sorry for the latency. I think this is a bit overkill. Back than we
> tried to avoid memsetting the whole opts, and it should be doable to
> avoid it - at least for most packets.
> 
> AFAICS the in the relevant code path, the egress skb has no extensions.
> Something alike the following (completely untested) should be sufficient:
> ---
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index f816c26cbd14..63bbac884f18 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -583,6 +583,8 @@ static bool mptcp_established_options_dss(struct
> sock *sk, struct sk_buff *skb,
>  				map_size += TCPOLEN_MPTCP_DSS_CHECKSUM;
> 
>  			opts->ext_copy = *mpext;
> +		} else {
> +			opts->ext_copy.use_map = 0;
>  		}
> 
>  		dss_size = map_size;
> ---

Thank you for the v2 and the review!

I guess you noticed, but I asked syzbot to validate this patch, and it
looks like it is not enough:

  https://lore.kernel.org/69f87f79.050a0220.3460d5.0002.GAE@google.com

Please note that the calltrace is different: now in mptcp_write_options.
What's strange is that "if (mpext->use_ack)" didn't cause a warning, but
"if (mpext->use_map)" did. Both "use_ack" and "use_map" are from the
same u8 bitfield.

https://github.com/torvalds/linux/blob/6d35786de28116ecf78797a62b84e6bf3c45aa5a/net/mptcp/options.c#L1450-L1460

> BTW I'm not sure that the sashiko report on V1 is relevant/true/correct:
> if such statement would be correct, syzbot would have reported the KMSAN
> splat on the bitfield set operation, which happens earlier in the code
> path WRT the actual syzbot splat and is hit on every packet.
TBH, after having re-read KMSAN doc [1] I'm not sure how it deals with
bitfields, but I guess it could only mark it as initialised when the
whole byte is initialised. Maybe the second approach I suggested in v1
could work?

 → using two new unions to clear these bitfields with
'opts->cleared = 0' from mptcp_{syn,synack,established}_options() and
'opts->ext_copy.cleared = 0' from mptcp_established_options_dss()?

But it looks a bit too hacky.

[1] https://docs.kernel.org/dev-tools/kmsan.html

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v2 mptcp] mptcp: Clear mptcp_out_options in mptcp_{syn,synack,established}_options().
Posted by MPTCP CI 1 day, 4 hours ago
Hi Kuniyuki,

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): Unstable: 1 failed test(s): packetdrill_dss ⚠️ 
- 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/25302049625

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


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)