Add failure tests for slices that escape a subprogram from a dynptr in the callee stack. Cover immediate use of a ring buffer clone slice, use after the shared reservation is released, and immediate use of an skb dynptr slice in a qdisc program. Cover the error path where a subprogram would otherwise lose the last dynptr capable of releasing a referenced resource. Update the existing callback leak test for the same earlier return-time diagnostic. Also add a success control which derives a slice from a caller-owned dynptr and creates a separate clone in the callee. Destroying that local clone on return must not invalidate the caller dynptr's slice. Assisted-by: LLM Signed-off-by: Xu Yunxiang --- .../selftests/bpf/prog_tests/bpf_qdisc.c | 2 + ...disc_fail__invalid_dynptr_returned_slice.c | 76 +++++++++++++++ .../testing/selftests/bpf/progs/dynptr_fail.c | 97 ++++++++++++++++++- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/bpf_qdisc_fail__invalid_dynptr_returned_slice.c diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c index 6dbd1487343c0..122ecb7e98e2a 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c @@ -11,6 +11,7 @@ #include "bpf_qdisc_fail__invalid_dynptr.skel.h" #include "bpf_qdisc_fail__invalid_dynptr_slice.skel.h" #include "bpf_qdisc_fail__invalid_dynptr_cross_frame.skel.h" +#include "bpf_qdisc_fail__invalid_dynptr_returned_slice.skel.h" #include "bpf_qdisc_fail__untrusted_write.skel.h" #include "bpf_qdisc_dynptr_use_after_invalidate_clone.skel.h" @@ -230,6 +231,7 @@ void test_ns_bpf_qdisc(void) test_incompl_ops(); RUN_TESTS(bpf_qdisc_fail__invalid_dynptr); RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_cross_frame); + RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_returned_slice); RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_slice); RUN_TESTS(bpf_qdisc_fail__untrusted_write); RUN_TESTS(bpf_qdisc_dynptr_use_after_invalidate_clone); diff --git a/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__invalid_dynptr_returned_slice.c b/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__invalid_dynptr_returned_slice.c new file mode 100644 index 0000000000000..8217f4c4c00c4 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__invalid_dynptr_returned_slice.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include "bpf_experimental.h" +#include "bpf_qdisc_common.h" +#include "bpf_misc.h" + +char _license[] SEC("license") = "GPL"; + +int proto; + +static __noinline struct ethhdr *slice_in_subprog(struct sk_buff *skb) +{ + struct bpf_dynptr ptr; + + bpf_dynptr_from_skb((struct __sk_buff *)skb, 0, &ptr); + return bpf_dynptr_slice(&ptr, 0, NULL, sizeof(struct ethhdr)); +} + +SEC("struct_ops") +__failure __msg("invalid mem access 'scalar'") +int BPF_PROG(invalid_dynptr_returned_slice, struct sk_buff *skb, + struct Qdisc *sch, struct bpf_sk_buff_ptr *to_free) +{ + struct ethhdr *hdr; + + hdr = slice_in_subprog(skb); + if (!hdr) { + bpf_qdisc_skb_drop(skb, to_free); + return NET_XMIT_DROP; + } + + /* this should fail */ + proto = hdr->h_proto; + + bpf_qdisc_skb_drop(skb, to_free); + + return NET_XMIT_DROP; +} + +SEC("struct_ops") +__auxiliary +struct sk_buff *BPF_PROG(bpf_qdisc_test_dequeue, struct Qdisc *sch) +{ + return NULL; +} + +SEC("struct_ops") +__auxiliary +int BPF_PROG(bpf_qdisc_test_init, struct Qdisc *sch, struct nlattr *opt, + struct netlink_ext_ack *extack) +{ + return 0; +} + +SEC("struct_ops") +__auxiliary +void BPF_PROG(bpf_qdisc_test_reset, struct Qdisc *sch) +{ +} + +SEC("struct_ops") +__auxiliary +void BPF_PROG(bpf_qdisc_test_destroy, struct Qdisc *sch) +{ +} + +SEC(".struct_ops") +struct Qdisc_ops test = { + .enqueue = (void *)invalid_dynptr_returned_slice, + .dequeue = (void *)bpf_qdisc_test_dequeue, + .init = (void *)bpf_qdisc_test_init, + .reset = (void *)bpf_qdisc_test_reset, + .destroy = (void *)bpf_qdisc_test_destroy, + .id = "bpf_qdisc_test", +}; diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 1cd61d72c166f..f57e6162dbf5b 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -127,7 +127,7 @@ static int missing_release_callback_fn(__u32 index, void *data) /* Any dynptr initialized within a callback must have bpf_dynptr_put called */ SEC("?raw_tp") -__failure __msg("Unreleased reference id") +__failure __msg("cannot overwrite referenced dynptr") int ringbuf_missing_release_callback(void *ctx) { bpf_loop(10, missing_release_callback_fn, NULL, 0); @@ -1892,6 +1892,101 @@ int clone_invalidate4(void *ctx) return 0; } +static __noinline void clone_slice_in_subprog(struct bpf_dynptr *ptr, int **data) +{ + struct bpf_dynptr clone; + + bpf_dynptr_clone(ptr, &clone); + *data = bpf_dynptr_data(&clone, 0, sizeof(val)); +} + +static __noinline void caller_slice_in_subprog(struct bpf_dynptr *ptr, int **data) +{ + struct bpf_dynptr clone; + + *data = bpf_dynptr_data(ptr, 0, sizeof(val)); + bpf_dynptr_clone(ptr, &clone); +} + +static __noinline void reserve_dynptr_in_subprog(void) +{ + struct bpf_dynptr ptr; + + bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr); +} + +/* A subprogram cannot lose the last dynptr that can release a resource. */ +SEC("?raw_tp") +__failure __msg("cannot overwrite referenced dynptr") +int referenced_dynptr_lost_on_subprog_return(void *ctx) +{ + reserve_dynptr_in_subprog(); + + return 0; +} + +/* + * Destroying a local clone on return must not invalidate a slice whose + * source dynptr belongs to the caller. + */ +SEC("?raw_tp") +__success +int caller_dynptr_slice_across_subprog_valid(void *ctx) +{ + struct bpf_dynptr ptr; + int *data = NULL; + + bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr); + caller_slice_in_subprog(&ptr, &data); + if (data) + *data = 123; + bpf_ringbuf_submit_dynptr(&ptr, 0); + + return 0; +} + +/* + * A slice that escapes a clone's call frame is invalid once the local + * clone is destroyed on return. + */ +SEC("?raw_tp") +__failure __msg("invalid mem access 'scalar'") +int clone_slice_returned_frame_invalid(void *ctx) +{ + struct bpf_dynptr ptr; + int *data = NULL; + + bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr); + clone_slice_in_subprog(&ptr, &data); + if (data) + /* this should fail */ + *data = 123; + bpf_ringbuf_submit_dynptr(&ptr, 0); + + return 0; +} + +/* + * Releasing the shared reservation must invalidate a slice that escaped + * from a clone's call frame. + */ +SEC("?raw_tp") +__failure __msg("invalid mem access 'scalar'") +int clone_slice_returned_frame_release_invalid(void *ctx) +{ + struct bpf_dynptr ptr; + int *data = NULL; + + bpf_ringbuf_reserve_dynptr(&ringbuf, val, 0, &ptr); + clone_slice_in_subprog(&ptr, &data); + bpf_ringbuf_submit_dynptr(&ptr, 0); + if (data) + /* this should fail */ + *data = 123; + + return 0; +} + /* Invalidating a dynptr should invalidate any data slices * of its parent */ -- 2.43.0