From: Paolo Abeni <[email protected]>
syzbot reported the following uninit splat:
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
The output path currently omits initializing the mptcp extension
`use_map` flag in a few corner cases.
Address the issue always zeroing all the extensions flags before
eventually initializing the individual bits. To that extent, introduce
and use a struct_group to avoid multiple bitwise operations.
Fixes: cfcceb7a39fc ("tcp: shrink per-packet memset in __tcp_transmit_skb()")
Cc: [email protected]
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=ff020673c5e3d94d9478
Signed-off-by: Paolo Abeni <[email protected]>
Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
include/net/mptcp.h | 7 +++++--
net/mptcp/options.c | 6 +++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index f7263fe2a2e4..ee70f597a4de 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -27,7 +27,9 @@ struct mptcp_ext {
u32 subflow_seq;
u16 data_len;
__sum16 csum;
- u8 use_map:1,
+
+ struct_group(flags,
+ u8 use_map:1,
dsn64:1,
data_fin:1,
use_ack:1,
@@ -35,9 +37,10 @@ struct mptcp_ext {
mpc_map:1,
frozen:1,
reset_transient:1;
- u8 reset_reason:4,
+ u8 reset_reason:4,
csum_reqd:1,
infinite_map:1;
+ ); /* end of flags group */
};
#define MPTCPOPT_HMAC_LEN 20
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 51ca334678b4..f9f587203c35 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -572,6 +572,11 @@ static bool mptcp_established_options_dss(struct sock *sk,
struct sk_buff *skb,
unsigned int ack_size;
bool ret = false;
+ /* Zero `use_ack` and `use_map` flags with one shot. */
+ BUILD_BUG_ON(sizeof_field(struct mptcp_ext, flags) != sizeof(u16));
+ BUILD_BUG_ON(!IS_ALIGNED(offsetof(struct mptcp_ext, flags),
+ sizeof(u16)));
+ *(u16 *)&opts->ext_copy.flags = 0;
opts->csum_reqd = READ_ONCE(msk->csum_enabled);
mpext = skb ? mptcp_get_ext(skb) : NULL;
@@ -595,7 +600,6 @@ static bool mptcp_established_options_dss(struct sock *sk,
struct sk_buff *skb,
/* passive sockets msk will set the 'can_ack' after accept(), even
* if the first subflow may have the already the remote key handy
*/
- opts->ext_copy.use_ack = 0;
if (!READ_ONCE(msk->can_ack)) {
*size = ALIGN(dss_size, 4);
return ret;
--
2.53.0
