On Mon, 2023-01-16 at 23:09 +0100, Ilya Leoshkevich wrote:
> On Fri, 2023-01-13 at 00:33 -0800, Andrii Nakryiko wrote:
> > This patch set fixes and extends libbpf's bpf_tracing.h support for
> > tracing
> > arguments of kprobes/uprobes, and syscall as a special case.
> >
> > Depending on the architecture, anywhere between 3 and 8 arguments
> > can
> > be
> > passed to a function in registers (so relevant to kprobes and
> > uprobes), but
> > before this patch set libbpf's macros in bpf_tracing.h only
> > supported
> > up to
> > 5 arguments, which is limiting in practice. This patch set extends
> > bpf_tracing.h to support up to 8 arguments, if architecture allows.
> > This
> > includes explicit PT_REGS_PARMx() macro family, as well as
> > BPF_KPROBE() macro.
> >
> > Now, with tracing syscall arguments situation is sometimes quite
> > different.
> > For a lot of architectures syscall argument passing through
> > registers
> > differs
> > from function call sequence at least a little. For i386 it differs
> > *a
> > lot*.
> > This patch set addresses this issue across all currently supported
> > architectures and hopefully fixes existing issues. syscall(2)
> > manpage
> > defines
> > that either 6 or 7 arguments can be supported, depending on
> > architecture, so
> > libbpf defines 6 or 7 registers per architecture to be used to
> > fetch
> > syscall
> > arguments.
> >
> > Also, BPF_UPROBE and BPF_URETPROBE are introduced as part of this
> > patch set.
> > They are aliases for BPF_KPROBE and BPF_KRETPROBE (as mechanics of
> > argument
> > fetching of kernel functions and user-space functions are
> > identical),
> > but it
> > allows BPF users to have less confusing BPF-side code when working
> > with
> > uprobes.
> >
> > For both sets of changes selftests are extended to test these new
> > register
> > definitions to architecture-defined limits. Unfortunately I don't
> > have ability
> > to test it on all architectures, and BPF CI only tests 3
> > architecture
> > (x86-64,
> > arm64, and s390x), so it would be greatly appreciated if CC'ed
> > people
> > can help
> > review and test changes on architectures they are familiar with
> > (and
> > maybe
> > have direct access to for testing). Thank you.
> >
> > Cc: Alan Maguire <alan.maguire@xxxxxxxxxx>
> > Cc: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > Cc: Pu Lehui <pulehui@xxxxxxxxxx>
> > Cc: Hengqi Chen <hengqi.chen@xxxxxxxxx>
> > Cc: Vladimir Isaev <isaev@xxxxxxxxxxxx>
> > Cc: Björn Töpel <bjorn@xxxxxxxxxx>
> > Cc: Kenta Tada <Kenta.Tada@xxxxxxxx>
> > Cc: Florent Revest <revest@xxxxxxxxxxxx>
> >
> > Andrii Nakryiko (25):
> > libbpf: add support for fetching up to 8 arguments in kprobes
> > libbpf: add 6th argument support for x86-64 in bpf_tracing.h
> > libbpf: fix arm and arm64 specs in bpf_tracing.h
> > libbpf: complete mips spec in bpf_tracing.h
> > libbpf: complete powerpc spec in bpf_tracing.h
> > libbpf: complete sparc spec in bpf_tracing.h
> > libbpf: complete riscv arch spec in bpf_tracing.h
> > libbpf: fix and complete ARC spec in bpf_tracing.h
> > libbpf: complete LoongArch (loongarch) spec in bpf_tracing.h
> > libbpf: add BPF_UPROBE and BPF_URETPROBE macro aliases
> > selftests/bpf: validate arch-specific argument registers limits
> > libbpf: improve syscall tracing support in bpf_tracing.h
> > libbpf: define x86-64 syscall regs spec in bpf_tracing.h
> > libbpf: define i386 syscall regs spec in bpf_tracing.h
> > libbpf: define s390x syscall regs spec in bpf_tracing.h
> > libbpf: define arm syscall regs spec in bpf_tracing.h
> > libbpf: define arm64 syscall regs spec in bpf_tracing.h
> > libbpf: define mips syscall regs spec in bpf_tracing.h
> > libbpf: define powerpc syscall regs spec in bpf_tracing.h
> > libbpf: define sparc syscall regs spec in bpf_tracing.h
> > libbpf: define riscv syscall regs spec in bpf_tracing.h
> > libbpf: define arc syscall regs spec in bpf_tracing.h
> > libbpf: define loongarch syscall regs spec in bpf_tracing.h
> > selftests/bpf: add 6-argument syscall tracing test
> > libbpf: clean up now not needed __PT_PARM{1-6}_SYSCALL_REG
> > defaults
> >
> > tools/lib/bpf/bpf_tracing.h | 301
> > +++++++++++++++-
> > --
> > .../bpf/prog_tests/test_bpf_syscall_macro.c | 18 +-
> > .../bpf/prog_tests/uprobe_autoattach.c | 33 +-
> > tools/testing/selftests/bpf/progs/bpf_misc.h | 25 ++
> > .../selftests/bpf/progs/bpf_syscall_macro.c | 26 ++
> > .../bpf/progs/test_uprobe_autoattach.c | 48 ++-
> > 6 files changed, 405 insertions(+), 46 deletions(-)
> >
>
> With the following fixup for 24/25:
>
> diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> index c07c5c52d5fc..75ccdef9bc78 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> @@ -88,15 +88,33 @@ __u64 mmap_flags;
> __u64 mmap_fd;
> __u64 mmap_offset;
>
> +#if defined(bpf_target_s390)
> +SEC("ksyscall/old_mmap")
> +#else
> SEC("ksyscall/mmap")
> +#endif
> int BPF_KSYSCALL(mmap_enter, void *addr, size_t length, int prot,
> int
> flags,
> int fd, off_t offset)
> {
> pid_t pid = bpf_get_current_pid_tgid() >> 32;
> +#if defined(bpf_target_s390)
> + unsigned long args[6] = {0};
> +#endif
>
> if (pid != filter_pid)
> return 0;
>
> +#if defined(bpf_target_s390)
> + /* For __ARCH_WANT_SYS_OLD_MMAP, parse mmap_arg_struct. */
> + bpf_probe_read_user(args, sizeof(args), addr);
> + addr = (void *)args[0];
> + length = (size_t)args[1];
> + prot = (int)args[2];
> + flags = (int)args[3];
> + fd = (int)args[4];
> + offset = (off_t)args[5];
> +#endif
> +
> mmap_addr = (__u64)addr;
> mmap_length = length;
> mmap_prot = prot;
>
> # ./test_progs -t syscall_macro
> #19 bpf_syscall_macro:OK
>
> Tested-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx> # s390x
While the above fixup works, I realized that it's ugly. It's better to
admit that mmap and old_mmap are different syscalls and create a
different probe, even if it means duplicating pid filtering code:
diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
index c07c5c52d5fc..2420cad54d36 100644
--- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
+++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
@@ -88,6 +88,29 @@ __u64 mmap_flags;
__u64 mmap_fd;
__u64 mmap_offset;
+#if defined(bpf_target_s390)
+/* Attach to old_mmap on architectures with __ARCH_WANT_SYS_OLD_MMAP.
*/
+SEC("ksyscall/old_mmap")
+int BPF_KSYSCALL(old_mmap_enter, void *arg)
+{
+ pid_t pid = bpf_get_current_pid_tgid() >> 32;
+ unsigned long a[6] = {0};
+
+ if (pid != filter_pid)
+ return 0;
+
+ /* Parse mmap_arg_struct. */
+ bpf_probe_read_user(a, sizeof(a), arg);
+ mmap_addr = (__u64)(void *)a[0];
+ mmap_length = (size_t)a[1];
+ mmap_prot = (int)a[2];
+ mmap_flags = (int)a[3];
+ mmap_fd = (int)a[4];
+ mmap_offset = (off_t)a[5];
+
+ return 0;
+}
+#else
SEC("ksyscall/mmap")
int BPF_KSYSCALL(mmap_enter, void *addr, size_t length, int prot, int
flags,
int fd, off_t offset)
@@ -106,5 +129,6 @@ int BPF_KSYSCALL(mmap_enter, void *addr, size_t
length, int prot, int flags,
return 0;
}
+#endif
char _license[] SEC("license") = "GPL";
