Re: [PATCH v2 bpf-next 5/5] selftests/bpf: use BPF_KSYSCALL and SEC("ksyscall") in selftests

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Thu, 2022-07-14 at 00:07 -0700, Andrii Nakryiko wrote:
> Convert few selftest that used plain SEC("kprobe") with arch-specific
> syscall wrapper prefix to ksyscall/kretsyscall and corresponding
> BPF_KSYSCALL macro. test_probe_user.c is especially benefiting from
> this
> simplification.
> 
> Tested-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  .../selftests/bpf/progs/bpf_syscall_macro.c   |  6 ++---
>  .../selftests/bpf/progs/test_attach_probe.c   | 15 +++++------
>  .../selftests/bpf/progs/test_probe_user.c     | 27 +++++------------
> --
>  3 files changed, 16 insertions(+), 32 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> index 05838ed9b89c..e1e11897e99b 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> @@ -64,9 +64,9 @@ int BPF_KPROBE(handle_sys_prctl)
>         return 0;
>  }
>  
> -SEC("kprobe/" SYS_PREFIX "sys_prctl")
> -int BPF_KPROBE_SYSCALL(prctl_enter, int option, unsigned long arg2,
> -                      unsigned long arg3, unsigned long arg4,
> unsigned long arg5)
> +SEC("ksyscall/prctl")
> +int BPF_KSYSCALL(prctl_enter, int option, unsigned long arg2,
> +                unsigned long arg3, unsigned long arg4, unsigned
> long arg5)
>  {
>         pid_t pid = bpf_get_current_pid_tgid() >> 32;
>  
> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c
> b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> index f1c88ad368ef..a1e45fec8938 100644
> --- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> @@ -1,11 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2017 Facebook
>  
> -#include <linux/ptrace.h>
> -#include <linux/bpf.h>
> +#include "vmlinux.h"
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_tracing.h>
> -#include <stdbool.h>
> +#include <bpf/bpf_core_read.h>
>  #include "bpf_misc.h"
>  
>  int kprobe_res = 0;
> @@ -31,8 +30,8 @@ int handle_kprobe(struct pt_regs *ctx)
>         return 0;
>  }
>  
> -SEC("kprobe/" SYS_PREFIX "sys_nanosleep")
> -int BPF_KPROBE(handle_kprobe_auto)
> +SEC("ksyscall/nanosleep")
> +int BPF_KSYSCALL(handle_kprobe_auto, struct __kernel_timespec *req,
> struct __kernel_timespec *rem)
>  {
>         kprobe2_res = 11;
>         return 0;
> @@ -56,11 +55,11 @@ int handle_kretprobe(struct pt_regs *ctx)
>         return 0;
>  }
>  
> -SEC("kretprobe/" SYS_PREFIX "sys_nanosleep")
> -int BPF_KRETPROBE(handle_kretprobe_auto)
> +SEC("kretsyscall/nanosleep")
> +int BPF_KRETPROBE(handle_kretprobe_auto, int ret)
>  {
>         kretprobe2_res = 22;
> -       return 0;
> +       return ret;
>  }
>  
>  SEC("uprobe")
> diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c
> b/tools/testing/selftests/bpf/progs/test_probe_user.c
> index 702578a5e496..8e1495008e4d 100644
> --- a/tools/testing/selftests/bpf/progs/test_probe_user.c
> +++ b/tools/testing/selftests/bpf/progs/test_probe_user.c
> @@ -1,35 +1,20 @@
>  // SPDX-License-Identifier: GPL-2.0
> -
> -#include <linux/ptrace.h>
> -#include <linux/bpf.h>
> -
> -#include <netinet/in.h>
> -
> +#include "vmlinux.h"
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_core_read.h>
>  #include "bpf_misc.h"
>  
>  static struct sockaddr_in old;
>  
> -SEC("kprobe/" SYS_PREFIX "sys_connect")
> -int BPF_KPROBE(handle_sys_connect)
> +SEC("ksyscall/connect")
> +int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in
> *uservaddr, int addrlen)
>  {
> -#if SYSCALL_WRAPPER == 1
> -       struct pt_regs *real_regs;
> -#endif
>         struct sockaddr_in new;
> -       void *ptr;
> -
> -#if SYSCALL_WRAPPER == 0
> -       ptr = (void *)PT_REGS_PARM2(ctx);
> -#else
> -       real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx);
> -       bpf_probe_read_kernel(&ptr, sizeof(ptr),
> &PT_REGS_PARM2(real_regs));
> -#endif
>  
> -       bpf_probe_read_user(&old, sizeof(old), ptr);
> +       bpf_probe_read_user(&old, sizeof(old), uservaddr);
>         __builtin_memset(&new, 0xab, sizeof(new));
> -       bpf_probe_write_user(ptr, &new, sizeof(new));
> +       bpf_probe_write_user(uservaddr, &new, sizeof(new));
>  
>         return 0;
>  }

