| 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
| ||
|
Message-ID: <20180423002301.lnhdn6oueweqztvb@ast-mbp>
Date: Sun, 22 Apr 2018 18:23:02 -0600
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Yonghong Song <yhs@...com>
Cc: ast@...com, daniel@...earbox.net, netdev@...r.kernel.org,
kernel-team@...com
Subject: Re: [PATCH bpf-next v3 8/9] tools/bpf: add a test for bpf_get_stack
with raw tracepoint prog
On Fri, Apr 20, 2018 at 03:18:41PM -0700, Yonghong Song wrote:
> The test attached a raw_tracepoint program to sched/sched_switch.
> It tested to get stack for user space, kernel space and user
> space with build_id request. It also tested to get user
> and kernel stack into the same buffer with back-to-back
> bpf_get_stack helper calls.
>
> Whenever the kernel stack is available, the user space
> application will check to ensure that the kernel function
> for raw_tracepoint ___bpf_prog_run is part of the stack.
>
> Signed-off-by: Yonghong Song <yhs@...com>
> ---
> tools/testing/selftests/bpf/Makefile | 3 +-
> tools/testing/selftests/bpf/test_get_stack_rawtp.c | 102 ++++++++++++++++++
> tools/testing/selftests/bpf/test_progs.c | 115 +++++++++++++++++++++
> 3 files changed, 219 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/test_get_stack_rawtp.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 0b72cc7..54e9e74 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -32,7 +32,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
> test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \
> sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
> sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o test_adjust_tail.o \
> - test_btf_haskv.o test_btf_nokv.o
> + test_btf_haskv.o test_btf_nokv.o test_get_stack_rawtp.o
>
> # Order correspond to 'make run_tests' order
> TEST_PROGS := test_kmod.sh \
> @@ -56,6 +56,7 @@ $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a
> $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
> $(OUTPUT)/test_sock: cgroup_helpers.c
> $(OUTPUT)/test_sock_addr: cgroup_helpers.c
> +$(OUTPUT)/test_progs: trace_helpers.c
>
> .PHONY: force
>
> diff --git a/tools/testing/selftests/bpf/test_get_stack_rawtp.c b/tools/testing/selftests/bpf/test_get_stack_rawtp.c
> new file mode 100644
> index 0000000..ba1dcf9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_get_stack_rawtp.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +/* Permit pretty deep stack traces */
> +#define MAX_STACK_RAWTP 100
> +struct stack_trace_t {
> + int pid;
> + int kern_stack_size;
> + int user_stack_size;
> + int user_stack_buildid_size;
> + __u64 kern_stack[MAX_STACK_RAWTP];
> + __u64 user_stack[MAX_STACK_RAWTP];
> + struct bpf_stack_build_id user_stack_buildid[MAX_STACK_RAWTP];
> +};
> +
> +struct bpf_map_def SEC("maps") perfmap = {
> + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = sizeof(int),
> + .value_size = sizeof(__u32),
> + .max_entries = 2,
> +};
> +
> +struct bpf_map_def SEC("maps") stackdata_map = {
> + .type = BPF_MAP_TYPE_PERCPU_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = sizeof(struct stack_trace_t),
> + .max_entries = 1,
> +};
> +
> +/* Allocate per-cpu space twice the needed. For the code below
> + * usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
> + * if (usize < 0)
> + * return 0;
> + * ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
> + *
> + * If we have value_size = MAX_STACK_RAWTP * sizeof(__u64),
> + * verifier will complain that access "raw_data + usize"
> + * with size "max_len - usize" may be out of bound.
> + * The maximum "raw_data + usize" is "raw_data + max_len"
> + * and the maximum "max_len - usize" is "max_len", verifier
> + * concludes that the maximum buffer access range is
> + * "raw_data[0...max_len * 2 - 1]" and hence reject the program.
> + *
> + * Doubling the to-be-used max buffer size can fix this verifier
> + * issue and avoid complicated C programming massaging.
> + * This is an acceptable workaround since there is one entry here.
> + */
> +struct bpf_map_def SEC("maps") rawdata_map = {
> + .type = BPF_MAP_TYPE_PERCPU_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = MAX_STACK_RAWTP * sizeof(__u64) * 2,
> + .max_entries = 1,
> +};
> +
> +SEC("tracepoint/sched/sched_switch")
> +int bpf_prog1(void *ctx)
> +{
> + int max_len, max_buildid_len, usize, ksize, total_size;
> + struct stack_trace_t *data;
> + void *raw_data;
> + __u32 key = 0;
> +
> + data = bpf_map_lookup_elem(&stackdata_map, &key);
> + if (!data)
> + return 0;
> +
> + max_len = MAX_STACK_RAWTP * sizeof(__u64);
> + max_buildid_len = MAX_STACK_RAWTP * sizeof(struct bpf_stack_build_id);
> + data->pid = bpf_get_current_pid_tgid();
> + data->kern_stack_size = bpf_get_stack(ctx, data->kern_stack,
> + max_len, 0);
> + data->user_stack_size = bpf_get_stack(ctx, data->user_stack, max_len,
> + BPF_F_USER_STACK);
> + data->user_stack_buildid_size = bpf_get_stack(
> + ctx, data->user_stack_buildid, max_buildid_len,
> + BPF_F_USER_STACK | BPF_F_USER_BUILD_ID);
> + bpf_perf_event_output(ctx, &perfmap, 0, data, sizeof(*data));
> +
> + /* write both kernel and user stacks to the same buffer */
> + raw_data = bpf_map_lookup_elem(&rawdata_map, &key);
> + if (!raw_data)
> + return 0;
> +
> + usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
> + if (usize < 0)
> + return 0;
> +
> + ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
> + if (ksize < 0)
may be instead of teaching verifier about ARSH (which doesn't look
straighforward) such use case can be done as:
u32 max_len, usize, ksize;
ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
if ((int)ksize < 0)
That's certainly suboptimal and very much non obvious to program
developers, but at least it can unblock the bpf_get_stack part
landing and proper ARSH support can be added later?
Just a thought.
> + return 0;
> +
> + total_size = usize + ksize;
> + if (total_size > 0 && total_size <= max_len)
> + bpf_perf_event_output(ctx, &perfmap, 0, raw_data, total_size);
> +
> + return 0;
> +}
the rest of the test looks great. Thank you for adding such exhaustive test.
Powered by blists - more mailing lists