linux-kernel - Re: [PATCH bpf-next v3 1/2] arm64: stacktrace: Implement arch_bpf_stack

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAADnVQLGGTshMiQAWwJ9UzrEVDR4Z8yk+ki9pUqKLgcH0DRAjA@mail.gmail.com>
Date: Thu, 22 Feb 2024 18:04:35 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: Puranjay Mohan <puranjay12@...il.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Zi Shen Lim <zlim.lnx@...il.com>, Will Deacon <will@...nel.org>, 
	bpf <bpf@...r.kernel.org>, 
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, LKML <linux-kernel@...r.kernel.org>, 
	Kumar Kartikeya Dwivedi <memxor@...il.com>
Subject: Re: [PATCH bpf-next v3 1/2] arm64: stacktrace: Implement
 arch_bpf_stack_walk() for the BPF JIT

On Thu, Feb 22, 2024 at 11:28 AM Catalin Marinas
<catalin.marinas@....com> wrote:
>
> On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote:
> > This will be used by bpf_throw() to unwind till the program marked as
> > exception boundary and run the callback with the stack of the main
> > program.
> >
> > This is required for supporting BPF exceptions on ARM64.
> >
> > Signed-off-by: Puranjay Mohan <puranjay12@...il.com>
> > ---
> >  arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> > index 7f88028a00c0..66cffc5fc0be 100644
> > --- a/arch/arm64/kernel/stacktrace.c
> > +++ b/arch/arm64/kernel/stacktrace.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/efi.h>
> >  #include <linux/export.h>
> > +#include <linux/filter.h>
> >  #include <linux/ftrace.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/sched.h>
> > @@ -266,6 +267,31 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry,
> >       kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs);
> >  }
> >
> > +struct bpf_unwind_consume_entry_data {
> > +     bool (*consume_entry)(void *cookie, u64 ip, u64 sp, u64 fp);
> > +     void *cookie;
> > +};
> > +
> > +static bool
> > +arch_bpf_unwind_consume_entry(const struct kunwind_state *state, void *cookie)
> > +{
> > +     struct bpf_unwind_consume_entry_data *data = cookie;
> > +
> > +     return data->consume_entry(data->cookie, state->common.pc, 0,
> > +                                state->common.fp);
> > +}
> > +
> > +noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *cookie, u64 ip, u64 sp,
> > +                                                             u64 fp), void *cookie)
> > +{
> > +     struct bpf_unwind_consume_entry_data data = {
> > +             .consume_entry = consume_entry,
> > +             .cookie = cookie,
> > +     };
> > +
> > +     kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL);
> > +}
>
> Too many "cookies", I found reading this confusing. If you ever respin,
> please use some different "cookie" names.
>
> I guess you want this to be merged via the bpf tree?

We typically take bpf jit patches through bpf-next, since
we do cross arch jits refactoring from time to time,
but nothing like this is pending for this merge window,
so if you want it to go through arm64 tree that's fine with us.

> Acked-by: Catalin Marinas <catalin.marinas@....com>

Thank you for the review!