Non-RCU struct iterator results borrow their lifetime from the iterator's current element. Associate these PTR_TO_BTF_ID results with the iterator reference and invalidate the previous result when processing the next iterator argument. This applies before modeling either next outcome; destroy continues to invalidate the result through reference release. Preserve that lifetime when loading a fully trusted BTF field. Capture the source lifetime before marking the destination, as the registers can alias. Keep RCU, user and percpu field results under their existing rules. A borrowed field can also back a dynptr. For example, a FILE dynptr owns reader state but borrows its file. Record the backing register's lifetime in the common dynptr constructor path, using the acquired ID for an owned reference and parent_id for a borrowed pointer. Keep clone handling and the ownership meaning of reg_is_referenced() unchanged. Reuse the existing child-reference leak check before advancing an iterator and propagate destroy errors. Outstanding child resources must be explicitly released before their backing lifetime ends. Preserve the existing rule that only the initial ID is removed from acquired references when release_reference() invalidates descendants. This issue was found during BPF verifier testing with an in-house runtime semantic checker. Fixes: 4cbee026db54 ("bpf: return VMA snapshot from task_vma iterator") Suggested-by: Amery Hung Assisted-by: LLM Signed-off-by: Xu Yunxiang --- kernel/bpf/verifier.c | 88 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 73 insertions(+), 15 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6c6b8d8520cdf..3d47487233b75 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -209,6 +209,9 @@ static int acquire_reference(struct bpf_verifier_env *env, int insn_idx, int par static int __release_reference_nomark(struct bpf_verifier_state *state, int id); static int release_reference_nomark(struct bpf_verifier_env *env, int id); static int release_reference(struct bpf_verifier_env *env, int id); +static int check_reference_children_leak(struct bpf_verifier_env *env, int parent_id); +static u32 reg_lifetime_id(struct bpf_verifier_env *env, + const struct bpf_reg_state *reg); static void invalidate_non_owning_refs(struct bpf_verifier_env *env); static void invalidate_rcu_protected_refs(struct bpf_verifier_env *env); static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env); @@ -753,8 +756,8 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (err) return err; - /* Track parent's id if the parent is a referenced object */ - parent_id = ref_obj->id; + /* All non-clone constructors take their backing object in R1. */ + parent_id = reg_lifetime_id(env, &cur_regs(env)[BPF_REG_1]); if (dynptr_type_referenced(type)) { int id; @@ -1024,7 +1027,7 @@ static int unmark_stack_slots_iter(struct bpf_verifier_env *env, struct bpf_reg_state *reg, int nr_slots) { struct bpf_func_state *state = bpf_func(env, reg); - int spi, i, j; + int spi, i, j, err; spi = iter_get_spi(env, reg, nr_slots); if (spi < 0) @@ -1034,8 +1037,11 @@ static int unmark_stack_slots_iter(struct bpf_verifier_env *env, struct bpf_stack_state *slot = &state->stack[spi - i]; struct bpf_reg_state *st = &slot->spilled_ptr; - if (i == 0) - WARN_ON_ONCE(release_reference(env, st->id)); + if (i == 0) { + err = release_reference(env, st->id); + if (err) + return err; + } bpf_mark_reg_not_init(env, st); @@ -1577,6 +1583,12 @@ static bool reg_is_referenced(struct bpf_verifier_env *env, const struct bpf_reg return find_reference_state(env->cur_state, reg->id); } +static u32 reg_lifetime_id(struct bpf_verifier_env *env, + const struct bpf_reg_state *reg) +{ + return reg_is_referenced(env, reg) ? reg->id : reg->parent_id; +} + static int release_lock_state(struct bpf_verifier_env *env, int type, int id, void *ptr) { struct bpf_verifier_state *state = env->cur_state; @@ -6219,9 +6231,14 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, } if (atype == BPF_READ && value_regno >= 0) { + u32 parent_id = reg_lifetime_id(env, reg); + ret = mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag); if (ret < 0) return ret; + if ((regs[value_regno].type & PTR_TRUSTED) && + !(regs[value_regno].type & (MEM_RCU | MEM_PERCPU | MEM_USER))) + regs[value_regno].parent_id = parent_id; } return 0; @@ -7817,6 +7834,27 @@ static bool is_kfunc_arg_iter(struct bpf_call_arg_meta *meta, int arg_idx, return btf_param_match_suffix(meta->btf, arg, "__iter"); } +static int invalidate_iter_owned_btf_ptrs(struct bpf_verifier_env *env, u32 parent_id) +{ + struct bpf_func_state *unused; + struct bpf_reg_state *reg; + int err; + + err = check_reference_children_leak(env, parent_id); + if (err) + return err; + + /* Struct iterators can release their previous element on next. */ + bpf_for_each_reg_in_vstate(env->cur_state, unused, reg, ({ + if (base_type(reg->type) != PTR_TO_BTF_ID || reg->parent_id != parent_id) + continue; + bpf_diag_record_scrub(env, reg, BPF_DIAG_MOD_REF_RELEASE); + mark_reg_invalid(env, reg); + })); + + return 0; +} + static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, int insn_idx, struct bpf_call_arg_meta *meta) { @@ -7914,6 +7952,13 @@ static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state * meta->iter.frameno = reg->frameno; update_ref_obj(&meta->ref_obj, &state->stack[spi].spilled_ptr); + if (bpf_is_iter_next_kfunc(meta) && + !(state->stack[spi].spilled_ptr.type & MEM_RCU)) { + err = invalidate_iter_owned_btf_ptrs(env, meta->ref_obj.id); + if (err) + return err; + } + if (is_iter_destroy_kfunc(meta)) { err = unmark_stack_slots_iter(env, reg, nr_slots); if (err) @@ -10088,6 +10133,23 @@ static int idstack_pop(struct bpf_idmap *idmap) return idmap->map[--idmap->cnt].old; } +static int check_reference_children_leak(struct bpf_verifier_env *env, int parent_id) +{ + struct bpf_verifier_state *state = env->cur_state; + int i; + + for (i = 0; i < state->acquired_refs; i++) { + if (state->refs[i].type != REF_TYPE_PTR || + state->refs[i].parent_id != parent_id) + continue; + verbose(env, "Leaking reference id=%d alloc_insn=%d. Release it first.\n", + state->refs[i].id, state->refs[i].insn_idx); + return -EINVAL; + } + + return 0; +} + /* Release id and objects derived from it iteratively in a DFS manner */ static int release_reference(struct bpf_verifier_env *env, int id) { @@ -10097,7 +10159,7 @@ static int release_reference(struct bpf_verifier_env *env, int id) struct bpf_stack_state *stack; struct bpf_func_state *state; struct bpf_reg_state *reg; - int i, err; + int err; idstack->cnt = 0; err = idstack_push(idstack, id); @@ -10114,15 +10176,9 @@ static int release_reference(struct bpf_verifier_env *env, int id) * Child references are inaccessible after parent is released, * any child references that exist at this point are a leak. */ - for (i = 0; i < vstate->acquired_refs; i++) { - if (vstate->refs[i].type != REF_TYPE_PTR) - continue; - if (vstate->refs[i].parent_id != id) - continue; - verbose(env, "Leaking reference id=%d alloc_insn=%d. Release it first.\n", - vstate->refs[i].id, vstate->refs[i].insn_idx); - return -EINVAL; - } + err = check_reference_children_leak(env, id); + if (err) + return err; bpf_for_each_reg_in_vstate_mask(vstate, state, reg, stack, mask, ({ if (reg->id != id && reg->parent_id != id) @@ -14446,6 +14502,8 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, regs[BPF_REG_0].btf = desc_btf; regs[BPF_REG_0].type = type; regs[BPF_REG_0].btf_id = ptr_type_id; + if (bpf_is_iter_next_kfunc(&meta) && !(type & MEM_RCU)) + regs[BPF_REG_0].parent_id = meta.ref_obj.id; } if (is_kfunc_ret_null(&meta)) { -- 2.43.0