diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index a2a40caca0a0..2c74d676ede9 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -162,11 +162,6 @@ struct bpf_reg_state { * pointing to bpf_func_state. */ u32 frameno; - /* Tracks subreg definition. The stored value is the insn_idx of the - * writing insn. This is safe because subreg_def is used before any insn - * patching which only happens after main verification finished. - */ - s32 subreg_def; /* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */ bool precise; }; @@ -1637,7 +1632,6 @@ struct bpf_kfunc_desc_tab { }; /* Functions exported from verifier.c, used by fixups.c */ -bool bpf_is_reg64(struct bpf_insn *insn, u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t); void bpf_clear_insn_aux_data(struct bpf_verifier_env *env, int start, int len); void bpf_mark_subprog_exc_cb(struct bpf_verifier_env *env, int subprog); bool bpf_allow_tail_call_in_subprogs(struct bpf_verifier_env *env); @@ -1661,5 +1655,6 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env); int bpf_jit_subprogs(struct bpf_verifier_env *env); int bpf_fixup_call_args(struct bpf_verifier_env *env); int bpf_do_misc_fixups(struct bpf_verifier_env *env); +int bpf_insn_def32(struct bpf_prog *prog, struct bpf_insn *insn); #endif /* _LINUX_BPF_VERIFIER_H */ diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index d2ff416d7ad6..447c54828cb9 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -68,8 +68,8 @@ static int insn_def_regno(const struct bpf_insn *insn) * code only. It returns TRUE if the source or destination register operates * on 64-bit, otherwise return FALSE. */ -bool bpf_is_reg64(struct bpf_insn *insn, - u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t) +static bool bpf_is_reg64(struct bpf_prog *prog, struct bpf_insn *insn, + u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t) { u8 code, class, op; @@ -103,6 +103,10 @@ bool bpf_is_reg64(struct bpf_insn *insn, if (class == BPF_ALU64 && op == BPF_END && (insn->imm == 16 || insn->imm == 32)) return false; + /* address space casts converted to BPF_ALU, see bpf_do_misc_fixups() */ + if (is_addr_space_cast32(prog, insn)) + return false; + if (class == BPF_ALU64 || class == BPF_JMP || (class == BPF_ALU && op == BPF_END && insn->imm == 64)) return true; @@ -154,15 +158,18 @@ bool bpf_is_reg64(struct bpf_insn *insn, return true; } -/* Return TRUE if INSN has defined any 32-bit value explicitly. */ -static bool insn_has_def32(struct bpf_insn *insn) +/* + * Return the 32-bit subregister defined by INSN, or -1 if INSN does not + * explicitly define a 32-bit value. + */ +int bpf_insn_def32(struct bpf_prog *prog, struct bpf_insn *insn) { int dst_reg = insn_def_regno(insn); - if (dst_reg == -1) - return false; + if (dst_reg < 0 || bpf_is_reg64(prog, insn, dst_reg, NULL, DST_OP)) + return -1; - return !bpf_is_reg64(insn, dst_reg, NULL, DST_OP); + return dst_reg; } static int kfunc_desc_cmp_by_imm_off(const void *a, const void *b) @@ -279,7 +286,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env, * (cnt == 1) is taken or not. There is no guarantee INSN at OFF is the * original insn at old prog. */ - data[off].zext_dst = insn_has_def32(insn + off + cnt - 1); + data[off].zext_dst = bpf_insn_def32(new_prog, insn + off + cnt - 1) >= 0; if (cnt == 1) return; @@ -291,7 +298,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env, for (i = off; i < off + cnt - 1; i++) { /* Expand insni[off]'s seen count to the patched range. */ data[i].seen = old_seen; - data[i].zext_dst = insn_has_def32(insn + i); + data[i].zext_dst = bpf_insn_def32(new_prog, insn + i) >= 0; } /* @@ -730,7 +737,7 @@ int bpf_opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, * BPF_STX + SRC_OP, so it is safe to pass NULL * here. */ - if (bpf_is_reg64(&insn, load_reg, NULL, DST_OP)) { + if (bpf_is_reg64(env->prog, &insn, load_reg, NULL, DST_OP)) { if (class == BPF_LD && BPF_MODE(code) == BPF_IMM) i++; diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c index edfc2480b0f0..ef9a5a922887 100644 --- a/kernel/bpf/liveness.c +++ b/kernel/bpf/liveness.c @@ -2232,6 +2232,7 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) struct bpf_insn *insns = env->prog->insnsi; struct insn_live_regs *state; int insn_cnt = env->prog->len; + u64 pos, insn_pos; int err = 0, i, j; bool changed; @@ -2291,9 +2292,18 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) } for (i = 0; i < insn_cnt; ++i) { + int def32 = bpf_insn_def32(env->prog, &insns[i]); + u32 out = state[i].out; u32 in = state[i].in; insn_aux[i].live_regs_before = mask_lo(in) | mask_hi(in); + /* + * On architectures where 32-bit operations do not reset upper halves + * of the registers, the verifier needs to zero extend a destination + * register if an instruction defines a 32-bit subregister and the + * upper half of that register is alive after the instruction. + */ + insn_aux[i].zext_dst = def32 >= 0 && (mask_hi(out) & BIT(def32)); } if (env->log.level & BPF_LOG_LEVEL2) { @@ -2310,7 +2320,11 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) else verbose(env, "."); verbose(env, " "); + pos = env->log.end_pos; bpf_verbose_insn(env, &insns[i]); + insn_pos = env->log.end_pos; + if (insn_aux[i].zext_dst) + verbose(env, "%*c; zext", bpf_vlog_alignment(insn_pos - pos), ' '); verbose(env, "\n"); if (bpf_is_ldimm64(&insns[i])) i++; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index dc1acbe0172e..9eabc5123e5a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2132,12 +2132,9 @@ static int reg_bounds_sanity_check(struct bpf_verifier_env *env, /* Mark a register as having a completely unknown (scalar) value. */ void bpf_mark_reg_unknown_imprecise(struct bpf_reg_state *reg) { - s32 subreg_def = reg->subreg_def; - memset(reg, 0, sizeof(*reg)); reg->type = SCALAR_VALUE; reg->var_off = tnum_unknown; - reg->subreg_def = subreg_def; __mark_reg_unbounded(reg); } @@ -2213,7 +2210,6 @@ static int mark_btf_ld_reg(struct bpf_verifier_env *env, } } -#define DEF_NOT_SUBREG (0) static void init_reg_state(struct bpf_verifier_env *env, struct bpf_func_state *state) { @@ -2222,7 +2218,6 @@ static void init_reg_state(struct bpf_verifier_env *env, for (i = 0; i < MAX_BPF_REG; i++) { bpf_mark_reg_not_init(env, ®s[i]); - regs[i].subreg_def = DEF_NOT_SUBREG; } /* frame pointer */ @@ -3058,30 +3053,14 @@ static void mark_stack_slots_scratched(struct bpf_verifier_env *env, mark_stack_slot_scratched(env, spi - i); } -static void mark_insn_zext(struct bpf_verifier_env *env, - struct bpf_reg_state *reg) -{ - s32 def_idx = reg->subreg_def; - - if (def_idx == DEF_NOT_SUBREG) - return; - - env->insn_aux_data[def_idx - 1].zext_dst = true; - /* The dst will be zero extended, so won't be sub-register anymore. */ - reg->subreg_def = DEF_NOT_SUBREG; -} - static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *regs, u32 regno, enum bpf_reg_arg_type t) { - struct bpf_insn *insn = env->prog->insnsi + env->insn_idx; struct bpf_reg_state *reg; - bool rw64; mark_reg_scratched(env, regno); reg = ®s[regno]; - rw64 = bpf_is_reg64(insn, regno, reg, t); if (t == SRC_OP) { /* check whether register used as source operand can be read */ if (reg->type == NOT_INIT) { @@ -3092,9 +3071,6 @@ static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *r if (regno == BPF_REG_FP) return 0; - if (rw64) - mark_insn_zext(env, reg); - return 0; } else { /* check whether register used as dest operand can be written to */ @@ -3102,7 +3078,6 @@ static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *r verbose(env, "frame pointer is read only\n"); return -EACCES; } - reg->subreg_def = rw64 ? DEF_NOT_SUBREG : env->insn_idx + 1; if (t == DST_OP) mark_reg_unknown(env, regs, regno); } @@ -3758,11 +3733,6 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, if (size <= spill_size && bpf_stack_narrow_access_ok(off, size, spill_size)) { - /* The earlier check_reg_arg() has decided the - * subreg_def for this insn. Save it first. - */ - s32 subreg_def = state->regs[dst_regno].subreg_def; - if (env->bpf_capable && size == 4 && spill_size == 4 && get_reg_width(reg) <= 32) /* Ensure stack slot has an ID to build a relation @@ -3770,7 +3740,6 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, */ assign_scalar_id_before_mov(env, reg); state->regs[dst_regno] = *reg; - state->regs[dst_regno].subreg_def = subreg_def; /* Break the relation on a narrowing fill. * coerce_reg_to_size will adjust the boundaries. @@ -6246,12 +6215,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b } else { mark_reg_known_zero(env, regs, value_regno); - /* A load of ctx field could have different - * actual load size with the one encoded in the - * insn. When the dst is PTR, it is for sure not - * a sub-register. - */ - regs[value_regno].subreg_def = DEF_NOT_SUBREG; if (base_type(info.reg_type) == PTR_TO_BTF_ID) { regs[value_regno].btf = info.btf; regs[value_regno].btf_id = info.btf_id; @@ -7350,10 +7313,6 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat if (spi < 0) return spi; - /* - * For CONST_PTR_TO_DYNPTR, reg is already scratched by check_reg_arg - * in check_helper_call and mark_btf_func_reg_size in check_kfunc_call. - */ mark_stack_slots_scratched(env, spi, BPF_DYNPTR_NR_SLOTS); reg = &state->stack[spi].spilled_ptr; @@ -9457,7 +9416,6 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, /* All non-void global functions return a 64-bit SCALAR_VALUE. */ if (!subprog_returns_void(env, subprog)) { mark_reg_unknown(env, caller->regs, BPF_REG_0); - caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG; } if (env->subprog_info[subprog].might_throw) { @@ -10477,9 +10435,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn } invalidate_outgoing_stack_args(env, cur_func(env)); - /* helper call returns 64-bit value. */ - regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG; - /* update return register (already marked as written above) */ ret_type = fn->ret_type; ret_flag = type_flag(ret_type); @@ -10719,30 +10674,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return 0; } -/* mark_btf_func_reg_size() is used when the reg size is determined by - * the BTF func_proto's return value size and argument. - */ -static void __mark_btf_func_reg_size(struct bpf_verifier_env *env, struct bpf_reg_state *regs, - u32 regno, size_t reg_size) -{ - struct bpf_reg_state *reg = ®s[regno]; - - if (regno == BPF_REG_0) { - /* Function return value */ - reg->subreg_def = reg_size == sizeof(u64) ? - DEF_NOT_SUBREG : env->insn_idx + 1; - } else if (reg_size == sizeof(u64)) { - /* Function argument */ - mark_insn_zext(env, reg); - } -} - -static void mark_btf_func_reg_size(struct bpf_verifier_env *env, u32 regno, - size_t reg_size) -{ - return __mark_btf_func_reg_size(env, cur_regs(env), regno, reg_size); -} - static bool is_kfunc_acquire(struct bpf_call_arg_meta *meta) { return meta->kfunc_flags & KF_ACQUIRE; @@ -12961,7 +12892,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, struct bpf_call_arg_meta meta; struct bpf_insn_aux_data *insn_aux; int err, insn_idx = *insn_idx_p; - const struct btf_param *args; u32 i, nargs, ptr_type_id; struct bpf_kfunc_desc *desc; struct btf *desc_btf; @@ -13013,7 +12943,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, verbose(env, "failed to mark s32 range for retval in forked state for lock\n"); return err; } - __mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32)); } else if (!insn->off && insn->imm == special_kfunc_list[KF___bpf_trap]) { verbose(env, "unexpected __bpf_trap() due to uninitialized variable?\n"); return -EFAULT; @@ -13166,7 +13095,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, u32 regno = caller_saved[i]; bpf_mark_reg_not_init(env, ®s[regno]); - regs[regno].subreg_def = DEF_NOT_SUBREG; } invalidate_outgoing_stack_args(env, cur_func(env)); @@ -13188,7 +13116,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (meta.btf == btf_vmlinux && (meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock] || meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) __mark_reg_const_zero(env, ®s[BPF_REG_0]); - mark_btf_func_reg_size(env, BPF_REG_0, t->size); } else if (btf_type_is_ptr(t)) { ptr_type = btf_type_skip_modifiers(desc_btf, t->type, &ptr_type_id); err = check_special_kfunc(env, &meta, regs, insn_aux, ptr_type, desc_btf); @@ -13279,7 +13206,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, /* For mark_ptr_or_null_reg, see 93c230e3f5bd6 */ regs[BPF_REG_0].id = ++env->id_gen; } - mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *)); if (is_kfunc_acquire(&meta)) { id = acquire_reference(env, insn_idx, 0); if (id < 0) @@ -13316,18 +13242,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, caller_info->stack_arg_cnt = stack_arg_cnt; } - args = (const struct btf_param *)(meta.func_proto + 1); - for (i = 0; i < min_t(int, nargs, MAX_BPF_FUNC_REG_ARGS); i++) { - u32 regno = i + 1; - - t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL); - if (btf_type_is_ptr(t)) - mark_btf_func_reg_size(env, regno, sizeof(void *)); - else - /* scalar. ensured by check_kfunc_args() */ - mark_btf_func_reg_size(env, regno, t->size); - } - if (bpf_is_iter_next_kfunc(&meta)) { err = process_iter_next_call(env, insn_idx, &meta); if (err) @@ -14820,14 +14734,14 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, if (dst_reg->type != PTR_TO_ARENA) *dst_reg = *src_reg; - dst_reg->subreg_def = env->insn_idx + 1; - - if (BPF_CLASS(insn->code) == BPF_ALU64) + if (BPF_CLASS(insn->code) == BPF_ALU64) { /* * 32-bit operations zero upper bits automatically. * 64-bit operations need to be converted to 32. */ aux->needs_zext = true; + aux->zext_dst = true; + } /* Any arithmetic operations are allowed on arena pointers */ return 0; @@ -15023,18 +14937,14 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) if (insn->imm) { /* off == BPF_ADDR_SPACE_CAST */ mark_reg_unknown(env, regs, insn->dst_reg); - if (insn->imm == 1) { /* cast from as(1) to as(0) */ + if (insn->imm == 1) /* cast from as(1) to as(0) */ dst_reg->type = PTR_TO_ARENA; - /* PTR_TO_ARENA is 32-bit */ - dst_reg->subreg_def = env->insn_idx + 1; - } } else if (insn->off == 0) { /* case: R1 = R2 * copy register state to dest reg */ assign_scalar_id_before_mov(env, src_reg); *dst_reg = *src_reg; - dst_reg->subreg_def = DEF_NOT_SUBREG; } else { /* case: R1 = (s8, s16 s32)R2 */ if (is_pointer_value(env, insn->src_reg)) { @@ -15052,7 +14962,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) if (!no_sext) clear_scalar_id(dst_reg); coerce_reg_to_size_sx(dst_reg, insn->off >> 3); - dst_reg->subreg_def = DEF_NOT_SUBREG; } else { mark_reg_unknown(env, regs, insn->dst_reg); } @@ -15077,7 +14986,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) */ if (!is_src_reg_u32) clear_scalar_id(dst_reg); - dst_reg->subreg_def = env->insn_idx + 1; } else { /* case: W1 = (s8, s16)W2 */ bool no_sext = reg_umax(src_reg) < (1ULL << (insn->off - 1)); @@ -15087,7 +14995,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) *dst_reg = *src_reg; if (!no_sext) clear_scalar_id(dst_reg); - dst_reg->subreg_def = env->insn_idx + 1; coerce_subreg_to_size_sx(dst_reg, insn->off >> 3); } } else { @@ -15956,12 +15863,8 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s continue; if ((!(reg->id & BPF_ADD_CONST) && !(known_reg->id & BPF_ADD_CONST)) || reg->delta == known_reg->delta) { - s32 saved_subreg_def = reg->subreg_def; - *reg = *known_reg; - reg->subreg_def = saved_subreg_def; } else { - s32 saved_subreg_def = reg->subreg_def; s32 saved_off = reg->delta; u32 saved_id = reg->id; @@ -15971,12 +15874,11 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s /* reg = known_reg; reg += delta */ *reg = *known_reg; /* - * Must preserve off, id and subreg_def flag, - * otherwise another sync_linked_regs() will be incorrect. + * Must preserve off and id, otherwise another sync_linked_regs() + * will be incorrect. */ reg->delta = saved_off; reg->id = saved_id; - reg->subreg_def = saved_subreg_def; scalar32_min_max_add(reg, &fake_reg); scalar_min_max_add(reg, &fake_reg); @@ -16411,8 +16313,6 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn) * Already marked as written above. */ mark_reg_unknown(env, regs, BPF_REG_0); - /* ld_abs load up to 32-bit skb data. */ - regs[BPF_REG_0].subreg_def = env->insn_idx + 1; /* * See bpf_gen_ld_abs() which emits a hidden BPF_EXIT with r0=0 * which must be explored by the verifier when in a subprog.