mirror of
https://github.com/torvalds/linux.git
synced 2026-09-23 05:04:02 +02:00
Merge branch 'bpf-preserve-scalar-zero-spills-for-stack-reads'
Woojin Ji says: ==================== bpf: Preserve scalar zero spills for stack reads Changes in v3: - Apply scalar-zero spill preservation to fixed-offset mixed stack reads as well. - Remove the var-offset-only flag from mark_reg_stack_read(). - Keep pure register-fill behavior unchanged. - Add fixed-offset mixed zero/spill-zero selftest coverage. - Address selftest nits: drop extra unpriv checks, align asm formatting, and remove the pruning-sensitive test. - Link to v2: https://patch.msgid.link/20260613-bpf-stack-var-off-zero-v1-v2-0-a324af0f00ea@gmail.com Stack reads currently lose the known-zero fact when loaded bytes come from a spilled scalar constant zero rather than from STACK_ZERO bytes in some paths. This series teaches the stack read zero reconstruction path to preserve that fact while marking the contributing spill slots precise. The original reproducer used a variable-offset stack byte read emitted by clang 22.1.6 at -O2/-O3 from a small helper-based BPF C program. Review of v2 pointed out that fixed-offset reads can benefit as well: pure scalar zero spill reads were already handled, but a fixed read spanning both STACK_ZERO and scalar const-zero STACK_SPILL bytes, e.g. 0000ssss, still fell back to an unknown scalar. v3 handles that mixed fixed-offset case without changing the existing pure register-fill behavior. I still do not have a confirmed deployed-program regression, so this stays targeted at bpf-next. Tested with: - make O=../../out/kernel olddefconfig - make O=../../out/kernel -j$(nproc) kernel/bpf/verifier.o - make O=../../out/kernel LLVM=1 -j$(nproc) bzImage - make -C tools/testing/selftests/bpf O=../../../../../out/kernel VMLINUX_BTF=../../../../../out/kernel/vmlinux TEST_KMOD_TARGETS= LLVM=1 -j$(nproc) test_progs - QEMU guest: ./test_progs -t verifier_var_off -v Summary: 1/24 PASSED, 0 SKIPPED, 0 FAILED - QEMU guest: ./test_progs -t verifier_spill_fill -t verifier_live_stack -t verifier_search_pruning -v Summary: 3/128 PASSED, 0 SKIPPED, 0 FAILED - QEMU guest: ./veristat -o csv verifier_var_off.bpf.o verifier_spill_fill.bpf.o Assisted-by: opencode:gpt-5.5 Signed-off-by: Woojin Ji <random6.xyz@gmail.com> --- ==================== Link: https://patch.msgid.link/20260625-bpf-stack-var-off-zero-v1-v3-0-a068210a761b@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
Alexei Starovoitov
commit
26b29f868b
|
|
@ -1243,6 +1243,11 @@ static inline void bpf_bt_set_frame_slot(struct backtrack_state *bt, u32 frame,
|
|||
bt->stack_masks[frame] |= 1ull << slot;
|
||||
}
|
||||
|
||||
static inline void bpf_bt_set_frame_slot_mask(struct backtrack_state *bt, u32 frame, u64 mask)
|
||||
{
|
||||
bt->stack_masks[frame] |= mask;
|
||||
}
|
||||
|
||||
static inline void bt_set_frame_stack_arg_slot(struct backtrack_state *bt, u32 frame, u32 slot)
|
||||
{
|
||||
bt->stack_arg_masks[frame] |= 1 << slot;
|
||||
|
|
|
|||
|
|
@ -3702,14 +3702,21 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
|
|||
* SCALAR. This function does not deal with register filling; the caller must
|
||||
* ensure that all spilled registers in the stack range have been marked as
|
||||
* read.
|
||||
*
|
||||
* STACK_SPILL bytes backed by spilled scalar const zeroes are also considered
|
||||
* zero bytes. In that case, mark the contributing stack slots precise so
|
||||
* pruning cannot reuse a zero-spill state for a later non-zero spill state.
|
||||
*
|
||||
* Returns an error if precision backtracking fails.
|
||||
*/
|
||||
static void mark_reg_stack_read(struct bpf_verifier_env *env,
|
||||
/* func where src register points to */
|
||||
struct bpf_func_state *ptr_state,
|
||||
int min_off, int max_off, int dst_regno)
|
||||
static int mark_reg_stack_read(struct bpf_verifier_env *env,
|
||||
/* func where src register points to */
|
||||
struct bpf_func_state *ptr_state,
|
||||
int min_off, int max_off, int dst_regno)
|
||||
{
|
||||
struct bpf_verifier_state *vstate = env->cur_state;
|
||||
struct bpf_func_state *state = vstate->frame[vstate->curframe];
|
||||
u64 zero_spill_mask = 0;
|
||||
int i, slot, spi;
|
||||
u8 *stype;
|
||||
int zeros = 0;
|
||||
|
|
@ -3719,19 +3726,33 @@ static void mark_reg_stack_read(struct bpf_verifier_env *env,
|
|||
spi = slot / BPF_REG_SIZE;
|
||||
mark_stack_slot_scratched(env, spi);
|
||||
stype = ptr_state->stack[spi].slot_type;
|
||||
if (stype[slot % BPF_REG_SIZE] != STACK_ZERO)
|
||||
break;
|
||||
zeros++;
|
||||
if (stype[slot % BPF_REG_SIZE] == STACK_ZERO) {
|
||||
zeros++;
|
||||
continue;
|
||||
}
|
||||
if (stype[slot % BPF_REG_SIZE] == STACK_SPILL &&
|
||||
bpf_register_is_null(&ptr_state->stack[spi].spilled_ptr)) {
|
||||
zero_spill_mask |= 1ull << spi;
|
||||
zeros++;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
if (zeros == max_off - min_off) {
|
||||
/* Any access_size read into register is zero extended,
|
||||
* so the whole register == const_zero.
|
||||
*/
|
||||
__mark_reg_const_zero(env, &state->regs[dst_regno]);
|
||||
if (zero_spill_mask) {
|
||||
bpf_bt_set_frame_slot_mask(&env->bt, ptr_state->frameno, zero_spill_mask);
|
||||
return mark_chain_precision_batch(env, env->cur_state);
|
||||
}
|
||||
} else {
|
||||
/* have read misc data from the stack */
|
||||
mark_reg_unknown(env, state->regs, dst_regno);
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Read the stack at 'off' and put the results into the register indicated by
|
||||
|
|
@ -3753,6 +3774,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
|
|||
int i, slot = -off - 1, spi = slot / BPF_REG_SIZE;
|
||||
struct bpf_reg_state *reg;
|
||||
u8 *stype, type;
|
||||
int err;
|
||||
int insn_flags = INSN_F_STACK_ACCESS;
|
||||
int hist_spi = spi, hist_frame = reg_state->frameno;
|
||||
|
||||
|
|
@ -3835,7 +3857,10 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
|
|||
__mark_reg_const_zero(env, &state->regs[dst_regno]);
|
||||
insn_flags = 0; /* not restoring original register state */
|
||||
} else {
|
||||
mark_reg_unknown(env, state->regs, dst_regno);
|
||||
err = mark_reg_stack_read(env, reg_state, off, off + size,
|
||||
dst_regno);
|
||||
if (err)
|
||||
return err;
|
||||
insn_flags = 0; /* not restoring original register state */
|
||||
}
|
||||
}
|
||||
|
|
@ -3880,8 +3905,11 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
|
|||
}
|
||||
return -EACCES;
|
||||
}
|
||||
if (dst_regno >= 0)
|
||||
mark_reg_stack_read(env, reg_state, off, off + size, dst_regno);
|
||||
if (dst_regno >= 0) {
|
||||
err = mark_reg_stack_read(env, reg_state, off, off + size, dst_regno);
|
||||
if (err)
|
||||
return err;
|
||||
}
|
||||
insn_flags = 0; /* we are not restoring spilled register */
|
||||
}
|
||||
if (insn_flags)
|
||||
|
|
@ -3935,7 +3963,10 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env, struct bpf_reg
|
|||
|
||||
min_off = reg_smin(reg) + off;
|
||||
max_off = reg_smax(reg) + off;
|
||||
mark_reg_stack_read(env, ptr_state, min_off, max_off + size, dst_regno);
|
||||
err = mark_reg_stack_read(env, ptr_state, min_off, max_off + size,
|
||||
dst_regno);
|
||||
if (err)
|
||||
return err;
|
||||
check_fastcall_stack_contract(env, ptr_state, env->insn_idx, min_off);
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -634,6 +634,32 @@ __naked void partial_stack_load_preserves_partial_zeros(void)
|
|||
: __clobber_common);
|
||||
}
|
||||
|
||||
SEC("raw_tp")
|
||||
__log_level(2)
|
||||
__success
|
||||
__msg("mark_precise: frame0: regs= stack=-8")
|
||||
__msg("R2=0")
|
||||
__naked void stack_load_preserves_mixed_zero_and_zero_spill(void)
|
||||
{
|
||||
asm volatile (
|
||||
/* fp-8 has scalar const-zero spill bytes and STACK_ZERO bytes. */
|
||||
".8byte %[fp4_st_zero];" /* LLVM-18+: *(u32 *)(r10 -4) = 0; */
|
||||
"r0 = 0;"
|
||||
"*(u32 *)(r10 -8) = r0;"
|
||||
|
||||
"r1 = %[single_byte_buf];"
|
||||
"r2 = *(u64 *)(r10 -8);"
|
||||
"r1 += r2;"
|
||||
"*(u8 *)(r1 + 0) = r2;" /* this should be fine */
|
||||
|
||||
"r0 = 0;"
|
||||
"exit;"
|
||||
:
|
||||
: __imm_ptr(single_byte_buf),
|
||||
__imm_insn(fp4_st_zero, BPF_ST_MEM(BPF_W, BPF_REG_FP, -4, 0))
|
||||
: __clobber_common);
|
||||
}
|
||||
|
||||
char two_byte_buf[2] SEC(".data.two_byte_buf");
|
||||
|
||||
SEC("raw_tp")
|
||||
|
|
|
|||
|
|
@ -59,6 +59,116 @@ __naked void stack_read_priv_vs_unpriv(void)
|
|||
" ::: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("cgroup/skb")
|
||||
__description("variable-offset stack read preserves spilled zero")
|
||||
__success
|
||||
__log_level(2)
|
||||
__msg("mark_precise: frame0: regs= stack=-8")
|
||||
__msg("R3=0")
|
||||
__retval(0)
|
||||
__naked void stack_read_var_off_preserves_spilled_zero(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
r0 = 0; \
|
||||
*(u64*)(r10 - 8) = r0; \
|
||||
r2 = *(u32*)(r1 + 0); \
|
||||
r2 &= 7; \
|
||||
r2 -= 8; \
|
||||
r2 += r10; \
|
||||
r3 = *(u8*)(r2 + 0); \
|
||||
r1 = r10; \
|
||||
r1 += -1; \
|
||||
r1 += r3; \
|
||||
*(u8*)(r1 + 0) = r3; \
|
||||
r0 = 0; \
|
||||
exit; \
|
||||
" ::: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("cgroup/skb")
|
||||
__description("variable-offset stack read preserves spilled zero across slots")
|
||||
__success
|
||||
__log_level(2)
|
||||
__msg("mark_precise: frame0: regs= stack=-8,-16")
|
||||
__msg("R3=0")
|
||||
__retval(0)
|
||||
__naked void stack_read_var_off_preserves_spilled_zero_across_slots(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
r0 = 0; \
|
||||
*(u64*)(r10 - 8) = r0; \
|
||||
*(u64*)(r10 - 16) = r0; \
|
||||
r2 = *(u32*)(r1 + 0); \
|
||||
r2 &= 15; \
|
||||
r2 -= 16; \
|
||||
r2 += r10; \
|
||||
r3 = *(u8*)(r2 + 0); \
|
||||
r1 = r10; \
|
||||
r1 += -1; \
|
||||
r1 += r3; \
|
||||
*(u8*)(r1 + 0) = r3; \
|
||||
r0 = 0; \
|
||||
exit; \
|
||||
" ::: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("cgroup/skb")
|
||||
__description("variable-offset stack read preserves partial spilled zero")
|
||||
__success
|
||||
__log_level(2)
|
||||
__msg("mark_precise: frame0: regs= stack=-8")
|
||||
__msg("R3=0")
|
||||
__retval(0)
|
||||
__naked void stack_read_var_off_preserves_partial_spilled_zero(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
r0 = 0; \
|
||||
*(u8*)(r10 - 9) = r0; \
|
||||
*(u8*)(r10 - 10) = r0; \
|
||||
*(u8*)(r10 - 11) = r0; \
|
||||
*(u8*)(r10 - 12) = r0; \
|
||||
*(u8*)(r10 - 13) = r0; \
|
||||
*(u8*)(r10 - 14) = r0; \
|
||||
*(u8*)(r10 - 15) = r0; \
|
||||
*(u32*)(r10 - 8) = r0; \
|
||||
r2 = *(u32*)(r1 + 0); \
|
||||
r2 &= 15; \
|
||||
if r2 > 10 goto l0_%=; \
|
||||
r2 -= 15; \
|
||||
r2 += r10; \
|
||||
r3 = *(u8*)(r2 + 0); \
|
||||
r1 = r10; \
|
||||
r1 += -1; \
|
||||
r1 += r3; \
|
||||
*(u8*)(r1 + 0) = r3; \
|
||||
l0_%=: r0 = 0; \
|
||||
exit; \
|
||||
" ::: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("cgroup/skb")
|
||||
__description("variable-offset stack read partial spill with misc data")
|
||||
__failure
|
||||
__msg("invalid variable-offset write to stack R1")
|
||||
__naked void stack_read_var_off_partial_spill_with_misc_data(void)
|
||||
{
|
||||
asm volatile (" \
|
||||
r0 = 0; \
|
||||
*(u32*)(r10 - 8) = r0; \
|
||||
r2 = *(u32*)(r1 + 0); \
|
||||
r2 &= 7; \
|
||||
r2 -= 8; \
|
||||
r2 += r10; \
|
||||
r3 = *(u8*)(r2 + 0); \
|
||||
r1 = r10; \
|
||||
r1 += -1; \
|
||||
r1 += r3; \
|
||||
*(u8*)(r1 + 0) = 0; \
|
||||
r0 = 0; \
|
||||
exit; \
|
||||
" ::: __clobber_all);
|
||||
}
|
||||
|
||||
SEC("cgroup/skb")
|
||||
__description("variable-offset stack read, uninitialized")
|
||||
__success
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user