Message-ID: <CAAVpQUB5mGbD79ZLVCi0G8UDpwjQW15=BV9maB6ikLzCYTjA0A@mail.gmail.com>
Date: Wed, 11 Feb 2026 15:01:29 -0800
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	John Fastabend <john.fastabend@...il.com>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Michal Luczaj <mhal@...x.co>, Kuniyuki Iwashima <kuni1840@...il.com>, bpf <bpf@...r.kernel.org>, 
	Network Development <netdev@...r.kernel.org>, Martin KaFai Lau <martin.lau@...nel.org>
Subject: Re: [PATCH v1 bpf 1/2] bpf: Reject access to unix_sk(sk)->peer.

On Wed, Feb 11, 2026 at 2:39 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Wed, Feb 11, 2026 at 2:23 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
> >
> > On Wed, Feb 11, 2026 at 1:26 PM Alexei Starovoitov
> > <alexei.starovoitov@...il.com> wrote:
> > >
> > > On Tue, Feb 10, 2026 at 8:23 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
> > > >
> > > > On Tue, Feb 10, 2026 at 6:47 PM Alexei Starovoitov
> > > > <alexei.starovoitov@...il.com> wrote:
> > > > >
> > > > > On Mon, Feb 9, 2026 at 5:19 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
> > > > > >
> > > > > > On Mon, Feb 9, 2026 at 3:00 PM Alexei Starovoitov
> > > > > > <alexei.starovoitov@...il.com> wrote:
> > > > > > >
> > > > > > > On Sat, Feb 7, 2026 at 3:07 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
> > > > > > > >
> > > > > > > > Michal Luczaj reported use-after-free of unix_sk(sk)->peer by
> > > > > > > > bpf_skc_to_unix_sock(). [0]
> > > > > > > >
> > > > > > > > Accessing unix_sk(sk)->peer is safe only under unix_state_lock(),
> > > > > > > > but there are many functions where bpf prog can access the field
> > > > > > > > locklessly via fentry/fexit.
> > > > > > > >
> > > > > > > > unix_dgram_connect() could clear unix_sk(sk)->peer and release
> > > > > > > > the last refcnt of the peer sk while a bpf prog is accessing it,
> > > > > > > > resulting in use-after-free.
> > > > > > > >
> > > > > > > > Another problematic scenario is that unix_sk(sk)->peer could
> > > > > > > > go away while being passed to bpf_setsockopt() in bpf iter.
> > > > > > > >
> > > > > > > > To avoid such issues, let's reject access to unix_sk(sk)->peer
> > > > > > > > by marking the pointer with PTR_UNTRUSTED.
> > > > > > > >
> > > > > > > > If needed, we could add a new helper later that uses unix_peer_get()
> > > > > > > > and requires bpf_sk_release().
> > > > > > > >
> > > > > > > > [0]:
> > > > > > > > BUG: KASAN: slab-use-after-free in bpf_skc_to_unix_sock+0xa4/0xb0
> > > > > > > > Read of size 2 at addr ffff888147d38890 by task test_progs/2495
> > > > > > > > Call Trace:
> > > > > > > >  dump_stack_lvl+0x5d/0x80
> > > > > > > >  print_report+0x170/0x4f3
> > > > > > > >  kasan_report+0xe1/0x180
> > > > > > > >  bpf_skc_to_unix_sock+0xa4/0xb0
> > > > > > > >  bpf_prog_564a1c39c35d86a2_unix_shutdown_entry+0x8a/0x8e
> > > > > > > >  bpf_trampoline_6442564662+0x47/0xab
> > > > > > > >  unix_shutdown+0x9/0x880
> > > > > > > >  __sys_shutdown+0xe1/0x160
> > > > > > > >  __x64_sys_shutdown+0x52/0x90
> > > > > > > >  do_syscall_64+0x6b/0x3a0
> > > > > > > >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > > > > > > >
> > > > > > > > Fixes: 9eeb3aa33ae0 ("bpf: Add bpf_skc_to_unix_sock() helper")
> > > > > > > > Reported-by: Michal Luczaj <mhal@...x.co>
> > > > > > > > Closes: https://lore.kernel.org/all/408569e7-2b82-4eff-b767-79ce6ef6cae0@rbox.co/
> > > > > > > > Suggested-by: Martin KaFai Lau <martin.lau@...nel.org>
> > > > > > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> > > > > > > > ---
> > > > > > > >  kernel/bpf/verifier.c                         | 18 +++++++++++++
> > > > > > > >  .../selftests/bpf/progs/verifier_sock.c       | 25 +++++++++++++++++++
> > > > > > > >  2 files changed, 43 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > > > > > index 3135643d5695..b328a1640c82 100644
> > > > > > > > --- a/kernel/bpf/verifier.c
> > > > > > > > +++ b/kernel/bpf/verifier.c
> > > > > > > > @@ -7076,6 +7076,7 @@ static int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val,
> > > > > > > >  #define BTF_TYPE_SAFE_RCU_OR_NULL(__type)  __PASTE(__type, __safe_rcu_or_null)
> > > > > > > >  #define BTF_TYPE_SAFE_TRUSTED(__type)  __PASTE(__type, __safe_trusted)
> > > > > > > >  #define BTF_TYPE_SAFE_TRUSTED_OR_NULL(__type)  __PASTE(__type, __safe_trusted_or_null)
> > > > > > > > +#define BTF_TYPE_SAFE_UNTRUSTED(__type)  __PASTE(__type, __safe_untrusted)
> > > > > > > >
> > > > > > > >  /*
> > > > > > > >   * Allow list few fields as RCU trusted or full trusted.
> > > > > > > > @@ -7154,6 +7155,10 @@ BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct vm_area_struct) {
> > > > > > > >         struct file *vm_file;
> > > > > > > >  };
> > > > > > > >
> > > > > > > > +BTF_TYPE_SAFE_UNTRUSTED(struct unix_sock) {
> > > > > > > > +       struct sock *peer;
> > > > > > > > +};
> > > > > > > > +
> > > > > > >
> > > > > > > That's just one out of many. It's not really a fix,
> > > > > > > but a bandaid for one case.
> > > > > >
> > > > > > Right, this is only one example where a pointer lifetime
> > > > > > does not match with bpf prog.
> > > > > >
> > > > > > > We discussed it in the past and the only way forward is to make
> > > > > > > helpers only accept trusted args.
> > > > > > > Recently we made KF_TRUSTED_ARGS a default for all kfuncs.
> > > > > > > We should do the same for helpers.
> > > > > >
> > > > > > What about just reading an untrusted pointer ?
> > > > >
> > > > > What do you mean 'untrusted' ?
> > > > > There are 3 types of ptr_to_btf_id:
> > > > > trusted, untrusted, legacy (without either trusted or untrusted flags).
> > > >
> > > > Ah sorry, I used 'untrusted' since the pointer is not always
> > > > valid, but I think legacy is the one I meant in this case.
> > > >
> > > >
> > > > >
> > > > > kfuncs now accept only trusted, while helpers still accept
> > > > > trusted and legacy. The legacy should be disallowed.
> > > > > That might break some bpf programs, so it has to be done carefully.
> > > > >
> > > > > > I used bpf_skc_to_unix_sock() in selftest in case compiler
> > > > > > could optimise out branches that always return 0.
> > > > > >
> > > > > > Currently, this passes the verifier and can trigger UAF.
> > > > > >
> > > > > > u = bpf_skc_to_unix_sock(sock->sk);
> > > > > > if (!u || !u->peer)
> > > > > >     return 0;
> > > > > > if (u->peer->__sk_common.skc_family) # unsafe access
> > > > > >     return 0;
> > > > >
> > > > > No. It's fine. Both legacy and untrusted ptr_to_btf_id get probe_mem
> > > > > treatment. So above works and won't crash the kernel.
> > > > > It's effectively inlined probe_read_kernel.
> > > >
> > > > Yes, it doesn't crash.
> > > >
> > > > I was just unsure about the bpf expectation [0] like how we
> > > > should treat KASAN use-after-free report by bpf prog reading
> > > > a pointer whose lifetime is not guaranteed by bpf helper/verifier.
> > > >
> > > > So, if this is WAI, I'm fine with that.
> > > >
> > > > [0]: https://lore.kernel.org/all/CAAVpQUDrC1rO2diM3gVg5GV5CTVAUS2RbLP+SpHBGqKcUT0iMA@mail.gmail.com/
> > >
> > > That's different. That's the actual UAF inside bpf_skc_to_unix_sock().
> > > That needs to be fixed with what I described above by allowing
> > > only trusted pointers into helpers.
> >
> > I got this point.
> >
> > > In my earlier reply I meant that:
> > > if (u->peer->__sk_common.skc_family) # unsafe access
> > > is fine. It's safe access, because it's sanitized by the verifier/JIT.
> >
> > but I'm a bit confused here..
> >
> > Even though the verifier makes sure the pointer is not NULL,
> > u->peer can be freed concurrently by another thread,
> > then how can verifier/JIT prevent UAF ?
> >
> > Or can this be still called 'safe access' regardless of runtime UAF
> > because the user-provided bpf prog was verified once and not in-tree
> > kernel code ?
>
> The verifier/JIT are not preventing UAF. They're preventing crashes.
> u->peer->__sk_common.skc_family is equivalent to two
> copy_from_kernel_nofault(). They may read garbage, but won't crash
> and won't cause any ill kernel effects. It's up to the program
> to use the values in a sane way.

I see, that's fair.  Thanks for explaining !

Powered by blists - more mailing lists