Hi,

The first two tests succeed, but test_probe_user fails on s390x with:

    serial_test_probe_user:FAIL:check_kprobe_res
    wrong kprobe res from probe read: 0.0.0.0:0

I'm not sure what is causing this, but at least the loaded BPF code
looks sane:

# bpftool prog dump xlated id 81
int handle_sys_connect(struct pt_regs * ctx):
   0: (bf) r6 = r1                      ; kernel pt_regs
   1: (18) r1 = map[id:33][0]+0
   3: (71) r1 = *(u8 *)(r1 +0)
   4: (79) r6 = *(u64 *)(r6 +40)        ; kernel gpr2
                                        ; PT_REGS_PARM1
   5: (b7) r1 = 152
   6: (bf) r3 = r6
   7: (0f) r3 += r1                     ; user orig_gpr2
                                        ; PT_REGS_PARM1_CORE_SYSCALL
                                        ; fd
   8: (bf) r1 = r10
   9: (07) r1 += -16
  10: (b4) w2 = 8
  11: (bc) w2 = w2
  12: (85) call bpf_probe_read_kernel#-76640 ; (&tmp, 8, &fd)
  13: (b7) r1 = 48
  14: (bf) r3 = r6
  15: (0f) r3 += r1                     ; user gpr3
                                        ; __PT_PARM2_REG
                                        ; uservaddr
  16: (bf) r1 = r10
  17: (07) r1 += -16
  18: (b4) w2 = 8
  19: (bc) w2 = w2
  20: (85) call bpf_probe_read_kernel#-76640 (&tmp, 8, &uservaddr)
  21: (b7) r1 = 56
  22: (0f) r6 += r1                     ; user gpr4 
                                        ; __PT_PARM3_REG
                                        ; addrlen
  23: (79) r7 = *(u64 *)(r10 -16)       ; uservaddr
  24: (bf) r1 = r10
  25: (07) r1 += -16
  26: (b4) w2 = 8
  27: (bc) w2 = w2
  28: (bf) r3 = r6
  29: (85) call bpf_probe_read_kernel#-76640 (&tmp, 8, &addrlen)
  30: (18) r1 = map[id:32][0]+0         ; &old
  32: (b4) w2 = 16
  33: (bc) w2 = w2
  34: (bf) r3 = r7
  35: (85) call bpf_probe_read_user#-76928 (&old, 16, uservaddr)
  36: (18) r1 = 0xabababababababab
  38: (7b) *(u64 *)(r10 -16) = r1       ; memset(&new, 0xab, 16)
  39: (7b) *(u64 *)(r10 -8) = r1
  40: (bf) r2 = r10
  41: (07) r2 += -16
  42: (bf) r1 = r7
  43: (b4) w3 = 16
  44: (bc) w3 = w3
  45: (85) call bpf_probe_write_user#-76352 (uservaddr, &new, 16)
  46: (b4) w0 = 0
  47: (bc) w0 = w0
  48: (95) exit

Best regards,
Ilya