Re: [PATCH v7 bpf-next 09/10] bpf: wire up sleepable bpf_get_stack() and bpf_get_task_stack() helpers

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Mon, Nov 11, 2024 at 5:29 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (24/11/11 09:49), Andrii Nakryiko wrote:
> > > On (24/08/29 10:42), Andrii Nakryiko wrote:
> > > > Now that build ID related internals in kernel/bpf/stackmap.c can be used
> > > > both in sleepable and non-sleepable contexts, we need to add additional
> > > > rcu_read_lock()/rcu_read_unlock() protection around fetching
> > > > perf_callchain_entry, but with the refactoring in previous commit it's
> > > > now pretty straightforward. We make sure to do rcu_read_unlock (in
> > > > sleepable mode only) right before stack_map_get_build_id_offset() call
> > > > which can sleep. By that time we don't have any more use of
> > > > perf_callchain_entry.
> > >
> > > Shouldn't this be backported to stable kernels?  It seems that those still
> > > do suspicious-RCU deference:
> > >
> > > __bpf_get_stack()
> > >   get_perf_callchain()
> > >     perf_callchain_user()
> > >       perf_get_guest_cbs()
> >
> > Do you see this issue in practice or have some repro?
> > __bpf_get_stack() shouldn't be callable from sleepable BPF programs
> > until my patch set, so I don't think there is anything to be
> > backported. But maybe I'm missing something, which is why I'm asking
> > whether this is a conclusion drawn from source code analysis, or there
> > was actually a report somewhere.
>
> I see a syzkaller report (internal) which triggers this call chain
> and RCU-usage error.  Not sure how practical that is, but syzkaller
> was able to hit it (the report I'm looking at is against 5.15, but
> __bpf_get_stack()-wise I don't see any differences between 5.15,
> 6.1 and 6.6)

Hmm.. thinking about this some more, I suspect we do allow
bpf_get_stack() and bpf_get_stackid() from sleepable uprobes, so yeah,
it's possible to run into this.

But for backporting this into older kernels, we'd need to prepare a
separate patch that would fix the RCU issue, but wouldn't add
sleepable build ID parts.