diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 33dc3d402057..228a3f6889c2 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -50,6 +50,13 @@ enum bpf_iter_state { BPF_ITER_STATE_DRAINED, }; +/* Bytes [off, off + size) of the stack of frame frameno. Nothing when size is 0. */ +struct bpf_stack_range { + u32 frameno; + s32 off; + u32 size; +}; + struct bpf_reg_state { /* Ordering of fields matters. See states_equal() */ enum bpf_reg_type type; @@ -83,6 +90,12 @@ struct bpf_reg_state { struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */ u32 mem_size; + /* + * bpf_dynptr_slice() and bpf_dynptr_slice_rdwr() return the + * buffer when the data is not contiguous: the register may + * point to this part of the stack. + */ + struct bpf_stack_range buf; }; /* For dynptr stack slots */ @@ -1091,6 +1104,8 @@ struct bpf_verifier_env { bool bypass_spec_v1; bool bypass_spec_v4; bool seen_direct_write; + /* a register may point to the stack and is not PTR_TO_STACK, see bpf_reg_state::buf */ + bool seen_stack_slice; /* a global function is verified for BPF_FUNC_CTX_NO_RCU_LOCK */ bool no_rcu_read_lock; bool seen_exception; @@ -1815,6 +1830,8 @@ struct bpf_call_arg_meta { struct arg_constant_desc arg_constant; /* The type of the dynptr that the kfunc writes through */ enum bpf_dynptr_type dynptr_write_type; + /* The buffer of bpf_dynptr_slice() or bpf_dynptr_slice_rdwr() */ + struct bpf_stack_range slice_buf; /* arg_{btf,btf_id,owning_ref} are used by kfunc-specific handling, * generally to pass info about user-defined local kptr types to later diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c index b5204798231d..08e12714bb95 100644 --- a/kernel/bpf/diagnostics.c +++ b/kernel/bpf/diagnostics.c @@ -1561,7 +1561,8 @@ static void diag_record_mod(struct bpf_verifier_env *env, u32 insn_idx, if (origin) { event.mod.origin = *origin; event.mod.origin_valid = true; - } else if (diag_mod_insn_origin(env, insn_idx, &target, &event.mod.origin)) { + } else if (reason != BPF_DIAG_MOD_SLICE_BUF && /* the insn writes the buffer */ + diag_mod_insn_origin(env, insn_idx, &target, &event.mod.origin)) { event.mod.origin_valid = true; } if (old_reg && new_reg && reason == BPF_DIAG_MOD_WRITE && @@ -2237,6 +2238,9 @@ static void diag_print_mod(struct bpf_verifier_env *env, const struct bpf_diag_h case BPF_DIAG_MOD_PKT_DATA_CHANGE: reason = "packet data may have moved"; break; + case BPF_DIAG_MOD_SLICE_BUF: + reason = "the stack buffer of this dynptr slice was written, or its frame returned"; + break; case BPF_DIAG_MOD_NON_OWN_REF: reason = "leaving the protected region invalidated this borrowed pointer"; break; diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h index a4102fb049ec..490decc133f6 100644 --- a/kernel/bpf/diagnostics.h +++ b/kernel/bpf/diagnostics.h @@ -23,6 +23,7 @@ enum bpf_diag_mod_reason { BPF_DIAG_MOD_VAR_WRITE, BPF_DIAG_MOD_REF_RELEASE, BPF_DIAG_MOD_PKT_DATA_CHANGE, + BPF_DIAG_MOD_SLICE_BUF, BPF_DIAG_MOD_NON_OWN_REF, BPF_DIAG_MOD_CALLER_SAVED, }; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 3cbee140ede8..22b46f7d0043 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -3088,6 +3088,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid) * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in * the bpf program. * + * Please also note that when the buffer is on the stack, the verifier invalidates + * the returned pointer when the program stores to the buffer (helpers and kfuncs + * may fill it), or returns from the function that has it. + * * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only * data slice (can be either direct pointer to the data or a pointer to the user * provided buffer, with its contents containing the data, if unable to obtain @@ -3173,6 +3177,12 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr *p, u64 offset, * Please note that, as in the example above, the user must check that the * returned pointer is not null before using it. * + * Please also note that when the buffer is on the stack, the verifier invalidates + * the returned pointer when the program stores to the buffer (helpers and kfuncs + * may fill it), or returns from the function that has it. Change the data through + * the returned pointer, and before the comparison with the buffer, as in the + * example above: after it the compiler may use one pointer in place of the other. + * * Please also note that in the case of skb and xdp dynptrs, bpf_dynptr_slice_rdwr * does not change the underlying packet data pointers, so a call to * bpf_dynptr_slice_rdwr will not invalidate any ctx->data/data_end pointers in diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c index fa29d9b994e4..916408c63355 100644 --- a/kernel/bpf/log.c +++ b/kernel/bpf/log.c @@ -684,6 +684,8 @@ static void print_reg_state(struct bpf_verifier_env *env, if (base_type(t) == PTR_TO_MEM) { verbose_a("sz="); verbose_unum(env, reg->mem_size); + if (reg->buf.size) + verbose_a("buf=fp[%u]%d+%u", reg->buf.frameno, reg->buf.off, reg->buf.size); } if (t == CONST_PTR_TO_DYNPTR) verbose_a("type=%s", dynptr_type_str(reg->dynptr.type)); diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c index 1123a14e7389..06dee7e421ea 100644 --- a/kernel/bpf/scev.c +++ b/kernel/bpf/scev.c @@ -1831,7 +1831,11 @@ scev_reg_for_read(struct bpf_verifier_env *env, const struct bpf_func_state *st, return zero ? &env->scev->zero_reg : &env->scev->unknown_reg; } -/* Materialize a writable value in the current state, growing its stack if needed. */ +/* + * Materialize a writable value in the current state, growing its stack if needed. + * This does not go through check_stack_write(). The callers skip is_unbound_scev_reg() slots, + * thus the buffer of a live dynptr slice, see bpf_reg_state::buf, does not become a spill here. + */ static int scev_reg_for_write(struct bpf_verifier_env *env, struct bpf_func_state *st, u32 r, struct bpf_reg_state **out) { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 217bcfa6ebfe..fb96c61ebec0 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1010,6 +1010,13 @@ static bool dynptr_write_changes_pkt_data(enum bpf_dynptr_type type) return type == BPF_DYNPTR_TYPE_SKB || type == BPF_DYNPTR_TYPE_SKB_META; } +/* The data of the other types is contiguous: bpf_dynptr_slice() does not use the buffer. */ +static bool dynptr_slice_may_be_buffer(enum bpf_dynptr_type type) +{ + return type != BPF_DYNPTR_TYPE_LOCAL && type != BPF_DYNPTR_TYPE_RINGBUF && + type != BPF_DYNPTR_TYPE_SKB_META; +} + static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg_state *reg, enum bpf_arg_type arg_type) { @@ -4558,6 +4565,31 @@ static int check_stack_read(struct bpf_verifier_env *env, return err; } +/* + * An access through a slice that may be the buffer on the stack, see bpf_reg_state::buf, is + * not seen as an access to the stack. That's fine as long as nothing is tracked in the buffer. + * The output of helpers and kfuncs is not tracked. Here an insn stores to [min_off, max_off) + * of frame @frameno, a dynptr, an iterator or irq flags are made there, or the frame is gone. + */ +static void invalidate_stack_slices(struct bpf_verifier_env *env, u32 frameno, + int min_off, int max_off) +{ + struct bpf_func_state *state; + struct bpf_reg_state *reg; + + if (!env->seen_stack_slice) + return; + + bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({ + if (base_type(reg->type) != PTR_TO_MEM || !reg->buf.size || + reg->buf.frameno != frameno || reg->buf.off >= max_off || + reg->buf.off + (int)reg->buf.size <= min_off) + continue; + bpf_diag_record_scrub(env, reg, BPF_DIAG_MOD_SLICE_BUF); + mark_reg_invalid(env, reg); + })); +} + /* check_stack_write dispatches to check_stack_write_fixed_off or * check_stack_write_var_off. * @@ -4572,6 +4604,8 @@ static int check_stack_write(struct bpf_verifier_env *env, int value_regno, int insn_idx) { struct bpf_func_state *state = bpf_func(env, reg); + int min_off = reg_smin(reg) + off; + int max_off = reg_smax(reg) + off + size; int err; if (tnum_is_const(reg->var_off)) { @@ -4586,6 +4620,8 @@ static int check_stack_write(struct bpf_verifier_env *env, reg, off, size, value_regno, insn_idx); } + /* After the write is modelled: the insn may store a slice to its own buffer. */ + invalidate_stack_slices(env, state->frameno, min_off, max_off); return err; } @@ -10466,15 +10502,18 @@ static int mark_raw_stack(struct bpf_verifier_env *env, struct bpf_call_arg_meta */ for (slot = 0; slot < MAX_BPF_FUNC_ARGS; slot++) { struct bpf_reg_state *reg; - argno_t argno = argno_from_arg(slot + 1); if (!meta->arg_raw_mem.size[slot]) continue; reg = get_func_arg_reg(caller, cur_regs(env), slot); + /* + * check_stack_range_initialized() checked the range, its offset is a constant. + * Not check_stack_write(): unknown bytes in its buffer leave a dynptr slice valid. + */ for (i = 0; i < meta->arg_raw_mem.size[slot]; i++) { - err = check_mem_access(env, insn_idx, reg, argno, i, BPF_B, - BPF_WRITE, -1, false, false); + err = check_stack_write_fixed_off(env, bpf_func(env, reg), + reg->var_off.value + i, 1, -1, insn_idx); if (err) return err; } @@ -12176,6 +12215,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) callee->allocated_stack / BPF_REG_SIZE - 1); if (err) return err; + invalidate_stack_slices(env, callee->frameno, INT_MIN, 0); /* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite, * there function call logic would reschedule callback visit. If iteration @@ -15411,6 +15451,9 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_call_arg return -EFAULT; } regs[BPF_REG_0].parent_id = meta->dynptr.id; + regs[BPF_REG_0].buf = meta->slice_buf; + if (meta->slice_buf.size) + env->seen_stack_slice = true; } else { return 0; } @@ -15668,6 +15711,22 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, ref_convert_owning_non_owning(env, id); } + if ((is_kfunc_call(&meta, special_kfunc_list[KF_bpf_dynptr_slice]) || + is_kfunc_call(&meta, special_kfunc_list[KF_bpf_dynptr_slice_rdwr])) && + dynptr_slice_may_be_buffer(meta.dynptr.type)) { + struct bpf_reg_state *buf = ®s[BPF_REG_3]; + + if (buf->type == PTR_TO_STACK) { + meta.slice_buf.frameno = buf->frameno; + meta.slice_buf.off = reg_smin(buf); + meta.slice_buf.size = reg_smax(buf) - reg_smin(buf) + + meta.arg_constant.value; + } else if (base_type(buf->type) == PTR_TO_MEM) { + /* a slice is the buffer of the next one */ + meta.slice_buf = buf->buf; + } + } + if (meta.func_id == special_kfunc_list[KF_bpf_throw]) { if (!bpf_jit_supports_exceptions()) { verbose(env, "JIT does not support calling kfunc %s#%d\n",