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, 4 Aug 2020 23:40:05 -0700 From: Andrii Nakryiko <andrii.nakryiko@...il.com> To: Jiri Olsa <jolsa@...nel.org> Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andriin@...com>, Wenbo Zhang <ethercflow@...il.com>, Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, Martin KaFai Lau <kafai@...com>, David Miller <davem@...hat.com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...omium.org>, Brendan Gregg <bgregg@...flix.com>, Florent Revest <revest@...omium.org>, Al Viro <viro@...iv.linux.org.uk> Subject: Re: [PATCH v9 bpf-next 13/14] selftests/bpf: Add test for d_path helper On Sat, Aug 1, 2020 at 10:05 AM Jiri Olsa <jolsa@...nel.org> wrote: > > Adding test for d_path helper which is pretty much > copied from Wenbo Zhang's test for bpf_get_fd_path, > which never made it in. > > The test is doing fstat/close on several fd types, > and verifies we got the d_path helper working on > kernel probes for vfs_getattr/filp_close functions. > > Original-patch-by: Wenbo Zhang <ethercflow@...il.com> > Signed-off-by: Jiri Olsa <jolsa@...nel.org> > --- Nothing wrong with this BPF implementation, but seem one suggestion below. Acked-by: Andrii Nakryiko <andriin@...com> > .../testing/selftests/bpf/prog_tests/d_path.c | 147 ++++++++++++++++++ > .../testing/selftests/bpf/progs/test_d_path.c | 64 ++++++++ > 2 files changed, 211 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path.c > create mode 100644 tools/testing/selftests/bpf/progs/test_d_path.c > [...] > diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c > new file mode 100644 > index 000000000000..9d342d7a1de6 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_d_path.c > @@ -0,0 +1,64 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include "vmlinux.h" > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > + > +#define MAX_PATH_LEN 128 > +#define MAX_FILES 7 > + > +pid_t my_pid = 0; > +__u32 cnt_stat = 0; > +__u32 cnt_close = 0; > +char paths_stat[MAX_FILES][MAX_PATH_LEN] = {}; > +char paths_close[MAX_FILES][MAX_PATH_LEN] = {}; > +int rets_stat[MAX_FILES] = {}; > +int rets_close[MAX_FILES] = {}; > + > +SEC("fentry/vfs_getattr") > +int BPF_PROG(prog_stat, struct path *path, struct kstat *stat, > + __u32 request_mask, unsigned int query_flags) > +{ > + pid_t pid = bpf_get_current_pid_tgid() >> 32; > + int ret; > + > + if (pid != my_pid) > + return 0; > + > + if (cnt_stat >= MAX_FILES) > + return 0; > + ret = bpf_d_path(path, paths_stat[cnt_stat], MAX_PATH_LEN); > + > + /* We need to recheck cnt_stat for verifier. */ > + if (cnt_stat >= MAX_FILES) > + return 0; > + rets_stat[cnt_stat] = ret; > + > + cnt_stat++; > + return 0; > +} > + > +SEC("fentry/filp_close") > +int BPF_PROG(prog_close, struct file *file, void *id) > +{ > + pid_t pid = bpf_get_current_pid_tgid() >> 32; > + int ret; > + > + if (pid != my_pid) > + return 0; > + > + if (cnt_close >= MAX_FILES) > + return 0; > + ret = bpf_d_path(&file->f_path, > + paths_close[cnt_close], MAX_PATH_LEN); > + > + /* We need to recheck cnt_stat for verifier. */ you need to do it because you are re-reading a global variable; if you stored cnt_close in a local variable, then did >= MAX_FILES check once, you probably could have avoided this duplication. Same for another instance above. > + if (cnt_close >= MAX_FILES) > + return 0; > + rets_close[cnt_close] = ret; > + > + cnt_close++; > + return 0; > +} > + > +char _license[] SEC("license") = "GPL"; > -- > 2.25.4 >
Powered by blists - more mailing lists