On Wed, Jun 19, 2024 at 2:08 PM Ignat Korchagin <ignat@xxxxxxxxxxxxxx> wrote:
>
> On Wed, Jun 19, 2024 at 1:31 PM D. Wythe <alibuda@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > > Signed-off-by: Ignat Korchagin <ignat@xxxxxxxxxxxxxx>
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Link: https://lore.kernel.org/netdev/20240613194047.36478-1-kuniyu@xxxxxxxxxx/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@xxxxxxxxxxxxxxxxx>
> >
> >
> > 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
> >
> >
> >
