diff mbox series

[net,v3] net: do not leave a dangling sk pointer, when socket creation fails

Message ID 20240617210205.67311-1-ignat@cloudflare.com (mailing list archive)
State Accepted
Commit 6cd4a78d962bebbaf8beb7d2ead3f34120e3f7b2
Delegated to: Netdev Maintainers
Headers show
Series [net,v3] net: do not leave a dangling sk pointer, when socket creation fails | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 859 this patch: 859
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 3 blamed authors not CCed: andrii@kernel.org kpsingh@kernel.org ast@kernel.org; 4 maintainers not CCed: andrii@kernel.org bpf@vger.kernel.org kpsingh@kernel.org ast@kernel.org
netdev/build_clang success Errors and warnings before: 863 this patch: 863
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 867 this patch: 867
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 9 this patch: 9
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-06-20--03-00 (tests: 659)

Commit Message

Ignat Korchagin June 17, 2024, 9:02 p.m. UTC
It is possible to trigger a use-after-free by:
  * attaching an fentry probe to __sock_release() and the probe calling the
    bpf_get_socket_cookie() helper
  * running traceroute -I 1.1.1.1 on a freshly booted VM

A KASAN enabled kernel will log something like below (decoded and stripped):
==================================================================
BUG: KASAN: slab-use-after-free in __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
Read of size 8 at addr ffff888007110dd8 by task traceroute/299

CPU: 2 PID: 299 Comm: traceroute Tainted: G            E      6.10.0-rc2+ #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
 <TASK>
dump_stack_lvl (lib/dump_stack.c:117 (discriminator 1))
print_report (mm/kasan/report.c:378 mm/kasan/report.c:488)
? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
kasan_report (mm/kasan/report.c:603)
? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
kasan_check_range (mm/kasan/generic.c:183 mm/kasan/generic.c:189)
__sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
bpf_get_socket_ptr_cookie (./arch/x86/include/asm/preempt.h:94 ./include/linux/sock_diag.h:42 net/core/filter.c:5094 net/core/filter.c:5092)
bpf_prog_875642cf11f1d139___sock_release+0x6e/0x8e
bpf_trampoline_6442506592+0x47/0xaf
__sock_release (net/socket.c:652)
__sock_create (net/socket.c:1601)
...
Allocated by task 299 on cpu 2 at 78.328492s:
kasan_save_stack (mm/kasan/common.c:48)
kasan_save_track (mm/kasan/common.c:68)
__kasan_slab_alloc (mm/kasan/common.c:312 mm/kasan/common.c:338)
kmem_cache_alloc_noprof (mm/slub.c:3941 mm/slub.c:4000 mm/slub.c:4007)
sk_prot_alloc (net/core/sock.c:2075)
sk_alloc (net/core/sock.c:2134)
inet_create (net/ipv4/af_inet.c:327 net/ipv4/af_inet.c:252)
__sock_create (net/socket.c:1572)
__sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
__x64_sys_socket (net/socket.c:1718)
do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

Freed by task 299 on cpu 2 at 78.328502s:
kasan_save_stack (mm/kasan/common.c:48)
kasan_save_track (mm/kasan/common.c:68)
kasan_save_free_info (mm/kasan/generic.c:582)
poison_slab_object (mm/kasan/common.c:242)
__kasan_slab_free (mm/kasan/common.c:256)
kmem_cache_free (mm/slub.c:4437 mm/slub.c:4511)
__sk_destruct (net/core/sock.c:2117 net/core/sock.c:2208)
inet_create (net/ipv4/af_inet.c:397 net/ipv4/af_inet.c:252)
__sock_create (net/socket.c:1572)
__sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
__x64_sys_socket (net/socket.c:1718)
do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

Fix this by clearing the struct socket reference in sk_common_release() to cover
all protocol families create functions, which may already attached the
reference to the sk object with sock_init_data().

Fixes: c5dbb89fc2ac ("bpf: Expose bpf_get_socket_cookie to tracing programs")
Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/netdev/20240613194047.36478-1-kuniyu@amazon.com/T/
---
Changes in v3:
  * re-added KASAN repro steps to the commit message (somehow stripped in v2)
  * stripped timestamps and thread id from the KASAN splat
  * removed comment from the code (commit message should be enough)

