mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Merge branch 'fix-fastcall-rewrite-with-indirect-stack-accesses'
Kumar Kartikeya Dwivedi says:
====================
Fix fastcall rewrite with indirect stack accesses
When a bpf_fastcall call is inlined, the verifier removes the spill/fill
pairs around it and shrinks the stack frame to exclude their slots. Two
kinds of stack access skip the check that must disable this rewrite, so
the program can then access kernel stack outside its frame: stack
buffers passed to helpers and kfuncs, which BPF CI reported after commit
5da4a9f26f ("bpf: Preserve stack initialization for generic output
buffers"), and a callee's load from its caller's stack, which checked
the callee's frame instead of the caller's.
Patch 1 moves the check into check_stack_access_within_bounds(), which
every stack access goes through with the frame that owns the slots, and
drops the calls from the stack read and write paths. Patch 2 adds tests.
Changelog:
----------
v1 -> v2
v1: https://lore.kernel.org/bpf/20260923084201.2437625-1-memxor@gmail.com
* Check the contract once in check_stack_access_within_bounds() and drop
the calls in the stack read and write paths, which also fixes the frame
check_stack_read_fixed_off() checked; fold v1 patch 2 into patch 1.
(Alexei)
* Scope the test comment to the output buffer tests and describe the
remaining tests individually. (BPF CI)
====================
Link: https://patch.msgid.link/20260924075352.2343553-1-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
commit
44e4e52c11
|
|
@ -3716,7 +3716,6 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
|
|||
if (err)
|
||||
return err;
|
||||
|
||||
check_fastcall_stack_contract(env, state, insn_idx, off);
|
||||
mark_stack_slot_scratched(env, spi);
|
||||
if (reg && !(off % BPF_REG_SIZE) && reg->type == SCALAR_VALUE && env->bpf_capable) {
|
||||
bool reg_value_fits;
|
||||
|
|
@ -3837,7 +3836,6 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
|
|||
return err;
|
||||
}
|
||||
|
||||
check_fastcall_stack_contract(env, state, insn_idx, min_off);
|
||||
/* Variable offset writes destroy any spilled pointers in range. */
|
||||
for (i = min_off; i < max_off; i++) {
|
||||
u8 new_type, *stype;
|
||||
|
|
@ -4022,7 +4020,6 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
|
|||
reg = ®_state->stack[spi].spilled_ptr;
|
||||
|
||||
mark_stack_slot_scratched(env, spi);
|
||||
check_fastcall_stack_contract(env, state, env->insn_idx, off);
|
||||
|
||||
/*
|
||||
* Refine the in-progress load record's origin to the source stack slot.
|
||||
|
|
@ -4209,7 +4206,6 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env, struct bpf_reg
|
|||
dst_regno);
|
||||
if (err)
|
||||
return err;
|
||||
check_fastcall_stack_contract(env, ptr_state, env->insn_idx, min_off);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -6614,6 +6610,15 @@ static int check_stack_access_within_bounds(
|
|||
return err;
|
||||
}
|
||||
|
||||
/*
|
||||
* Every stack access passes through here, including buffers passed to
|
||||
* helpers and kfuncs and accesses into a caller's frame, so check the
|
||||
* fastcall contract of the frame that owns the slots once for all of
|
||||
* them. Zero-sized accesses touch no stack and keep the rewrite enabled.
|
||||
*/
|
||||
if (access_size)
|
||||
check_fastcall_stack_contract(env, state, env->insn_idx, min_off);
|
||||
|
||||
/* Note that there is no stack access with offset zero, so the needed stack
|
||||
* size is -min_off, not -min_off+1.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -502,6 +502,175 @@ __naked void bad_helper_write(void)
|
|||
: __clobber_all);
|
||||
}
|
||||
|
||||
/*
|
||||
* A helper buffer or a callee's pointer that reaches a fastcall spill slot
|
||||
* must keep the spill/fill pair and the stack that covers it. Only load
|
||||
* these programs: without the fix, they access kernel stack outside their
|
||||
* frame.
|
||||
*
|
||||
* Uninitialized outputs without CAP_PERFMON have no explicit stack accesses.
|
||||
*/
|
||||
SEC("tc")
|
||||
__log_level(4)
|
||||
__msg_unpriv("subprog 0 (helper_uninit_stack) main {{.*}} stack 64")
|
||||
__xlated_unpriv("*(u64 *)(r10 -8) = r1")
|
||||
__xlated_unpriv("...")
|
||||
__xlated_unpriv("r1 = *(u64 *)(r10 -8)")
|
||||
__caps_unpriv(CAP_BPF | CAP_NET_ADMIN)
|
||||
__success_unpriv
|
||||
__naked void helper_uninit_stack(void)
|
||||
{
|
||||
asm volatile (
|
||||
"*(u64 *)(r10 - 8) = r1;"
|
||||
"call %[bpf_get_smp_processor_id];"
|
||||
"r1 = *(u64 *)(r10 - 8);"
|
||||
"r2 = 0;"
|
||||
"r3 = r10;"
|
||||
"r3 += -64;"
|
||||
"r4 = 8;"
|
||||
"call %[bpf_skb_load_bytes];"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_smp_processor_id),
|
||||
__imm(bpf_skb_load_bytes)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
/* Same output in the caller's stack, passed to the helper by a callee. */
|
||||
static __used __naked void helper_uninit_stack_callee(void)
|
||||
{
|
||||
asm volatile (
|
||||
"r3 = r2;"
|
||||
"r2 = 0;"
|
||||
"r4 = 8;"
|
||||
"call %[bpf_skb_load_bytes];"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_skb_load_bytes)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("tc")
|
||||
__log_level(4)
|
||||
__msg_unpriv("subprog 0 (helper_uninit_stack_caller) main {{.*}} stack 64")
|
||||
__xlated_unpriv("*(u64 *)(r10 -8) = r1")
|
||||
__xlated_unpriv("...")
|
||||
__xlated_unpriv("r1 = *(u64 *)(r10 -8)")
|
||||
__caps_unpriv(CAP_BPF | CAP_NET_ADMIN)
|
||||
__success_unpriv
|
||||
__naked void helper_uninit_stack_caller(void)
|
||||
{
|
||||
asm volatile (
|
||||
"*(u64 *)(r10 - 8) = r1;"
|
||||
"call %[bpf_get_smp_processor_id];"
|
||||
"r1 = *(u64 *)(r10 - 8);"
|
||||
"r2 = r10;"
|
||||
"r2 += -64;"
|
||||
"call helper_uninit_stack_callee;"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_smp_processor_id)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
/* A helper input whose only initialization is the fastcall spill. */
|
||||
struct {
|
||||
__uint(type, BPF_MAP_TYPE_ARRAY);
|
||||
__uint(max_entries, 1);
|
||||
__type(key, __u32);
|
||||
__type(value, __u64);
|
||||
} fastcall_map SEC(".maps");
|
||||
|
||||
SEC("tc")
|
||||
__log_level(4)
|
||||
__msg("subprog 0 (helper_reads_fastcall_spill) main {{.*}} stack 8")
|
||||
__xlated("*(u64 *)(r10 -8) = r1")
|
||||
__xlated("...")
|
||||
__xlated("r1 = *(u64 *)(r10 -8)")
|
||||
__success
|
||||
__naked void helper_reads_fastcall_spill(void)
|
||||
{
|
||||
asm volatile (
|
||||
"r1 = 0;"
|
||||
"*(u64 *)(r10 - 8) = r1;"
|
||||
"call %[bpf_get_smp_processor_id];"
|
||||
"r1 = *(u64 *)(r10 - 8);"
|
||||
"r1 = %[fastcall_map] ll;"
|
||||
"r2 = r10;"
|
||||
"r2 += -8;"
|
||||
"call %[bpf_map_lookup_elem];"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_smp_processor_id),
|
||||
__imm(bpf_map_lookup_elem),
|
||||
__imm_addr(fastcall_map)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
/* A callee load from the caller's fastcall spill slot. */
|
||||
static __used __naked void read_caller_stack_callee(void)
|
||||
{
|
||||
asm volatile (
|
||||
"r0 = *(u64 *)(r1 + 0);"
|
||||
"exit;"
|
||||
::: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("raw_tp")
|
||||
__log_level(4)
|
||||
__msg("subprog 0 (callee_reads_fastcall_spill) main {{.*}} stack 8")
|
||||
__xlated("*(u64 *)(r10 -8) = r1")
|
||||
__xlated("...")
|
||||
__xlated("r1 = *(u64 *)(r10 -8)")
|
||||
__success
|
||||
__naked void callee_reads_fastcall_spill(void)
|
||||
{
|
||||
asm volatile (
|
||||
"r1 = 1;"
|
||||
"*(u64 *)(r10 - 8) = r1;"
|
||||
"call %[bpf_get_smp_processor_id];"
|
||||
"r1 = *(u64 *)(r10 - 8);"
|
||||
"r1 = r10;"
|
||||
"r1 += -8;"
|
||||
"call read_caller_stack_callee;"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_smp_processor_id)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
/* A zero-sized buffer touches no stack, the rewrite is still applied. */
|
||||
SEC("raw_tp")
|
||||
__arch_x86_64
|
||||
__log_level(4)
|
||||
__msg("subprog 0 (helper_zero_size_buffer) main {{.*}} stack 0")
|
||||
__xlated("0: r1 = 1")
|
||||
__xlated("1: r0 =")
|
||||
__success
|
||||
__naked void helper_zero_size_buffer(void)
|
||||
{
|
||||
asm volatile (
|
||||
"r1 = 1;"
|
||||
"*(u64 *)(r10 - 8) = r1;"
|
||||
"call %[bpf_get_smp_processor_id];"
|
||||
"r1 = *(u64 *)(r10 - 8);"
|
||||
"r1 = r10;"
|
||||
"r1 += -64;"
|
||||
"r2 = 0;"
|
||||
"r3 = 0;"
|
||||
"call %[bpf_probe_read_kernel];"
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm(bpf_get_smp_processor_id),
|
||||
__imm(bpf_probe_read_kernel)
|
||||
: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("raw_tp")
|
||||
__arch_x86_64
|
||||
/* main, not patched */
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user