When checking whether a referenced dynptr can be overwritten, destroy_if_dynptr_stack_slot only counted sibling dynptrs in the current call frame. If a clone sharing the same virtual ref parent existed in a different frame (e.g., passed to a subprog), it would not be counted, causing the verifier to incorrectly reject the overwrite with "cannot overwrite referenced dynptr". Fix by extracting the counting into dynptr_ref_cnt() which uses bpf_for_each_reg_in_vstate_mask() to scan dynptr stack slots across all call frames. Fixes: 017f5c4ef73c ("bpf: Allow overwriting referenced dynptr when refcnt > 1") Reported-by: Eduard Zingerman Acked-by: Eduard Zingerman Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 52 ++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 375e5e5f9131..3a4233396aae 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -786,10 +786,29 @@ static void mark_reg_invalid(const struct bpf_verifier_env *env, struct bpf_reg_ __mark_reg_unknown(env, reg); } +static int dynptr_ref_cnt(struct bpf_verifier_env *env, int v_parent_id) +{ + struct bpf_stack_state *stack; + struct bpf_func_state *state; + struct bpf_reg_state *reg; + int ref_cnt = 0; + + bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, stack, 1 << STACK_DYNPTR, ({ + if (!stack || stack->slot_type[0] != STACK_DYNPTR) + continue; + if (!stack->spilled_ptr.dynptr.first_slot) + continue; + if (stack->spilled_ptr.parent_id == v_parent_id) + ref_cnt++; + })); + + return ref_cnt; +} + static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, struct bpf_func_state *state, int spi) { - int i, err = 0; + int err = 0; /* We always ensure that STACK_DYNPTR is never set partially, * hence just checking for slot_type[0] is enough. This is @@ -803,28 +822,15 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, if (!state->stack[spi].spilled_ptr.dynptr.first_slot) spi = spi + 1; - if (dynptr_type_referenced(state->stack[spi].spilled_ptr.dynptr.type)) { - int v_parent_id = state->stack[spi].spilled_ptr.parent_id; - int ref_cnt = 0; - - /* - * A referenced dynptr can be overwritten only if there is at - * least one other dynptr sharing the same virtual ref parent, - * ensuring the reference can still be properly released. - */ - for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) { - if (state->stack[i].slot_type[0] != STACK_DYNPTR) - continue; - if (!state->stack[i].spilled_ptr.dynptr.first_slot) - continue; - if (state->stack[i].spilled_ptr.parent_id == v_parent_id) - ref_cnt++; - } - - if (ref_cnt <= 1) { - verbose(env, "cannot overwrite referenced dynptr\n"); - return -EINVAL; - } + /* + * A referenced dynptr can be overwritten only if there is at + * least one other dynptr sharing the same virtual ref parent, + * ensuring the reference can still be properly released. + */ + if (dynptr_type_referenced(state->stack[spi].spilled_ptr.dynptr.type) && + dynptr_ref_cnt(env, state->stack[spi].spilled_ptr.parent_id) <= 1) { + verbose(env, "cannot overwrite referenced dynptr\n"); + return -EINVAL; } /* Invalidate the dynptr and any derived slices */ -- 2.53.0-Meta