On Thu, 2024-12-12 at 16:22 -0700, Daniel Xu wrote:
Previously, the verifier was treating all PTR_TO_STACK registers passed to a helper call as potentially written to by the helper. However, all calls to check_stack_range_initialized() already have precise access type information available.
Rather than treat ACCESS_HELPER as a proxy for BPF_WRITE, pass enum bpf_access_type to check_stack_range_initialized() to more precisely track helper arguments.
One benefit from this precision is that registers tracked as valid spills and passed as a read-only helper argument remain tracked after the call. Rather than being marked STACK_MISC afterwards.
An additional benefit is the verifier logs are also more precise. For this particular error, users will enjoy a slightly clearer message. See included selftest updates for examples.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz
I think this change is ok. With it there is only one use of 'enum bpf_access_src' remains, but it doesn't look like it could be removed.
Acked-by: Eduard Zingerman eddyz87@gmail.com
[...]
--- a/tools/testing/selftests/bpf/progs/uninit_stack.c +++ b/tools/testing/selftests/bpf/progs/uninit_stack.c @@ -55,33 +55,4 @@ exit_%=: r0 = 0; \ : __clobber_all); } -static __noinline void dummy(void) {}
-/* Pass a pointer to uninitialized stack memory to a helper.
- Passed memory block should be marked as STACK_MISC after helper call.
- */
-SEC("socket") -__log_level(7) __msg("fp-104=mmmmmmmm") -__naked int helper_uninit_to_misc(void *ctx)
Is it possible to peek a helper that writes into memory and not delete this test?
-{
- asm volatile (" \
/* force stack depth to be 128 */ \*(u64*)(r10 - 128) = r1; \r1 = r10; \r1 += -128; \r2 = 32; \call %[bpf_trace_printk]; \/* Call to dummy() forces print_verifier_state(..., true), \* thus showing the stack state, matched by __msg(). \*/ \call %[dummy]; \r0 = 0; \exit; \-"
:: __imm(bpf_trace_printk),__imm(dummy): __clobber_all);-}
[...]