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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 May 2020 18:39:57 -0700 From: Yonghong Song <yhs@...com> To: "Daniel T. Lee" <danieltimlee@...il.com>, Daniel Borkmann <daniel@...earbox.net>, Alexei Starovoitov <ast@...nel.org> CC: <netdev@...r.kernel.org>, <bpf@...r.kernel.org>, Andrii Nakryiko <andrii.nakryiko@...il.com>, John Fastabend <john.fastabend@...il.com> Subject: Re: [PATCH bpf-next 1/3] samples: bpf: refactor kprobe tracing user progs with libbpf On 5/12/20 7:43 AM, Daniel T. Lee wrote: > Currently, the kprobe BPF program attachment method for bpf_load is > quite old. The implementation of bpf_load "directly" controls and > manages(create, delete) the kprobe events of DEBUGFS. On the other hand, > using using the libbpf automatically manages the kprobe event. > (under bpf_link interface) > > By calling bpf_program__attach(_kprobe) in libbpf, the corresponding > kprobe is created and the BPF program will be attached to this kprobe. > To remove this, by simply invoking bpf_link__destroy will clean up the > event. > > This commit refactors kprobe tracing programs (tracex{1~7}_user.c) with > libbpf using bpf_link interface and bpf_program__attach. > > tracex2_kern.c, which tracks system calls (sys_*), has been modified to > append prefix depending on architecture. > > Signed-off-by: Daniel T. Lee <danieltimlee@...il.com> > --- > samples/bpf/Makefile | 12 +++---- > samples/bpf/tracex1_user.c | 41 ++++++++++++++++++++---- > samples/bpf/tracex2_kern.c | 8 ++++- > samples/bpf/tracex2_user.c | 55 ++++++++++++++++++++++++++------ > samples/bpf/tracex3_user.c | 65 ++++++++++++++++++++++++++++---------- > samples/bpf/tracex4_user.c | 55 +++++++++++++++++++++++++------- > samples/bpf/tracex6_user.c | 53 +++++++++++++++++++++++++++---- > samples/bpf/tracex7_user.c | 43 ++++++++++++++++++++----- > 8 files changed, 268 insertions(+), 64 deletions(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 424f6fe7ce38..4c91e5914329 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -64,13 +64,13 @@ fds_example-objs := fds_example.o > sockex1-objs := sockex1_user.o > sockex2-objs := sockex2_user.o > sockex3-objs := bpf_load.o sockex3_user.o > -tracex1-objs := bpf_load.o tracex1_user.o $(TRACE_HELPERS) > -tracex2-objs := bpf_load.o tracex2_user.o > -tracex3-objs := bpf_load.o tracex3_user.o > -tracex4-objs := bpf_load.o tracex4_user.o > +tracex1-objs := tracex1_user.o $(TRACE_HELPERS) > +tracex2-objs := tracex2_user.o > +tracex3-objs := tracex3_user.o > +tracex4-objs := tracex4_user.o > tracex5-objs := bpf_load.o tracex5_user.o $(TRACE_HELPERS) > -tracex6-objs := bpf_load.o tracex6_user.o > -tracex7-objs := bpf_load.o tracex7_user.o > +tracex6-objs := tracex6_user.o > +tracex7-objs := tracex7_user.o > test_probe_write_user-objs := bpf_load.o test_probe_write_user_user.o > trace_output-objs := bpf_load.o trace_output_user.o $(TRACE_HELPERS) > lathist-objs := bpf_load.o lathist_user.o > diff --git a/samples/bpf/tracex1_user.c b/samples/bpf/tracex1_user.c > index 55fddbd08702..1b15ab98f7d3 100644 > --- a/samples/bpf/tracex1_user.c > +++ b/samples/bpf/tracex1_user.c > @@ -1,21 +1,45 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <stdio.h> > -#include <linux/bpf.h> > #include <unistd.h> > -#include <bpf/bpf.h> > -#include "bpf_load.h" > +#include <bpf/libbpf.h> > #include "trace_helpers.h" > > +#define __must_check This is not very user friendly. Maybe not including linux/err.h and use libbpf API libbpf_get_error() instead? > +#include <linux/err.h> > + > int main(int ac, char **argv) > { > - FILE *f; > + struct bpf_link *link = NULL; > + struct bpf_program *prog; > + struct bpf_object *obj; > char filename[256]; > + FILE *f; > > snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); > + obj = bpf_object__open_file(filename, NULL); > + if (IS_ERR(obj)) { > + fprintf(stderr, "ERROR: opening BPF object file failed\n"); > + obj = NULL; > + goto cleanup; You do not need to goto cleanup, directly return 0 is okay here. The same for other files in this patch. > + } > + > + prog = bpf_object__find_program_by_name(obj, "bpf_prog1"); > + if (!prog) { > + fprintf(stderr, "ERROR: finding a prog in obj file failed\n"); > + goto cleanup; > + } > + > + /* load BPF program */ > + if (bpf_object__load(obj)) { > + fprintf(stderr, "ERROR: loading BPF object file failed\n"); > + goto cleanup; > + } > > - if (load_bpf_file(filename)) { > - printf("%s", bpf_log_buf); > - return 1; > + link = bpf_program__attach(prog); > + if (IS_ERR(link)) { > + fprintf(stderr, "ERROR: bpf_program__attach failed\n"); > + link = NULL; > + goto cleanup; > } > > f = popen("taskset 1 ping -c5 localhost", "r"); > @@ -23,5 +47,8 @@ int main(int ac, char **argv) > > read_trace_pipe(); > > +cleanup: > + bpf_link__destroy(link); > + bpf_object__close(obj); Typically in kernel, we do multiple labels for such cases like destroy_link: bpf_link__destroy(link); close_object: bpf_object__close(obj); The error path in the main() function jumps to proper label. This is more clean and less confusion. The same for other cases in this file. > return 0; > } > diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c > index d865bb309bcb..ff5d00916733 100644 > --- a/samples/bpf/tracex2_kern.c > +++ b/samples/bpf/tracex2_kern.c > @@ -11,6 +11,12 @@ > #include <bpf/bpf_helpers.h> > #include <bpf/bpf_tracing.h> > > +#ifdef __x86_64__ > +#define SYSCALL "__x64_" > +#else > +#define SYSCALL > +#endif See test_progs.h, one more case to handle: #ifdef __x86_64__ #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep" #elif defined(__s390x__) #define SYS_NANOSLEEP_KPROBE_NAME "__s390x_sys_nanosleep" #else #define SYS_NANOSLEEP_KPROBE_NAME "sys_nanosleep" #endif > + > struct bpf_map_def SEC("maps") my_map = { > .type = BPF_MAP_TYPE_HASH, > .key_size = sizeof(long), > @@ -77,7 +83,7 @@ struct bpf_map_def SEC("maps") my_hist_map = { > .max_entries = 1024, > }; > > -SEC("kprobe/sys_write") > +SEC("kprobe/" SYSCALL "sys_write") > int bpf_prog3(struct pt_regs *ctx) > { > long write_size = PT_REGS_PARM3(ctx); [...]
Powered by blists - more mailing lists