Changes in v2:
  * moved the NULL-ing of the socket reference to sk_common_release() (as
    suggested by Kuniyuki Iwashima)
  * trimmed down the KASAN report in the commit message to show only relevant
    info

 net/core/sock.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Kuniyuki Iwashima June 17, 2024, 9:15 p.m. UTC | #1
From: Ignat Korchagin <ignat@cloudflare.com>
Date: Mon, 17 Jun 2024 22:02:05 +0100
> It is possible to trigger a use-after-free by:
>   * attaching an fentry probe to __sock_release() and the probe calling the
>     bpf_get_socket_cookie() helper
>   * running traceroute -I 1.1.1.1 on a freshly booted VM
> 
> A KASAN enabled kernel will log something like below (decoded and stripped):
> ==================================================================
> BUG: KASAN: slab-use-after-free in __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> Read of size 8 at addr ffff888007110dd8 by task traceroute/299
> 
> CPU: 2 PID: 299 Comm: traceroute Tainted: G            E      6.10.0-rc2+ #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> Call Trace:
>  <TASK>
> dump_stack_lvl (lib/dump_stack.c:117 (discriminator 1))
> print_report (mm/kasan/report.c:378 mm/kasan/report.c:488)
> ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> kasan_report (mm/kasan/report.c:603)
> ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> kasan_check_range (mm/kasan/generic.c:183 mm/kasan/generic.c:189)
> __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> bpf_get_socket_ptr_cookie (./arch/x86/include/asm/preempt.h:94 ./include/linux/sock_diag.h:42 net/core/filter.c:5094 net/core/filter.c:5092)
> bpf_prog_875642cf11f1d139___sock_release+0x6e/0x8e
> bpf_trampoline_6442506592+0x47/0xaf
> __sock_release (net/socket.c:652)
> __sock_create (net/socket.c:1601)
> ...
> Allocated by task 299 on cpu 2 at 78.328492s:
> kasan_save_stack (mm/kasan/common.c:48)
> kasan_save_track (mm/kasan/common.c:68)
> __kasan_slab_alloc (mm/kasan/common.c:312 mm/kasan/common.c:338)
> kmem_cache_alloc_noprof (mm/slub.c:3941 mm/slub.c:4000 mm/slub.c:4007)
> sk_prot_alloc (net/core/sock.c:2075)
> sk_alloc (net/core/sock.c:2134)
> inet_create (net/ipv4/af_inet.c:327 net/ipv4/af_inet.c:252)
> __sock_create (net/socket.c:1572)
> __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> __x64_sys_socket (net/socket.c:1718)
> do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> 
> Freed by task 299 on cpu 2 at 78.328502s:
> kasan_save_stack (mm/kasan/common.c:48)
> kasan_save_track (mm/kasan/common.c:68)
> kasan_save_free_info (mm/kasan/generic.c:582)
> poison_slab_object (mm/kasan/common.c:242)
> __kasan_slab_free (mm/kasan/common.c:256)
> kmem_cache_free (mm/slub.c:4437 mm/slub.c:4511)
> __sk_destruct (net/core/sock.c:2117 net/core/sock.c:2208)
> inet_create (net/ipv4/af_inet.c:397 net/ipv4/af_inet.c:252)
> __sock_create (net/socket.c:1572)
> __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> __x64_sys_socket (net/socket.c:1718)
> do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> 
> Fix this by clearing the struct socket reference in sk_common_release() to cover
> all protocol families create functions, which may already attached the
> reference to the sk object with sock_init_data().
> 
> Fixes: c5dbb89fc2ac ("bpf: Expose bpf_get_socket_cookie to tracing programs")
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/netdev/20240613194047.36478-1-kuniyu@amazon.com/T/

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Thanks!


P.S. next time, please make sure 24h pass before reposting for netdev.

  See: Documentation/process/maintainer-netdev.rst
