From: Alexei Starovoitov Scrub slots if variable-offset stack write goes over spilled pointers. Otherwise is_spilled_reg() may == true && spilled_ptr.type == NOT_INIT and valid program is rejected by check_stack_read_fixed_off() with obscure "invalid size of register fill" message. Fixes: 01f810ace9ed ("bpf: Allow variable-offset stack access") Signed-off-by: Alexei Starovoitov --- kernel/bpf/verifier.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 70a100dd4d0c..01663bb4ae1d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5407,7 +5407,7 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env, { struct bpf_func_state *cur; /* state of the current function */ int min_off, max_off; - int i, err; + int i, j, err; struct bpf_reg_state *ptr_reg = NULL, *value_reg = NULL; struct bpf_insn *insn = &env->prog->insnsi[insn_idx]; bool writing_zero = false; @@ -5476,7 +5476,16 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env, } } - /* Erase all other spilled pointers. */ + /* + * Scrub slots if variable-offset stack write goes over spilled pointers. + * Otherwise is_spilled_reg() may == true && spilled_ptr.type == NOT_INIT + * and valid program is rejected by check_stack_read_fixed_off() + * with obscure "invalid size of register fill" message. + */ + if (is_stack_slot_special(&state->stack[spi])) { + for (j = 0; j < BPF_REG_SIZE; j++) + scrub_spilled_slot(&state->stack[spi].slot_type[j]); + } state->stack[spi].spilled_ptr.type = NOT_INIT; /* Update the slot type. */ -- 2.52.0 From: Alexei Starovoitov Add a test to make sure that variable length stack writes scrubs STACK_SPILL into STACK_MISC. Signed-off-by: Alexei Starovoitov --- .../selftests/bpf/progs/verifier_spill_fill.c | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c index a2b6d99628da..672e4446181e 100644 --- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c +++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c @@ -1316,4 +1316,45 @@ __naked void old_imprecise_scalar32_vs_cur_stack_misc(void) : __clobber_all); } +SEC("raw_tp") +__success +__naked void var_off_write_over_scalar_spill(void) +{ + asm volatile ( + /* Get an unknown value bounded to {0, 4} */ + "call %[bpf_ktime_get_ns];" + "r6 = r0;" + "r6 &= 4;" + + /* Spill a scalar to fp-16 */ + "r7 = 0xdeadbeef00000000 ll;" + "*(u64 *)(r10 - 16) = r7;" + + /* + * Variable-offset 4-byte write covering [fp-12, fp-4). + * This touches stype[3..0] of the spill slot at fp-16 but + * leaves stype[7..4] as STACK_SPILL. check_stack_write_var_off() + * must scrub the entire slot when setting spilled_ptr to NOT_INIT, + * otherwise a subsequent sub-register fill sees a non-scalar + * spilled_ptr and is rejected. + */ + "r8 = r10;" + "r8 += r6;" + "r8 += -12;" + "r9 = 0;" + "*(u32 *)(r8 + 0) = r9;" + + /* + * 4-byte read from fp-16. Without the fix this fails with + * "invalid size of register fill" because is_spilled_reg() + * sees STACK_SPILL while spilled_ptr.type == NOT_INIT. + */ + "r0 = *(u32 *)(r10 - 16);" + "r0 = 0;" + "exit;" + : + : __imm(bpf_ktime_get_ns) + : __clobber_all); +} + char _license[] SEC("license") = "GPL"; -- 2.52.0