From: Alexei Starovoitov A prog that guards a kernel function wants to read what the function reads, in the words of the function: SEC("fmod_ret/tcp_ack") int BPF_PROG(guard, struct sock *sk, const struct sk_buff *skb, int flag, int ret) { if (tcp_sk(sk)->snd_una == seq) dev = container_of(skb->dev->dev_list.next, struct net_device, dev_list); if (dev->ifindex == idx) tcp_sk() is a cast to the struct around struct sock, container_of() subtracts an offset. The verifier says: access beyond struct sock at off 2544 size 4 R2 is ptr_list_head invalid negative access: off=-120 and the prog falls back to bpf_core_cast() or bpf_probe_read_kernel(). These checks guard nothing. A load through an untrusted pointer is BPF_PROBE_MEM, whatever the address. bpf_rdonly_cast(p, 0) and the load of a 'u8 *' member give PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED, which has no bounds at all. The type of PTR_TO_BTF_ID | PTR_UNTRUSTED only says what a load gives. the walk of the type finds the load gives a member a number or a pointer, as before nothing: past the end, a number negative offset, a part of a pointer, a variable offset that matches no array The number is not a pointer: tcp_sk(sk)->out_of_order_queue.rb_node can be tested, not followed. Stores, atomics, helpers, __user and __percpu arguments are refused as before. Trusted and RCU pointers are checked as before. Five tests took an untrusted pointer to see an error of the walk. They load now. Signed-off-by: Alexei Starovoitov --- kernel/bpf/verifier.c | 19 +++++++++++++++++++ .../selftests/bpf/progs/map_kptr_fail.c | 2 +- .../bpf/progs/verifier_btf_array_access.c | 6 ++---- .../bpf/progs/verifier_btf_flex_array.c | 2 +- .../testing/selftests/bpf/verifier/map_kptr.c | 3 +-- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 960231ec591e..7d8449baca4c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6844,6 +6844,24 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, return -EINVAL; } + /* + * The type of an untrusted pointer is a hint. A load through the + * pointer is a BPF_PROBE_MEM load, whatever the address. Where the + * type describes the bytes, the result has the type of the member. + * Elsewhere the result is a number, as it is for + * PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED. + */ + if (atype == BPF_READ && value_regno >= 0 && (reg->type & PTR_UNTRUSTED) && + !(reg->type & (MEM_USER | MEM_PERCPU))) { + ret = SCALAR_VALUE; + if (!check_add_overflow(reg_smin(reg), off, &min_off) && min_off >= 0) + ret = btf_struct_access(NULL, reg, off, size, atype, &btf_id, &flag, + &field_name); + if (ret < 0) + ret = SCALAR_VALUE; + goto loaded; + } + if (check_add_overflow(reg_smin(reg), off, &min_off)) { verbose(env, "%s is ptr_%s access, offset computation overflows: register's minimal offset is %lld, instruction offset is %d\n", @@ -6926,6 +6944,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, if (ret < 0) return ret; +loaded: if (ret == PTR_TO_ARENA) return check_arena_member_load(env, regs, value_regno, flag, tname, field_name); diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c index ac4003bfb8b0..4553d41c6ebb 100644 --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c @@ -164,7 +164,7 @@ int marked_as_untrusted_or_null(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("access beyond struct prog_test_ref_kfunc at off 32 size 4") +__success int correct_btf_id_check_size(struct __sk_buff *ctx) { struct prog_test_ref_kfunc *p; diff --git a/tools/testing/selftests/bpf/progs/verifier_btf_array_access.c b/tools/testing/selftests/bpf/progs/verifier_btf_array_access.c index 9dc2c9a5f8b7..5accc0279c37 100644 --- a/tools/testing/selftests/bpf/progs/verifier_btf_array_access.c +++ b/tools/testing/selftests/bpf/progs/verifier_btf_array_access.c @@ -482,8 +482,7 @@ int flex_array_field(void *ctx) * not match. */ SEC("syscall") -__failure -__msg("invalid variable offset access into struct vring_used") +__success int flex_array_unaligned_step(void *ctx) { unsigned long i = bpf_get_prandom_u32(); @@ -511,8 +510,7 @@ int flex_array_unaligned_step(void *ctx) * The flexible-tail exemption must not apply to that fixed array. */ SEC("syscall") -__failure -__msg("invalid variable offset access into struct unix_address") +__success int flex_array_inner_array_out_of_bounds(void *ctx) { unsigned long i = bpf_get_prandom_u32(); diff --git a/tools/testing/selftests/bpf/progs/verifier_btf_flex_array.c b/tools/testing/selftests/bpf/progs/verifier_btf_flex_array.c index 59b84261f622..63a9f6372a8b 100644 --- a/tools/testing/selftests/bpf/progs/verifier_btf_flex_array.c +++ b/tools/testing/selftests/bpf/progs/verifier_btf_flex_array.c @@ -26,7 +26,7 @@ struct { SEC("syscall") __description("btf walk into flexible array of zero-sized elements") -__failure __msg("access beyond struct test_flex_batch at off 4 size 1") +__success int stash_and_peek(void *ctx) { struct test_flex_batch *b, *old; diff --git a/tools/testing/selftests/bpf/verifier/map_kptr.c b/tools/testing/selftests/bpf/verifier/map_kptr.c index 345cecc722a3..d6a6a58c73c4 100644 --- a/tools/testing/selftests/bpf/verifier/map_kptr.c +++ b/tools/testing/selftests/bpf/verifier/map_kptr.c @@ -219,8 +219,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .fixup_map_kptr = { 1 }, - .result = REJECT, - .errstr = "access beyond struct prog_test_ref_kfunc at off 32 size 8", + .result = ACCEPT, }, { "map_kptr: unref: inherit PTR_UNTRUSTED on struct walk", -- 2.55.0