D. Wythe June 19, 2024, 12:31 p.m. UTC | #2
On 6/18/24 5:02 AM, Ignat Korchagin wrote:
> It is possible to trigger a use-after-free by:
>    * attaching an fentry probe to __sock_release() and the probe calling the
>      bpf_get_socket_cookie() helper
>    * running traceroute -I 1.1.1.1 on a freshly booted VM
>
> A KASAN enabled kernel will log something like below (decoded and stripped):
> ==================================================================
> BUG: KASAN: slab-use-after-free in __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> Read of size 8 at addr ffff888007110dd8 by task traceroute/299
>
> CPU: 2 PID: 299 Comm: traceroute Tainted: G            E      6.10.0-rc2+ #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> Call Trace:
>   <TASK>
> dump_stack_lvl (lib/dump_stack.c:117 (discriminator 1))
> print_report (mm/kasan/report.c:378 mm/kasan/report.c:488)
> ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> kasan_report (mm/kasan/report.c:603)
> ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> kasan_check_range (mm/kasan/generic.c:183 mm/kasan/generic.c:189)
> __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> bpf_get_socket_ptr_cookie (./arch/x86/include/asm/preempt.h:94 ./include/linux/sock_diag.h:42 net/core/filter.c:5094 net/core/filter.c:5092)
> bpf_prog_875642cf11f1d139___sock_release+0x6e/0x8e
> bpf_trampoline_6442506592+0x47/0xaf
> __sock_release (net/socket.c:652)
> __sock_create (net/socket.c:1601)
> ...
> Allocated by task 299 on cpu 2 at 78.328492s:
> kasan_save_stack (mm/kasan/common.c:48)
> kasan_save_track (mm/kasan/common.c:68)
> __kasan_slab_alloc (mm/kasan/common.c:312 mm/kasan/common.c:338)
> kmem_cache_alloc_noprof (mm/slub.c:3941 mm/slub.c:4000 mm/slub.c:4007)
> sk_prot_alloc (net/core/sock.c:2075)
> sk_alloc (net/core/sock.c:2134)
> inet_create (net/ipv4/af_inet.c:327 net/ipv4/af_inet.c:252)
> __sock_create (net/socket.c:1572)
> __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> __x64_sys_socket (net/socket.c:1718)
> do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>
> Freed by task 299 on cpu 2 at 78.328502s:
> kasan_save_stack (mm/kasan/common.c:48)
> kasan_save_track (mm/kasan/common.c:68)
> kasan_save_free_info (mm/kasan/generic.c:582)
> poison_slab_object (mm/kasan/common.c:242)
> __kasan_slab_free (mm/kasan/common.c:256)
> kmem_cache_free (mm/slub.c:4437 mm/slub.c:4511)
> __sk_destruct (net/core/sock.c:2117 net/core/sock.c:2208)
> inet_create (net/ipv4/af_inet.c:397 net/ipv4/af_inet.c:252)
> __sock_create (net/socket.c:1572)
> __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> __x64_sys_socket (net/socket.c:1718)
> do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>
> Fix this by clearing the struct socket reference in sk_common_release() to cover
> all protocol families create functions, which may already attached the
> reference to the sk object with sock_init_data().
>
> Fixes: c5dbb89fc2ac ("bpf: Expose bpf_get_socket_cookie to tracing programs")
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/netdev/20240613194047.36478-1-kuniyu@amazon.com/T/
> ---
> Changes in v3:
>    * re-added KASAN repro steps to the commit message (somehow stripped in v2)
>    * stripped timestamps and thread id from the KASAN splat
>    * removed comment from the code (commit message should be enough)
>
> Changes in v2:
>    * moved the NULL-ing of the socket reference to sk_common_release() (as
>      suggested by Kuniyuki Iwashima)
>    * trimmed down the KASAN report in the commit message to show only relevant
>      info
>
>   net/core/sock.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8629f9aecf91..100e975073ca 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -3742,6 +3742,9 @@ void sk_common_release(struct sock *sk)
>   
>   	sk->sk_prot->unhash(sk);
>   
> +	if (sk->sk_socket)
> +		sk->sk_socket->sk = NULL;
> +
>   	/*
>   	 * In this point socket cannot receive new packets, but it is possible
>   	 * that some packets are in flight because some CPU runs receiver and

Reviewed-by: D. Wythe <alibuda@linux.alibaba.com>


A small tip:

It seems that you might have missed CCing some maintainers, using
scripts/get_maintainer.pl "Your patch" can help you avoid this issue
again.


D. Wythe
Ignat Korchagin June 19, 2024, 1:08 p.m. UTC | #3
On Wed, Jun 19, 2024 at 1:31 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
>
>
>
> On 6/18/24 5:02 AM, Ignat Korchagin wrote:
> > It is possible to trigger a use-after-free by:
> >    * attaching an fentry probe to __sock_release() and the probe calling the
> >      bpf_get_socket_cookie() helper
> >    * running traceroute -I 1.1.1.1 on a freshly booted VM
> >
> > A KASAN enabled kernel will log something like below (decoded and stripped):
> > ==================================================================
> > BUG: KASAN: slab-use-after-free in __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > Read of size 8 at addr ffff888007110dd8 by task traceroute/299
> >
> > CPU: 2 PID: 299 Comm: traceroute Tainted: G            E      6.10.0-rc2+ #2
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> > Call Trace:
> >   <TASK>
> > dump_stack_lvl (lib/dump_stack.c:117 (discriminator 1))
> > print_report (mm/kasan/report.c:378 mm/kasan/report.c:488)
> > ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > kasan_report (mm/kasan/report.c:603)
> > ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > kasan_check_range (mm/kasan/generic.c:183 mm/kasan/generic.c:189)
> > __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > bpf_get_socket_ptr_cookie (./arch/x86/include/asm/preempt.h:94 ./include/linux/sock_diag.h:42 net/core/filter.c:5094 net/core/filter.c:5092)
> > bpf_prog_875642cf11f1d139___sock_release+0x6e/0x8e
> > bpf_trampoline_6442506592+0x47/0xaf
> > __sock_release (net/socket.c:652)
> > __sock_create (net/socket.c:1601)
> > ...
> > Allocated by task 299 on cpu 2 at 78.328492s:
> > kasan_save_stack (mm/kasan/common.c:48)
> > kasan_save_track (mm/kasan/common.c:68)
> > __kasan_slab_alloc (mm/kasan/common.c:312 mm/kasan/common.c:338)
> > kmem_cache_alloc_noprof (mm/slub.c:3941 mm/slub.c:4000 mm/slub.c:4007)
> > sk_prot_alloc (net/core/sock.c:2075)
> > sk_alloc (net/core/sock.c:2134)
> > inet_create (net/ipv4/af_inet.c:327 net/ipv4/af_inet.c:252)
> > __sock_create (net/socket.c:1572)
> > __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> > __x64_sys_socket (net/socket.c:1718)
> > do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> > entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> >
> > Freed by task 299 on cpu 2 at 78.328502s:
> > kasan_save_stack (mm/kasan/common.c:48)
> > kasan_save_track (mm/kasan/common.c:68)
> > kasan_save_free_info (mm/kasan/generic.c:582)
> > poison_slab_object (mm/kasan/common.c:242)
> > __kasan_slab_free (mm/kasan/common.c:256)
> > kmem_cache_free (mm/slub.c:4437 mm/slub.c:4511)
> > __sk_destruct (net/core/sock.c:2117 net/core/sock.c:2208)
> > inet_create (net/ipv4/af_inet.c:397 net/ipv4/af_inet.c:252)
> > __sock_create (net/socket.c:1572)
> > __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> > __x64_sys_socket (net/socket.c:1718)
> > do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> > entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> >
> > Fix this by clearing the struct socket reference in sk_common_release() to cover
> > all protocol families create functions, which may already attached the
> > reference to the sk object with sock_init_data().
> >
> > Fixes: c5dbb89fc2ac ("bpf: Expose bpf_get_socket_cookie to tracing programs")
> > Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> > Cc: stable@vger.kernel.org
> > Link: https://lore.kernel.org/netdev/20240613194047.36478-1-kuniyu@amazon.com/T/
> > ---
> > Changes in v3:
> >    * re-added KASAN repro steps to the commit message (somehow stripped in v2)
> >    * stripped timestamps and thread id from the KASAN splat
> >    * removed comment from the code (commit message should be enough)
> >
> > Changes in v2:
> >    * moved the NULL-ing of the socket reference to sk_common_release() (as
> >      suggested by Kuniyuki Iwashima)
> >    * trimmed down the KASAN report in the commit message to show only relevant
> >      info
> >
> >   net/core/sock.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index 8629f9aecf91..100e975073ca 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -3742,6 +3742,9 @@ void sk_common_release(struct sock *sk)
> >
> >       sk->sk_prot->unhash(sk);
> >
> > +     if (sk->sk_socket)
> > +             sk->sk_socket->sk = NULL;
> > +
> >       /*
> >        * In this point socket cannot receive new packets, but it is possible
> >        * that some packets are in flight because some CPU runs receiver and
>
> Reviewed-by: D. Wythe <alibuda@linux.alibaba.com>
>
>
> A small tip:
>
> It seems that you might have missed CCing some maintainers, using
> scripts/get_maintainer.pl "Your patch" can help you avoid this issue
> again.

Thanks. I did scripts/get_maintainer.pl <file I'm modifying>. Not sure
if it is different.

>
> D. Wythe
>
>
>
Ignat Korchagin June 19, 2024, 2:34 p.m. UTC | #4
On Wed, Jun 19, 2024 at 2:08 PM Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> On Wed, Jun 19, 2024 at 1:31 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> >
> >
> >
> > On 6/18/24 5:02 AM, Ignat Korchagin wrote:
> > > It is possible to trigger a use-after-free by:
> > >    * attaching an fentry probe to __sock_release() and the probe calling the
> > >      bpf_get_socket_cookie() helper
> > >    * running traceroute -I 1.1.1.1 on a freshly booted VM
> > >
> > > A KASAN enabled kernel will log something like below (decoded and stripped):
> > > ==================================================================
> > > BUG: KASAN: slab-use-after-free in __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > > Read of size 8 at addr ffff888007110dd8 by task traceroute/299
> > >
> > > CPU: 2 PID: 299 Comm: traceroute Tainted: G            E      6.10.0-rc2+ #2
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> > > Call Trace:
> > >   <TASK>
> > > dump_stack_lvl (lib/dump_stack.c:117 (discriminator 1))
> > > print_report (mm/kasan/report.c:378 mm/kasan/report.c:488)
> > > ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > > kasan_report (mm/kasan/report.c:603)
> > > ? __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > > kasan_check_range (mm/kasan/generic.c:183 mm/kasan/generic.c:189)
> > > __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> > > bpf_get_socket_ptr_cookie (./arch/x86/include/asm/preempt.h:94 ./include/linux/sock_diag.h:42 net/core/filter.c:5094 net/core/filter.c:5092)
> > > bpf_prog_875642cf11f1d139___sock_release+0x6e/0x8e
> > > bpf_trampoline_6442506592+0x47/0xaf
> > > __sock_release (net/socket.c:652)
> > > __sock_create (net/socket.c:1601)
> > > ...
> > > Allocated by task 299 on cpu 2 at 78.328492s:
> > > kasan_save_stack (mm/kasan/common.c:48)
> > > kasan_save_track (mm/kasan/common.c:68)
> > > __kasan_slab_alloc (mm/kasan/common.c:312 mm/kasan/common.c:338)
> > > kmem_cache_alloc_noprof (mm/slub.c:3941 mm/slub.c:4000 mm/slub.c:4007)
> > > sk_prot_alloc (net/core/sock.c:2075)
> > > sk_alloc (net/core/sock.c:2134)
> > > inet_create (net/ipv4/af_inet.c:327 net/ipv4/af_inet.c:252)
> > > __sock_create (net/socket.c:1572)
> > > __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> > > __x64_sys_socket (net/socket.c:1718)
> > > do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> > > entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> > >
> > > Freed by task 299 on cpu 2 at 78.328502s:
> > > kasan_save_stack (mm/kasan/common.c:48)
> > > kasan_save_track (mm/kasan/common.c:68)
> > > kasan_save_free_info (mm/kasan/generic.c:582)
> > > poison_slab_object (mm/kasan/common.c:242)
> > > __kasan_slab_free (mm/kasan/common.c:256)
> > > kmem_cache_free (mm/slub.c:4437 mm/slub.c:4511)
> > > __sk_destruct (net/core/sock.c:2117 net/core/sock.c:2208)
> > > inet_create (net/ipv4/af_inet.c:397 net/ipv4/af_inet.c:252)
> > > __sock_create (net/socket.c:1572)
> > > __sys_socket (net/socket.c:1660 net/socket.c:1644 net/socket.c:1706)
> > > __x64_sys_socket (net/socket.c:1718)
> > > do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
> > > entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> > >
> > > Fix this by clearing the struct socket reference in sk_common_release() to cover
> > > all protocol families create functions, which may already attached the
> > > reference to the sk object with sock_init_data().
> > >
> > > Fixes: c5dbb89fc2ac ("bpf: Expose bpf_get_socket_cookie to tracing programs")
> > > Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> > > Cc: stable@vger.kernel.org
> > > Link: https://lore.kernel.org/netdev/20240613194047.36478-1-kuniyu@amazon.com/T/
> > > ---
> > > Changes in v3:
> > >    * re-added KASAN repro steps to the commit message (somehow stripped in v2)
> > >    * stripped timestamps and thread id from the KASAN splat
> > >    * removed comment from the code (commit message should be enough)
> > >
> > > Changes in v2:
> > >    * moved the NULL-ing of the socket reference to sk_common_release() (as
> > >      suggested by Kuniyuki Iwashima)
> > >    * trimmed down the KASAN report in the commit message to show only relevant
> > >      info
> > >
> > >   net/core/sock.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > >
> > > diff --git a/net/core/sock.c b/net/core/sock.c
> > > index 8629f9aecf91..100e975073ca 100644
> > > --- a/net/core/sock.c
> > > +++ b/net/core/sock.c
> > > @@ -3742,6 +3742,9 @@ void sk_common_release(struct sock *sk)
> > >
> > >       sk->sk_prot->unhash(sk);
> > >
> > > +     if (sk->sk_socket)
> > > +             sk->sk_socket->sk = NULL;
> > > +
> > >       /*
> > >        * In this point socket cannot receive new packets, but it is possible
> > >        * that some packets are in flight because some CPU runs receiver and
> >
> > Reviewed-by: D. Wythe <alibuda@linux.alibaba.com>
> >
> >
> > A small tip:
> >
> > It seems that you might have missed CCing some maintainers, using
> > scripts/get_maintainer.pl "Your patch" can help you avoid this issue
> > again.
>
> Thanks. I did scripts/get_maintainer.pl <file I'm modifying>. Not sure
> if it is different.

My bad: it is different or I actually forgot to re-run it, because
v2/v3 modifies a different file.

> >
> > D. Wythe
> >
> >
> >
Jakub Kicinski June 19, 2024, 3:13 p.m. UTC | #5
On Wed, 19 Jun 2024 15:34:47 +0100 Ignat Korchagin wrote:
> > Thanks. I did scripts/get_maintainer.pl <file I'm modifying>. Not sure
> > if it is different.  
> 
> My bad: it is different or I actually forgot to re-run it, because
> v2/v3 modifies a different file.

Also you should run it on the patch:

$ git format-patch HEAD~
$ ./scripts/get_maintainer.pl 0001-${subject}.patch

the file version doesn't include CCs based on the commit message, most
importantly doesn't CC people who authored / reviewed the commit under
the Fixes tag.
patchwork-bot+netdevbpf@kernel.org June 20, 2024, 9:20 a.m. UTC | #6
Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 17 Jun 2024 22:02:05 +0100 you wrote:
> It is possible to trigger a use-after-free by:
>   * attaching an fentry probe to __sock_release() and the probe calling the
>     bpf_get_socket_cookie() helper
>   * running traceroute -I 1.1.1.1 on a freshly booted VM
> 
> A KASAN enabled kernel will log something like below (decoded and stripped):
> ==================================================================
> BUG: KASAN: slab-use-after-free in __sock_gen_cookie (./arch/x86/include/asm/atomic64_64.h:15 ./include/linux/atomic/atomic-arch-fallback.h:2583 ./include/linux/atomic/atomic-instrumented.h:1611 net/core/sock_diag.c:29)
> Read of size 8 at addr ffff888007110dd8 by task traceroute/299
> 
> [...]

Here is the summary with links:
  - [net,v3] net: do not leave a dangling sk pointer, when socket creation fails
    https://git.kernel.org/netdev/net/c/6cd4a78d962b

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/core/sock.c b/net/core/sock.c
index 8629f9aecf91..100e975073ca 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3742,6 +3742,9 @@  void sk_common_release(struct sock *sk)
 
 	sk->sk_prot->unhash(sk);
 
+	if (sk->sk_socket)
+		sk->sk_socket->sk = NULL;
+
 	/*
 	 * In this point socket cannot receive new packets, but it is possible
 	 * that some packets are in flight because some CPU runs receiver and