From: Alexei Starovoitov The compiler can pass more or fewer arguments to a static subprog than its BTF prototype has parameters. See "bpf: Count stack arguments of a static subprog from its code". test the static subprog result ------------------------------- ---------------------------- ---------- more_than_prototype 4 in BTF, loads the 6th returns 11 more_than_prototype_not_written the same, 6th is not stored reject less_than_prototype 7 in BTF, reads r1 only returns 1 callback_takes_6 a callback, loads the 6th reject tgt_prog_takes_more 4 in BTF, loads the 6th, reject fexit attaches to it more_than_prototype also examines the x86 code of the subprog. The subprog passes no stack arguments to other functions. Thus the JIT must not move rsp in it. The verifier now rejects the test "stack_arg: read with not-initialized caller write" at the call. Update the message that it expects. Signed-off-by: Alexei Starovoitov --- .../selftests/bpf/prog_tests/tracing_struct.c | 15 ++- .../bpf/progs/tracing_struct_int128_tgt.c | 26 ++++- .../bpf/progs/tracing_struct_int128_tracer.c | 7 ++ .../selftests/bpf/progs/verifier_stack_arg.c | 105 ++++++++++++++++++ .../bpf/progs/verifier_stack_arg_order.c | 2 +- 5 files changed, 147 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c index de358f335d98..628b63b406ad 100644 --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c @@ -253,13 +253,13 @@ static void test_int128_tgt_prog(void) * A static function whose BTF is unreliable gets a trampoline that only knows * of the register arguments, so one taking stack arguments must be refused. */ -static void test_tgt_prog_unreliable(void) +static void test_tgt_prog_unreliable(bool takes_more) { #if defined(__x86_64__) || defined(__aarch64__) struct tracing_struct_int128_tracer *tracer = NULL; struct tracing_struct_int128_tgt *tgt; + struct bpf_program *prog, *entry; char log_buf[4096] = {}; - struct bpf_program *prog; int err, fd; tgt = tracing_struct_int128_tgt__open_and_load(); @@ -277,9 +277,10 @@ static void test_tgt_prog_unreliable(void) bpf_object__for_each_program(prog, tracer->obj) bpf_program__set_autoload(prog, false); - bpf_program__set_autoload(tracer->progs.unreliable_entry, true); - bpf_program__set_log_buf(tracer->progs.unreliable_entry, log_buf, sizeof(log_buf)); - err = bpf_program__set_attach_target(tracer->progs.unreliable_entry, fd, + entry = takes_more ? tracer->progs.takes_more_exit : tracer->progs.unreliable_entry; + bpf_program__set_autoload(entry, true); + bpf_program__set_log_buf(entry, log_buf, sizeof(log_buf)); + err = bpf_program__set_attach_target(entry, fd, takes_more ? "callee_takes_more" : "callee_unreliable"); if (!ASSERT_OK(err, "set_attach_target")) goto out; @@ -389,7 +390,9 @@ void test_tracing_struct(void) if (test__start_subtest("int128_tgt_prog")) test_int128_tgt_prog(); if (test__start_subtest("tgt_prog_unreliable")) - test_tgt_prog_unreliable(); + test_tgt_prog_unreliable(false); + if (test__start_subtest("tgt_prog_takes_more")) + test_tgt_prog_unreliable(true); if (test__start_subtest("stack_arg_tailcall")) test_stack_arg_tailcall(); if (test__start_subtest("union_args")) diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c index 2561422c25dd..35c9a0c9b5cf 100644 --- a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c +++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c @@ -8,7 +8,7 @@ struct int128_arg { }; __u64 base; -__u64 ret_odd, ret_stack, ret_split, ret_unreliable; +__u64 ret_odd, ret_stack, ret_split, ret_unreliable, ret_takes_more; #if defined(__clang__) @@ -68,6 +68,29 @@ static __noinline __u64 callee_unreliable(__u64 *p, __u64 b, __u64 c, __u64 d, return *p + b * 2 + c * 3 + d * 4 + e * 5 + f * 6; } +/* BTF says 4. The code takes 6, the last one on the stack. */ +static __attribute__((naked)) __noinline __attribute__((used)) +__u64 callee_takes_more(__u64 a, __u64 b, __u64 c, __u64 d) +{ + asm volatile ( + "r0 = *(u64 *)(r11 + 8);" + "r0 += r5;" + "exit;"); +} + +static __attribute__((naked)) __noinline __u64 call_takes_more(void) +{ + asm volatile ( + "r1 = 1;" + "r2 = 2;" + "r3 = 3;" + "r4 = 4;" + "r5 = 5;" + "*(u64 *)(r11 - 8) = 6;" + "call callee_takes_more;" + "exit;"); +} + #else const volatile bool has_stack_arg = false; @@ -93,6 +116,7 @@ int run(void *ctx) ret_unreliable = callee_unreliable(&x, base + 2, base + 3, base + 4, base + 5, base + 6); } + ret_takes_more = call_takes_more(); #endif #endif return 0; diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c index fc81c2eea4a8..a6d57e6b4798 100644 --- a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c +++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c @@ -74,4 +74,11 @@ int unreliable_entry(unsigned long long *ctx) return 0; } +/* The same, but only the code tells that there is a stack argument. */ +SEC("?fexit/callee_takes_more") +int takes_more_exit(unsigned long long *ctx) +{ + return 0; +} + char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c index b9f799f6e405..5062c5bab5f0 100644 --- a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c @@ -433,6 +433,111 @@ __naked void stack_arg_sequential_calls(void) ); } +/* + * The prototype is what the source says. A compiler may pass a pair in two + * registers, add a pointer to the result, drop arguments that are not used. + */ +static __naked __noinline __used long takes_6_says_4(long a, long b, long c, long d) +{ + asm volatile ( + "r0 = *(u64 *)(r11 + 8);" + "r0 += r5;" + "exit;" + ::: __clobber_all); +} + +SEC("tc") +__description("stack_arg: static subprog takes more than its prototype says") +__success __retval(11) +__arch_x86_64 +/* takes_6_says_4() passes no stack arguments: rsp stays where it is */ +__jited("func #1") +__jited(" endbr64") +__jited(" nopl (%rax,%rax)") +__jited(" nopl (%rax)") +__jited(" pushq %rbp") +__jited(" movq %rsp, %rbp") +__jited(" endbr64") +__jited(" movq %r9, %rax") +__naked void more_than_prototype(void) +{ + asm volatile ( + "r1 = 1;" + "r2 = 2;" + "r3 = 3;" + "r4 = 4;" + "r5 = 5;" + "*(u64 *)(r11 - 8) = 6;" + "call takes_6_says_4;" + "exit;" + ::: __clobber_all); +} + +static __naked __noinline __used long cb_takes_6(long i, void *ctx) +{ + asm volatile ( + "r0 = *(u64 *)(r11 + 8);" + "r0 = 0;" + "exit;" + ::: __clobber_all); +} + +SEC("tc") +__description("stack_arg: helpers pass no stack arguments to a callback") +__failure __msg("callback cb_takes_6() cannot have stack arguments") +__naked void callback_takes_6(void) +{ + asm volatile ( + "r1 = 1;" + "r2 = %[cb_takes_6];" + "r3 = 0;" + "r4 = 0;" + "call %[bpf_loop];" + "r0 = 0;" + "exit;" + : + : __imm_ptr(cb_takes_6), + __imm(bpf_loop) + : __clobber_common); +} + +static __naked __noinline __used long takes_1_says_7(long a, long b, long c, long d, long e, + long f, long g) +{ + asm volatile ( + "r0 = r1;" + "exit;" + ::: __clobber_all); +} + +SEC("tc") +__description("stack_arg: static subprog takes less than its prototype says") +__success __retval(1) +__naked void less_than_prototype(void) +{ + asm volatile ( + "r1 = 1;" + "call takes_1_says_7;" + "exit;" + ::: __clobber_all); +} + +SEC("tc") +__description("stack_arg: what the static subprog takes is not written") +__failure __msg("callee expects 6 args, stack arg1 is not initialized") +__naked void more_than_prototype_not_written(void) +{ + asm volatile ( + "r1 = 1;" + "r2 = 2;" + "r3 = 3;" + "r4 = 4;" + "r5 = 5;" + "call takes_6_says_4;" + "exit;" + ::: __clobber_all); +} + #else SEC("socket") diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c index ab1955852233..42b6d4a4b4d4 100644 --- a/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c @@ -152,7 +152,7 @@ __naked void stack_arg_read_without_write_1(void) SEC("tc") __description("stack_arg: read with not-initialized caller write") __failure -__msg("R0 !read_ok") +__msg("callee expects 7 args, stack arg1 is not initialized") __btf_func_path("btf__verifier_stack_arg_order.bpf.o") __naked void stack_arg_read_without_write_2(void) { -- 2.55.0