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

[Jiri Olsa <olsajiri@gmail.com>]

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

Masami, could patch 1 go through:

  https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
  probes/for-next

thanks,
jirka