[ Upstream commit 308c7a0ae8859b34d9d90a3dff953b2d14242145 ] This is a 6.12.y-sized equivalent, not a cherry-pick of the upstream diff. unmark_stack_slots_dynptr() walked clones only in the submitting pointer's frame. A program that clones a ringbuf dynptr in a subprog and submits the clone leaves the original STACK_DYNPTR slot active in the caller. The caller can then pass that stale original to bpf_dynptr_data(), even though the shared ringbuf reference has already been released. Walk every frame and invalidate matching STACK_DYNPTR slots. Check the slot type before reading dynptr metadata: a partial stack overwrite can turn a pointer spill into ordinary stack data while stale union fields remain, and those fields must not be interpreted as a dynptr ref_obj_id. The preceding patch makes a repeated release an ordinary verifier rejection, but it does not prevent non-release uses of a stale dynptr. Depends on "bpf: Invalidate dynptr slices by dynptr_id on release" only for the slice helper call inside the new loop. The upstream commit is Amery Hung's parent_id refactor. It cannot be applied here (see the first patch). Deviations from 308c7a0ae8859: keep dynptr_id / ref_obj_id; walk frames with the 6.12 STACK_DYNPTR slot loop instead of the parent_id DFS. Fixes: 361f129f3cc1 ("bpf: Add bpf_dynptr_clone") Reported-by: Xu Yunxiang Closes: https://lore.kernel.org/r/20260829033725.2365697-3-xyx2021@mail.ustc.edu.cn Assisted-by: Pi:GLM-5.3 Signed-off-by: Xu Yunxiang --- kernel/bpf/verifier.c | 30 +++++++-------- .../testing/selftests/bpf/progs/dynptr_fail.c | 37 +++++++++++++++++++ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 65ee9afaa5ff..572437d48b7d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -811,7 +811,7 @@ static void invalidate_slices_of_dynptr(struct bpf_verifier_env *env, int dynptr static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); - int spi, ref_obj_id, i, err; + int spi, ref_obj_id, i, err, frm; spi = dynptr_get_spi(env, reg); if (spi < 0) @@ -843,22 +843,22 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re return err; invalidate_slices_of_dynptr(env, state->stack[spi].spilled_ptr.id); - /* Invalidate any dynptr clones */ - for (i = 1; i < state->allocated_stack / BPF_REG_SIZE; i++) { - if (state->stack[i].spilled_ptr.ref_obj_id != ref_obj_id) + /* Invalidate any dynptr clones, including those in other frames. */ + for (frm = 0; frm <= env->cur_state->curframe; frm++) { + struct bpf_func_state *f = env->cur_state->frame[frm]; + + if (!f) continue; + for (i = 1; i < f->allocated_stack / BPF_REG_SIZE; i++) { + if (f->stack[i].slot_type[0] != STACK_DYNPTR) + continue; + if (f->stack[i].spilled_ptr.ref_obj_id != ref_obj_id) + continue; - /* it should always be the case that if the ref obj id - * matches then the stack slot also belongs to a - * dynptr - */ - if (state->stack[i].slot_type[0] != STACK_DYNPTR) { - verbose(env, "verifier internal error: misconfigured ref_obj_id\n"); - return -EFAULT; - } - if (state->stack[i].spilled_ptr.dynptr.first_slot) { - invalidate_slices_of_dynptr(env, state->stack[i].spilled_ptr.id); - invalidate_dynptr(env, state, i); + if (f->stack[i].spilled_ptr.dynptr.first_slot) { + invalidate_slices_of_dynptr(env, f->stack[i].spilled_ptr.id); + invalidate_dynptr(env, f, i); + } } } diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 52bd7f8871c5..02defdb63ddd 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -1534,6 +1534,43 @@ int clone_invalid2(struct xdp_md *xdp) } /* Invalidating a dynptr should invalidate its clones */ +static __noinline void submit_dynptr_clone(struct bpf_dynptr *ptr) +{ + struct bpf_dynptr clone; + + bpf_dynptr_clone(ptr, &clone); + bpf_ringbuf_submit_dynptr(&clone, 0); +} + +SEC("?raw_tp") +__failure __msg("Expected an initialized dynptr") +int clone_invalidate_cross_frame(void *ctx) +{ + struct bpf_dynptr ptr; + + bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(struct sample), 0, &ptr); + submit_dynptr_clone(&ptr); + bpf_dynptr_data(&ptr, 0, sizeof(struct sample)); + return 0; +} + +SEC("?raw_tp") +__success +int clone_invalidate_cross_frame_stale_spill(void *ctx) +{ + struct bpf_dynptr ptr; + void *data; + + bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(struct sample), 0, &ptr); + data = bpf_dynptr_data(&ptr, 0, sizeof(struct sample)); + asm volatile("*(u64 *)(r10 - 24) = %[data];" + "*(u32 *)(r10 - 20) = 0x12345678;" + :: [data] "r"(data) : "memory"); + /* A dead, scrubbed spill must not be mistaken for a dynptr slot. */ + submit_dynptr_clone(&ptr); + return 0; +} + SEC("?raw_tp") __failure __msg("Expected an initialized dynptr as arg #2") int clone_invalidate1(void *ctx) -- 2.43.0