netdev - Re: [PATCH v2 net-next 3/8] inet: implement lockless IP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iKa2oJZZRW6mrRyO7-=aXjBr7wak2oZ3X74yLq3B_jcDQ@mail.gmail.com>
Date: Wed, 18 Oct 2023 09:56:06 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Christoph Paasch <christophpaasch@...oud.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>, netdev <netdev@...r.kernel.org>, 
	Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [PATCH v2 net-next 3/8] inet: implement lockless IP_TOS

On Wed, Oct 18, 2023 at 5:37 AM Christoph Paasch
<christophpaasch@...oud.com> wrote:
>
> Hello Eric,
>
> > On Sep 21, 2023, at 8:42 PM, Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > Some reads of inet->tos are racy.
> >
> > Add needed READ_ONCE() annotations and convert IP_TOS option lockless.
> >
> > v2: missing changes in include/net/route.h (David Ahern)
> >
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > ---
> > include/net/ip.h                              |  3 +-
> > include/net/route.h                           |  4 +--
> > net/dccp/ipv4.c                               |  2 +-
> > net/ipv4/inet_diag.c                          |  2 +-
> > net/ipv4/ip_output.c                          |  4 +--
> > net/ipv4/ip_sockglue.c                        | 29 ++++++++-----------
> > net/ipv4/tcp_ipv4.c                           |  9 +++---
> > net/mptcp/sockopt.c                           |  8 ++---
> > net/sctp/protocol.c                           |  4 +--
> > .../selftests/net/mptcp/mptcp_connect.sh      |  2 +-
> > 10 files changed, 31 insertions(+), 36 deletions(-)
>
> This patch causes a NULL-pointer deref in my syzkaller instances:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 12bad6067 P4D 12bad6067 PUD 12bad5067 PMD 0
> Oops: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 2750 Comm: syz-executor.5 Not tainted 6.6.0-rc4-g7a5720a344e7 #49
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014
> RIP: 0010:tcp_get_metrics+0x118/0x8f0 net/ipv4/tcp_metrics.c:321
> Code: c7 44 24 70 02 00 8b 03 89 44 24 48 c7 44 24 4c 00 00 00 00 66 c7 44 24 58 02 00 66 ba 02 00 b1 01 89 4c 24 04 4c 89 7c 24 10 <49> 8b 0f 48 8b 89 50 05 00 00 48 89 4c 24 30 33 81 00 02 00 00 69
> RSP: 0018:ffffc90000af79b8 EFLAGS: 00010293
> RAX: 000000000100007f RBX: ffff88812ae8f500 RCX: ffff88812b5f8f01
> RDX: 0000000000000002 RSI: ffffffff8300f080 RDI: 0000000000000002
> RBP: 0000000000000002 R08: 0000000000000003 R09: ffffffff8205eca0
> R10: 0000000000000002 R11: ffff88812b5f8f00 R12: ffff88812a9e0580
> R13: 0000000000000000 R14: ffff88812ae8fbd2 R15: 0000000000000000
> FS: 00007f70a006b640(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000012bad7003 CR4: 0000000000170ee0
> Call Trace:
> <TASK>
> tcp_fastopen_cache_get+0x32/0x140 net/ipv4/tcp_metrics.c:567
> tcp_fastopen_cookie_check+0x28/0x180 net/ipv4/tcp_fastopen.c:419
> tcp_connect+0x9c8/0x12a0 net/ipv4/tcp_output.c:3839
> tcp_v4_connect+0x645/0x6e0 net/ipv4/tcp_ipv4.c:323
> __inet_stream_connect+0x120/0x590 net/ipv4/af_inet.c:676
> tcp_sendmsg_fastopen+0x2d6/0x3a0 net/ipv4/tcp.c:1021
> tcp_sendmsg_locked+0x1957/0x1b00 net/ipv4/tcp.c:1073
> tcp_sendmsg+0x30/0x50 net/ipv4/tcp.c:1336
> __sock_sendmsg+0x83/0xd0 net/socket.c:730
> __sys_sendto+0x20a/0x2a0 net/socket.c:2194
> __do_sys_sendto net/socket.c:2206 [inline]
> __se_sys_sendto net/socket.c:2202 [inline]
> __x64_sys_sendto+0x28/0x30 net/socket.c:2202
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x47/0xa0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> The reason is that setting IP_TOS calls sk_reset_dst, which then causes these issues in the places where we assume that the dst in the socket is set (specifically, the tcp_connect-path).
>

Thanks for the report.

You are right, too many places calling __sk_dst_get() would have to
properly use RCU,
this does not seem worth the pain.

I will send a fix.