Re: [PATCH bpf-next 2/3] selftests/bpf: Add uretprobe syscall test for regs integrity

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

Hi Jiri,

On Sun, 31 Mar 2024 21:37:03 +0200
Jiri Olsa <olsajiri@gmail.com> wrote:

> On Fri, Mar 29, 2024 at 03:30:11PM -0700, Andrii Nakryiko wrote:
> > On Wed, Mar 27, 2024 at 3:21 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Add uretprobe syscall test that compares register values before
> > > and after the uretprobe is hit. It also compares the register
> > > values seen from attached bpf program.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/include/linux/compiler.h                |   4 +
> > >  .../selftests/bpf/prog_tests/uprobe_syscall.c | 163 ++++++++++++++++++
> > >  .../selftests/bpf/progs/uprobe_syscall.c      |  15 ++
> > >  3 files changed, 182 insertions(+)
> > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > >  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> > 
> > [...]
> > 
> > > +__naked unsigned long uprobe_syscall_arch_test(void)
> > > +{
> > > +       asm volatile (
> > > +               "movq $0xdeadbeef, %rax\n"
> > > +               "ret\n"
> > > +       );
> > > +}
> > > +
> > > +__naked void uprobe_syscall_arch(struct pt_regs *before, struct pt_regs *after)
> > 
> > don't you get compiler warnings for using __naked with explicit
> > function arguments?
> 
> nope, both gcc and clang are silent
> 
> > 
> > > +{
> > > +       asm volatile (
> > > +               "movq %r15,   0(%rdi)\n"
> > > +               "movq %r14,   8(%rdi)\n"
> > 
> > [...]
> > 
> > > +       err = uprobe_syscall__attach(skel);
> > > +       if (!ASSERT_OK(err, "uprobe_syscall__attach"))
> > > +               goto cleanup;
> > > +
> > > +       uprobe_syscall_arch(&before, &after);
> > 
> > uprobe_syscall_arch() doesn't really do an explicit `syscall
> > uretprobe`, it should work for int3-based uretprobes as well? Let's
> > call it something a bit more generic then?
> 
> ok, how about
> 
>   uprobe_syscall_arch -> uretprobe_regs
>   uprobe_syscall_arch_test -> uretprobe_regs_trigger
> 
> > 
> > Also, I think patch #1 will go through Masami's trace tree, right? But
> > we can land selftests into bpf-next even before that, given they
> > should work for both syscall and interrupt based uretprobes.
> 
> hm, not sure.. I did not originally cc Masami/Steven :-\  adding now

Would you mean this patch?

https://lore.kernel.org/all/20240327102036.543283-2-jolsa@kernel.org/

It seems you don't Cc/To me nor linux-kernel-trace ML.

> 
> Masami, could patch 1 go through:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
>   probes/for-next

Could you resend it to linux-kernel-trace ML?  (only the first one?)

Thank you,

> 
> thanks,
> jirka


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>