Adding some new negative selftests which are responsible for asserting that the new relaxed fixed offset constraints applicable to BPF helpers and kfuncs taking trusted pointer arguments are enforced correctly by the BPF verifier. The BPF programs contained within the new negative selftests are mainly responsible for triggering the various branches and checks performed within the check_release_arg_reg_off() helper. Signed-off-by: Matt Bobrowski <mattbobrowski@xxxxxxxxxx> --- .../selftests/bpf/prog_tests/verifier.c | 2 + .../bpf/progs/verifier_arg_reg_off_reject.c | 154 ++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index 6816ff064516..e315bd0a1502 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -87,6 +87,7 @@ #include "verifier_xdp.skel.h" #include "verifier_xdp_direct_packet_access.skel.h" #include "verifier_bits_iter.skel.h" +#include "verifier_arg_reg_off_reject.skel.h" #define MAX_ENTRIES 11 @@ -204,6 +205,7 @@ void test_verifier_xadd(void) { RUN(verifier_xadd); } void test_verifier_xdp(void) { RUN(verifier_xdp); } void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); } void test_verifier_bits_iter(void) { RUN(verifier_bits_iter); } +void test_verifier_arg_reg_off_reject(void) { RUN(verifier_arg_reg_off_reject); } static int init_test_val_map(struct bpf_object *obj, char *map_name) { diff --git a/tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c b/tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c new file mode 100644 index 000000000000..b46656f4cb62 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Google LLC. */ + +#include <vmlinux.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <linux/limits.h> + +#include "bpf_misc.h" +#include "bpf_experimental.h" + +struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym; +void bpf_task_release(struct task_struct *p) __ksym; + +struct random_type { + u64 id; + u64 ref; +}; + +struct alloc_type { + u64 id; + struct nested_type { + u64 id; + } n; + struct random_type __kptr *r; +}; + +struct { + __uint(type, BPF_MAP_TYPE_RINGBUF); + __uint(max_entries, 256 * 1024); +} ringbuf SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __type(key, int); + __type(value, struct alloc_type); + __uint(max_entries, 1); +} array_map SEC(".maps"); + +SEC("tc") +__failure +__msg("R1 must have a fixed offset of 0 when passed to a OBJ_RELEASE/KF_RELEASE flagged BPF helper/kfunc which takes a void *") +int alloc_obj_release(void *ctx) +{ + struct alloc_type *a; + + a = bpf_obj_new(typeof(*a)); + if (!a) { + return 0; + } + /* bpf_obj_drop_impl() takes a void *, so when we attempt to pass in + * something with a reg->off, it should be rejected as we expect to have + * the original pointer passed to the respective BPF helper unmodified. + */ + bpf_obj_drop(&a->n); + return 0; +} + +SEC("lsm.s/file_open") +__failure +__msg("R1 must have a fixed offset of 0 when passed to a OBJ_RELEASE/KF_RELEASE flagged BPF helper/kfunc which takes a void *") +int BPF_PROG(mem_obj_release, struct file *file) +{ + int ret; + char *buf; + + buf = bpf_ringbuf_reserve(&ringbuf, PATH_MAX, 0); + if (!buf) + return 0; + + ret = bpf_d_path(&file->f_path, buf, PATH_MAX); + if (ret <= 0) { + bpf_ringbuf_discard(buf += 8, 0); + return 0; + } + + bpf_ringbuf_submit(buf += 8, 0); + return 0; +} + +SEC("tp_btf/task_newtask") +__failure +__msg("dereference of modified ptr_ ptr R1 off=44 disallowed") +__msg("R1 must have a fixed offset of 0 when passed to a OBJ_RELEASE/KF_RELEASE flagged BPF helper/kfunc which takes a void *") +int BPF_PROG(type_match_mismatch, struct task_struct *task, + u64 clone_flags) +{ + struct task_struct *acquired; + + acquired = bpf_task_acquire(bpf_get_current_task_btf()); + if (!acquired) + return 0; + + bpf_task_release((struct task_struct *)&acquired->flags); + return 0; +} + +SEC("tp_btf/task_newtask") +__failure +__msg("kernel function bpf_task_acquire args#0 expected pointer to STRUCT task_struct") +int BPF_PROG(trusted_type_match_mismatch, struct task_struct *task, + u64 clone_flags) +{ + /* Passing a trusted pointer with incorrect offset will result in a type + * mismatch. + */ + bpf_task_acquire((struct task_struct *)&bpf_get_current_task_btf()->flags); + return 0; +} + +SEC("tp_btf/task_newtask") +__failure +__msg("variable trusted_ptr_ access var_off=(0x0; 0xffffffff) disallowed") +int BPF_PROG(trusted_type_match_mismatch_var_off, struct task_struct *task, + u64 clone_flags) +{ + u32 var_off = bpf_get_prandom_u32(); + task = bpf_get_current_task_btf(); + + task = (void *)task + var_off; + /* Passing a trusted pointer with an incorrect variable offset, type + * match will succeed due to reg->off == 0, but the later call to + * __check_ptr_off_reg should fail as it's responsible for checking + * reg->var_off. + */ + bpf_task_acquire(task); + return 0; +} + +SEC("tp_btf/task_newtask") +__failure +__msg("variable trusted_ptr_ access var_off=(0x0; 0xffffffffffffffff) disallowed") +int BPF_PROG(trusted_type_match_mismatch_neg_var_off, struct task_struct *task, + u64 clone_flags) +{ + s64 var_off = task->start_time; + task = bpf_get_current_task_btf(); + + bpf_assert_range(var_off, -64, 64); + /* Need one bpf_throw() reference, otherwise BTF gen fails. */ + if (!task) + bpf_throw(1); + + task = (void *)task + var_off; + /* Passing a trusted pointer with an incorrect variable offset, type + * match will succeed due to reg->off == 0, but the later call to + * __check_ptr_off_reg should fail as it's responsible for checking + * reg->var_off. + */ + task = bpf_task_acquire(task); + return 0; +} + +char _license[] SEC("license") = "GPL"; -- 2.45.2.627.g7a2c4fd464-goog /M