netdev - Re: [bpf-next v2 0/5] samples/bpf: enhance syscall tracing program

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] [day] [month] [year] [list]

Date:   Tue, 20 Dec 2022 14:40:58 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     "Daniel T. Lee" <danieltimlee@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Yonghong Song <yhs@...com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [bpf-next v2 0/5] samples/bpf: enhance syscall tracing program

On Tue, Dec 20, 2022 at 3:59 AM Daniel T. Lee <danieltimlee@...il.com> wrote:
>
> Syscall tracing using kprobe is quite unstable. Since it uses the exact
> name of the kernel function, the program might broke due to the rename
> of a function. The problem can also be caused by a changes in the
> arguments of the function to which the kprobe connects. This commit
> enhances syscall tracing program with the following instruments.
>
> In this patchset, ksyscall is used instead of kprobe. By using
> ksyscall, libbpf will detect the appropriate kernel function name.
> (e.g. sys_write -> __s390_sys_write). This eliminates the need to worry
> about which wrapper function to attach in order to parse arguments.
> Also ksyscall provides more fine method with attaching system call, the
> coarse SYSCALL helper at trace_common.h can be removed.
>
> Next, BPF_SYSCALL is used to reduce the inconvenience of parsing
> arguments. Since the nature of SYSCALL_WRAPPER function wraps the
> argument once, additional process of argument extraction is required
> to properly parse the argument. The BPF_SYSCALL macro will reduces the
> hassle of parsing arguments from pt_regs.
>
> Lastly, vmlinux.h is applied to syscall tracing program. This change
> allows the bpf program to refer to the internal structure as a single
> "vmlinux.h" instead of including each header referenced by the bpf
> program.
>
> Additionally, this patchset changes the suffix of _kern to .bpf to make
> use of the new compile rule (CLANG-BPF) which is more simple and neat.
> By just changing the _kern suffix to .bpf will inherit the benefit of
> the new CLANG-BPF compile target.
>
> Also, this commit adds dummy gnu/stub.h to the samples/bpf directory.
> This will fix the compiling problem with 'clang -target bpf'.
>
> ---
> Changes in V2:
> - add gnu/stub.h hack to fix compile error with 'clang -target bpf'
>
> Daniel T. Lee (5):
>   samples/bpf: use kyscall instead of kprobe in syscall tracing program
>   samples/bpf: use vmlinux.h instead of implicit headers in syscall
>     tracing program
>   samples/bpf: change _kern suffix to .bpf with syscall tracing program
>   samples/bpf: fix tracex2 by using BPF_KSYSCALL macro
>   samples/bpf: use BPF_KSYSCALL macro in syscall tracing programs
>

Nice set of changes, thanks for cleaning these up! I don't see
anything obviously wrong, but these changes seem to break s390x build
(see [0]), please check what's going on.

  [0] https://github.com/kernel-patches/bpf/actions/runs/3740339876/jobs/6348606866


>  samples/bpf/Makefile                          | 10 ++--
>  samples/bpf/gnu/stubs.h                       |  1 +
>  ...p_perf_test_kern.c => map_perf_test.bpf.c} | 48 ++++++++-----------
>  samples/bpf/map_perf_test_user.c              |  2 +-
>  ...c => test_current_task_under_cgroup.bpf.c} | 11 ++---
>  .../bpf/test_current_task_under_cgroup_user.c |  2 +-
>  samples/bpf/test_map_in_map_kern.c            |  1 -
>  ...ser_kern.c => test_probe_write_user.bpf.c} | 20 ++++----
>  samples/bpf/test_probe_write_user_user.c      |  2 +-
>  samples/bpf/trace_common.h                    | 13 -----
>  ...trace_output_kern.c => trace_output.bpf.c} |  6 +--
>  samples/bpf/trace_output_user.c               |  2 +-
>  samples/bpf/{tracex2_kern.c => tracex2.bpf.c} | 13 ++---
>  samples/bpf/tracex2_user.c                    |  2 +-
>  14 files changed, 52 insertions(+), 81 deletions(-)
>  create mode 100644 samples/bpf/gnu/stubs.h
>  rename samples/bpf/{map_perf_test_kern.c => map_perf_test.bpf.c} (85%)
>  rename samples/bpf/{test_current_task_under_cgroup_kern.c => test_current_task_under_cgroup.bpf.c} (84%)
>  rename samples/bpf/{test_probe_write_user_kern.c => test_probe_write_user.bpf.c} (71%)
>  delete mode 100644 samples/bpf/trace_common.h
>  rename samples/bpf/{trace_output_kern.c => trace_output.bpf.c} (82%)
>  rename samples/bpf/{tracex2_kern.c => tracex2.bpf.c} (89%)
>
> --
> 2.34.1
>