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 Add verifier coverage for iterator-owned BTF pointer lifetimes. Cover current struct results, iterator transitions, trusted fields and FILE dynptr cleanup obligations. Keep current-result, explicit-discard and independent RCU lifetime controls. These annotations check verifier outcomes; they do not assert runtime execution of the newly added programs. Assisted-by: LLM Signed-off-by: Xu Yunxiang --- .../selftests/bpf/progs/iters_testmod.c | 332 ++++++++++++++++++ 1 file changed, 332 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/iters_testmod.c b/tools/testing/selftests/bpf/progs/iters_testmod.c index f65cc9766633e..81581459f11e3 100644 --- a/tools/testing/selftests/bpf/progs/iters_testmod.c +++ b/tools/testing/selftests/bpf/progs/iters_testmod.c @@ -28,6 +28,298 @@ int iter_next_trusted(const void *ctx) return 0; } +SEC("raw_tp/sys_enter") +__failure __msg("invalid mem access 'scalar'") +int iter_next_trusted_after_destroy(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + bpf_iter_task_vma_destroy(&vma_it); + return vma_ptr->vm_start; +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__failure __msg("invalid mem access 'scalar'") +int iter_next_trusted_after_next(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr, *next_vma_ptr; + u64 vm_start; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + next_vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!next_vma_ptr) + goto out; + + vm_start = vma_ptr->vm_start; + bpf_iter_task_vma_destroy(&vma_it); + return vm_start; +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__failure __msg("invalid mem access 'scalar'") +int iter_next_trusted_after_next_null(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr, *next_vma_ptr; + u64 vm_start; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + next_vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (next_vma_ptr) + goto out; + + vm_start = vma_ptr->vm_start; + bpf_iter_task_vma_destroy(&vma_it); + return vm_start; +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__failure __msg("invalid mem access 'scalar'") +int iter_next_trusted_field_after_destroy(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + struct file *file_ptr; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + file_ptr = vma_ptr->vm_file; + bpf_iter_task_vma_destroy(&vma_it); + if (file_ptr) + return file_ptr->f_mode; + return 0; +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__failure __msg("invalid mem access 'scalar'") +int iter_next_trusted_field_after_next(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr, *next_vma_ptr; + struct file *file_ptr; + u32 mode; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + file_ptr = vma_ptr->vm_file; + if (!file_ptr) + goto out; + + next_vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!next_vma_ptr) + goto out; + + mode = file_ptr->f_mode; + bpf_iter_task_vma_destroy(&vma_it); + return mode; +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__failure __msg("Leaking reference id=") +int iter_next_file_dynptr_after_next(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + struct bpf_dynptr dynptr; + struct file *file_ptr; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + file_ptr = vma_ptr->vm_file; + if (!file_ptr) + goto out; + + bpf_dynptr_from_file(file_ptr, 0, &dynptr); + bpf_iter_task_vma_next(&vma_it); + bpf_dynptr_file_discard(&dynptr); +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__failure __msg("Leaking reference id=") +int iter_next_file_dynptr_after_destroy(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + struct bpf_dynptr dynptr; + struct file *file_ptr; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + file_ptr = vma_ptr->vm_file; + if (!file_ptr) + goto out; + + bpf_dynptr_from_file(file_ptr, 0, &dynptr); + bpf_iter_task_vma_destroy(&vma_it); + bpf_dynptr_file_discard(&dynptr); + return 0; +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__success +int iter_next_file_dynptr_discard_before_advance(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + struct bpf_dynptr dynptr; + struct file *file_ptr; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + file_ptr = vma_ptr->vm_file; + if (!file_ptr) + goto out; + + bpf_dynptr_from_file(file_ptr, 0, &dynptr); + bpf_dynptr_file_discard(&dynptr); + bpf_iter_task_vma_next(&vma_it); +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__success +int iter_next_trusted_current_after_next(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (!vma_ptr) + goto out; + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (vma_ptr) + bpf_kfunc_trusted_vma_test(vma_ptr); +out: + bpf_iter_task_vma_destroy(&vma_it); + return 0; +} + +SEC("raw_tp/sys_enter") +__success +int iter_next_trusted_rcu_field_after_destroy(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + struct mm_struct *mm_ptr; + struct file *file_ptr = NULL; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (vma_ptr) { + mm_ptr = vma_ptr->vm_mm; + /* exe_file has RCU protection independent of the iterator. */ + if (mm_ptr) + file_ptr = mm_ptr->exe_file; + } + + bpf_iter_task_vma_destroy(&vma_it); + if (file_ptr) + return file_ptr->f_mode; + return 0; +} + +SEC("raw_tp/sys_enter") +__success +int iter_next_trusted_rcu_field_after_next(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task_vma vma_it; + struct vm_area_struct *vma_ptr; + struct mm_struct *mm_ptr; + struct file *file_ptr = NULL; + u32 mode = 0; + + bpf_iter_task_vma_new(&vma_it, cur_task, 0); + + vma_ptr = bpf_iter_task_vma_next(&vma_it); + if (vma_ptr) { + mm_ptr = vma_ptr->vm_mm; + if (mm_ptr) + file_ptr = mm_ptr->exe_file; + } + + bpf_iter_task_vma_next(&vma_it); + if (file_ptr) + mode = file_ptr->f_mode; + bpf_iter_task_vma_destroy(&vma_it); + return mode; +} + SEC("raw_tp/sys_enter") __failure __msg("Possibly NULL pointer passed to trusted R1") int iter_next_trusted_or_null(const void *ctx) @@ -66,6 +358,46 @@ int iter_next_rcu(const void *ctx) return 0; } +SEC("raw_tp/sys_enter") +__success +int iter_next_rcu_after_destroy(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task task_it; + struct task_struct *task_ptr; + + bpf_iter_task_new(&task_it, cur_task, 0); + + task_ptr = bpf_iter_task_next(&task_it); + bpf_iter_task_destroy(&task_it); + if (!task_ptr) + return 0; + + bpf_kfunc_rcu_task_test(task_ptr); + return 0; +} + +SEC("raw_tp/sys_enter") +__success +int iter_next_rcu_after_next(const void *ctx) +{ + struct task_struct *cur_task = bpf_get_current_task_btf(); + struct bpf_iter_task task_it; + struct task_struct *task_ptr; + + bpf_iter_task_new(&task_it, cur_task, 0); + + task_ptr = bpf_iter_task_next(&task_it); + if (!task_ptr) + goto out; + + bpf_iter_task_next(&task_it); + bpf_kfunc_rcu_task_test(task_ptr); +out: + bpf_iter_task_destroy(&task_it); + return 0; +} + SEC("raw_tp/sys_enter") __failure __msg("Possibly NULL pointer passed to trusted R1") int iter_next_rcu_or_null(const void *ctx) -- 2.43.0