Cover the store which used to be left as a plain BPF_STX without an exception table entry: 1: R1=trusted_ptr_Qdisc() ; struct Qdisc *next = sch->next_sched; 1: (79) r1 = *(u64 *)(r1 +216) ; R1=ptr_Qdisc() ; next->limit = 1000; 3: (63) *(u32 *)(r1 +20) = r2 ; R1=ptr_Qdisc() R2=1000 Assert that it is rejected now. # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t ns_bpf_qdisc [...] #257/1 ns_bpf_qdisc/fifo:OK #257/2 ns_bpf_qdisc/fq:OK #257/3 ns_bpf_qdisc/attach to mq:OK #257/4 ns_bpf_qdisc/attach to non root:OK #257/5 ns_bpf_qdisc/incompl_ops:OK #257/6 ns_bpf_qdisc/invalid_dynptr:OK #257/7 ns_bpf_qdisc/invalid_dynptr_cross_frame:OK #257/8 ns_bpf_qdisc/invalid_dynptr_slice:OK #257/9 ns_bpf_qdisc/untrusted_write:OK #257/10 ns_bpf_qdisc/dynptr_use_after_invalidate_clone:OK #257 ns_bpf_qdisc:OK Summary: 1/10 PASSED, 0 SKIPPED, 0/0 FAILED Signed-off-by: Daniel Borkmann --- v2->v3: - new patch .../selftests/bpf/prog_tests/bpf_qdisc.c | 2 + .../progs/bpf_qdisc_fail__untrusted_write.c | 65 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c index 77f1c0550c9b..6dbd1487343c 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__untrusted_write.skel.h" #include "bpf_qdisc_dynptr_use_after_invalidate_clone.skel.h" #define LO_IFINDEX 1 @@ -230,6 +231,7 @@ void test_ns_bpf_qdisc(void) RUN_TESTS(bpf_qdisc_fail__invalid_dynptr); RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_cross_frame); 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__untrusted_write.c b/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c new file mode 100644 index 000000000000..688c2a049ae3 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c @@ -0,0 +1,65 @@ +// 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"; + +SEC("struct_ops") +__failure __msg("only read is supported") +int BPF_PROG(untrusted_write, struct sk_buff *skb, struct Qdisc *sch, + struct bpf_sk_buff_ptr *to_free) +{ + struct Qdisc *next = sch->next_sched; + + /* + * sch is trusted, but the walk of next_sched yields a plain + * PTR_TO_BTF_ID which may fault on a dereference. A store through + * it does not get an exception table entry, there is no probed + * store to rewrite it into, hence it has to be rejected before + * bpf_qdisc_btf_struct_access() gets to allow the write to limit. + */ + next->limit = 1000; + + 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 *)untrusted_write, + .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", +}; -- 2.43.0