Unwinding for synchronous callbacks

include/linux/bpf_unwind.h line 184

[Severity: High]
Can evaluating the macro arguments in ___bpf_call_cb() on arm64 and __bpf_call_cb() on x86-64 clobber the caller-saved local register variables before the asm_goto_output() statement is reached?

On arm64, ___bpf_call_cb() in include/linux/bpf_unwind.h initializes x15 and x0..x4 directly from the macro expressions in sequence:

	register u64 __fn asm("x15") = (u64)(long)(fn);
	register u64 __a0 asm("x0") = (u64)(long)(a1);
	register u64 __a1 asm("x1") = (u64)(long)(a2);
	register u64 __a2 asm("x2") = (u64)(long)(a3);
	register u64 __a3 asm("x3") = (u64)(long)(a4);
	register u64 __a4 asm("x4") = (u64)(long)(a5);

And on x86-64, __bpf_call_cb() assigns __fn (r11) and __a5 (r8) before evaluating a1..a4 in the asm_goto_output() input operands:

	register u64 __fn asm("r11") = (u64)(long)(fn);
	register u64 __a5 asm("r8") = (u64)(long)(a5);
	...
		 : [thunk_target] "r"(__fn), "D"((u64)(long)(a1)),
		   "S"((u64)(long)(a2)), "d"((u64)(long)(a3)),
		   "c"((u64)(long)(a4)), "r"(__a5)

Per GCC and Clang local register variable rules, local register variables do not reserve caller-saved registers across function calls that occur between their assignment and the extended asm statement.

Later in this series, commit "bpf: Let the callback of bpf_for_each_map_elem unwind" updates bpf_each_rhash_elem() in kernel/bpf/hashtab.c to pass a helper call and pointer dereferences as macro arguments:

	bpf_call_cb(callback_fn, map, elem->data,
		    rhtab_elem_value(elem, map->key_size), callback_ctx, 0,
		    ret, unwound);

If evaluating a3 or another argument invokes a function call -- such as an out-of-line call to rhtab_elem_value() (which is marked static inline rather than __always_inline) or sanitizer instrumentation like __asan_load4() under outline KASAN or __tsan_read4() under KCSAN -- that call executes after __fn (x15 / r11) and earlier argument registers (x0, x1 / r8) have already been populated, clobbering the callback target and arguments prior to blr x15 or CALL_NOSPEC.

Should fn and a1..a5 be evaluated into ordinary C temporary variables first before assigning them to the local register variables, similar to how include/linux/arm-smccc.h stages register arguments?

include/linux/bpf_unwind.h line 253

[Severity: High]
Could the compiler eliminate the *slot = to store in bpf_unwind_leave() (on both x86-64 and arm64) as a dead store to the current function's stack frame right before returning?

In include/linux/bpf_unwind.h, both implementations of bpf_unwind_leave() compute slot from __builtin_dwarf_cfa() or __builtin_frame_address(0) as a non-volatile unsigned long * and perform a plain store:

	*slot = to;

Because bpf_unwind_leave() is __always_inline and runs at the end of the helper's unwound path immediately before return, nothing in C reads *slot before the function exits. In GCC's RTL dead-store elimination pass (pass_rtl_dse1, which runs before prologue/epilogue threading), non-volatile frame-related stores based on arg_pointer_rtx (__builtin_dwarf_cfa()) or hard_frame_pointer_rtx (__builtin_frame_address(0)) that are not read before EXIT_BLOCK are candidates for elimination. If *slot = to is elided, the helper returns to its normal call site instead of the caller's unwind pad, losing the BPF exception after the helper has already released its resources.

Existing kernel code that overwrites the stack return address via __builtin_frame_address(0) in drivers/misc/lkdtm/cfi.c:set_return_addr() explicitly marks the store volatile for this reason:

	/* Use of volatile is to make sure final write isn't seen as a dead store. */
	unsigned long * volatile *ret_addr =
		(unsigned long **)__builtin_frame_address(0) + FRAME_RA_OFFSET;

	/* Make sure we've found the right place on the stack before writing it. */
	if (no_pac_addr(*ret_addr) == expected)
		*ret_addr = (addr);

Should bpf_unwind_leave() use WRITE_ONCE(*slot, to) (and WRITE_ONCE(scs[-1], to) on arm64) or qualify slot as volatile so the return-address update cannot be optimized away?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

include/linux/bpf_unwind.h line 112

Can the argument evaluation here clobber the register variables?

__fn is bound to r11 and __a5 to r8 before a1 through a4 are evaluated as asm inputs. The arm64 version has the same shape, binding x15 and x0 through x4 one after another before later arguments are evaluated.

With gcc, a register variable is only guaranteed to hold its register at the asm statement itself. If evaluating a later macro argument involves a call, the earlier binding is not kept across it.

A later patch in this series (7a687425732b) calls:

    bpf_call_cb(callback_fn, map, elem->data,
                rhtab_elem_value(elem, map->key_size), callback_ctx, 0,
                ret, unwound);

The map->key_size load becomes a call to __tsan_read4 with CONFIG_KCSAN, or to __asan_load4_noabort with CONFIG_KASAN_OUTLINE, and kernel/bpf is not excluded from either. That call is placed after the register variables have been set.

On arm64 the call's own argument goes in x0, so the callback could get a garbage map pointer, and x15 (the call target) can be clobbered as well. On x86 r11 and r8 are call-clobbered, so the call __x86_indirect_thunk_r11 that follows could jump to a garbage address.

clang ties register variables only at the asm itself, so this would only affect gcc builds.

Would it be safer to evaluate fn and a1 through a5 into ordinary temporaries first, for example typeof(a) __t = a;, and only then declare the register variables, the way the arm-smccc.h macros do?

include/linux/bpf_unwind.h line 126

Can *slot hold something other than the address in the bpf program that called the helper?

bpf_unwind_leave() assumes that it does, and the comment says a miss is a bug. While the helper is traced, though, the slot can be replaced:

- function_graph: prepare_ftrace_return()/ftrace_graph_func() put return_to_handler in *parent
- kretprobe/rethook: arch_rethook_trampoline is put there
- a bpf fexit/fmod_ret trampoline calls the original function, so the return address is inside the trampoline

On arm64 the same replacement is made to the link register at entry, and the prologue then saves it into the frame record that bpf_unwind_leave() reads.

kernel/bpf/Makefile removes CC_FLAGS_FTRACE only from percpu_freelist.o, bpf_lru_list.o, queue_stack_maps.o, lpm_trie.o, ringbuf.o and rqspinlock.o. So bpf_loop() (bpf_iter.c), the map iterators (hashtab.c, arraymap.c), bpf_find_vma() (task_iter.c) and bpf_rbtree_add (helpers.c) can all be graph-traced. They can also be kretprobed, and so can bpf_user_ringbuf_drain().

Later commits in this series give these helpers pads (4e61c40fe5c0, 7a687425732b, 59e3a600668d, bee41eec3c39, ecd4c98df2ee) and add no notrace, NOKPROBE or attach restriction.

If the callback unwinds in that state, bpf_unwind_target(return_to_handler) misses in the bpf table and in the kernel table, and returns its argument:

    unsigned long to = bpf_unwind_target(*slot);   /* to == *slot */

    WARN_ON_ONCE(to == *slot);                     /* fires */
    *slot = to;                                    /* unchanged */

The helper then returns through the tracer to the normal continuation after the call instead of to the pad, and the exception is lost after the helper has already stopped and dropped what it holds.

There is also a typing consequence. According to b2f03cf6322f the pad gets the callback's r0 bit for bit, "a pointer stays the pointer it was, with its type and its reference". On the fallthrough path the verifier instead types r0 as the helper's own return (RET_INTEGER, for example). So a pointer or a referenced object the callback unwound with reaches code that treats it as a scalar, and the pad that would have consumed or released it never runs.

bpf_check_attach_target() already refuses the analogous case for bpf
targets: "Only fentry can attach to %s, which can unwind". Is there something equivalent for the C helpers that have a pad?

Would translating the slot with ftrace_graph_ret_addr() and the rethook equivalent (and refusing fexit/fmod_ret on these helpers) cover this? Alternatively, could the helpers with pads be made notrace and NOKPROBE, with tracing attachment to them denied?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40612543702

arch/arm64/net/bpf_jit_comp.c struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr · line 2613

[Severity: Medium]
Can this leak ctx.landing_ex if multi-function JIT compilation fails between passes?

During the first pass of jit_subprogs() -> bpf_int_jit_compile() for a subprogram (where prog->is_func is true and extra_pass is false), ctx is saved in prog->aux->jit_data and the cleanup block is skipped so that ctx.landing_ex and ctx.offset remain available for the second pass:

arch/arm64/net/bpf_jit_comp.c:bpf_int_jit_compile() {
    ...
	if (!prog->is_func || extra_pass) {
		...
	} else {
		jit_data->ctx = ctx;
		jit_data->ro_image = ro_image_ptr;
		jit_data->header = header;
		jit_data->ro_header = ro_header;
	}

	prog->bpf_func = (void *)ctx.ro_image + cfi_get_offset();
	prog->jited = 1;
	prog->jited_len = prog_size - cfi_get_offset();

	if (!prog->is_func || extra_pass) {
		...
		kvfree(ctx.offset);
		kvfree(ctx.landing_ex);
out_priv_stack:
		kfree(jit_data);
		prog->aux->jit_data = NULL;
	}
    ...
}

If a subsequent subprogram fails its initial JIT pass in jit_subprogs(), or if an earlier subprogram fails during the extra_pass loop, jit_subprogs() jumps to out_free and calls bpf_jit_free() on each subprogram. However, bpf_jit_free() frees jit_data without freeing jit_data->ctx.landing_ex (or the pre-existing jit_data->ctx.offset):

arch/arm64/net/bpf_jit_comp.c:bpf_jit_free() {
	if (prog->jited) {
		struct arm64_jit_data *jit_data = prog->aux->jit_data;
		...
		if (jit_data) {
			bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header);
			kfree(jit_data);
		}
    ...
}

Should bpf_jit_free() also call kvfree(jit_data->ctx.landing_ex) and kvfree(jit_data->ctx.offset) before freeing jit_data?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

kernel/bpf/verifier.c static int check_unwind_insns(struct bpf_verifier_env *env) · line 3462

[Severity: Medium]
What happens when unwind_goto follows a call to bpf_loop() that gets inlined by bpf_optimize_bpf_loop() -> inline_bpf_loop(), but the callback does not unwind (!env->subprog_info[subprog].can_unwind)?

Here check_unwind_insns() permits unwind_goto after any bpf_is_sync_callback_calling_insn(insn - 1), and check_cb_unwind() returns 0 when !env->subprog_info[subprog].can_unwind.

When bpf_loop() is inlined in kernel/bpf/fixups.c:inline_bpf_loop() (both here and after the later commit "bpf: Give the inlined bpf_loop a pad" when !can_unwind):

	insn_buf[cnt++] = BPF_JMP_IMM(BPF_JLE, BPF_REG_1, BPF_MAX_LOOPS, 2);
	insn_buf[cnt++] = BPF_MOV32_IMM(BPF_REG_0, -E2BIG);
	insn_buf[cnt++] = BPF_JMP_IMM(BPF_JA, 0, 0, 16);
	...
	insn_buf[cnt++] = BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_10, r8_offset);

the call instruction is replaced with 19 instructions ending in BPF_LDX_MEM without converting the trailing unwind_goto into BPF_JMP_A(0), and the BPF_JMP_IMM(BPF_JA, 0, 0, 16) instruction jumps directly to that unwind_goto.

If the target of that unwind_goto was marked seen during verification (for example, a shared pad or unwind_goto with offset 0) or when !is_priv, bpf_opt_hard_wire_dead_code_branches() leaves the unwind_goto instruction in place:

	if (bpf_is_unwind_goto(insn) && aux_data[i + 1 + insn->off].seen) {
		if (!aux_data[i + 1].seen) {
			memcpy(insn + 1, &trap, sizeof(trap));
			aux_data[i + 1].seen = env->pass_cnt;
		}
		continue;
	}

Wouldn't this leave unwind_goto immediately after BPF_LDX_MEM instead of a call instruction (and targeted by a jump), violating the JIT invariant in arch/x86/net/bpf_jit_comp.c and arch/arm64/net/bpf_jit_comp.c (including the x86-64 priv_frame_ptr path that assumes insn[i - 1] is a call ending in pop r9 and keys the exception table at addrs[i - 1] - 2)?

Conversely, in the later commit "bpf: Give the inlined bpf_loop a pad", if can_unwind is true for the callback subprog, but R1 (nr_loops) is known to be 0 at the bpf_loop() callsite, check_helper_call() skips push_callback_call() so the unwind pad is never verified. Yet inline_bpf_loop() sees subprog_info[callback_subprogno].can_unwind && bpf_is_unwind_goto(insn + 1) and emits an inlined pad jumping via BPF_JMP32_A to an unverified pad that bpf_opt_remove_dead_code() then strips.

Should inline_bpf_loop() rewrite a trailing unwind_goto to BPF_JMP_A(0) when the callback does not unwind during the call, or only emit the pad when the unwind_goto target was actually verified?

[ ... ]

kernel/bpf/verifier.c static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) · line 12201

[Severity: High]
Can a callback with a 9..16 byte BTF return type clobber the caller's R2 argument register when it exits via unwind_exit?

In bpf_compute_subprog_ret_regs(), ret_reg_pair is set for any static subprog whose BTF return type is 9..16 bytes, and btf_check_subprog_call() in push_callback_call() does not reject this for callbacks. At the top of prepare_func_exit():

	callee = state->frame[state->curframe];
	r0 = &callee->regs[BPF_REG_0];
	nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1;

When callee->in_callback_fn && unwinds is true, prepare_func_exit() now takes the else branch:

	} else {
		/*
		 * return to the caller whatever the callee had in the
		 * return register(s)
		 */
		for (i = 0; i < nregs; i++) {
			u32 regno = ret_regs[i];

			bpf_diag_mod_begin(env, &caller->regs[regno], &callee->regs[regno],
					   BPF_DIAG_MOD_WRITE);
			caller->regs[regno] = callee->regs[regno];
			bpf_diag_mod_end(env);
		}
	}

If nregs == 2, this copies callee->regs[BPF_REG_2] into caller->regs[BPF_REG_2] before redirecting *insn_idx back to callee->callsite.

When the verifier revisits the call instruction at callee->callsite, check_helper_call() or check_kfunc_call() re-runs argument validation and side effects using the overwritten caller->regs[BPF_REG_2]. In particular, for bpf_rbtree_add_impl(root, node, less, ...), R2 is the owning reference to the node being added:

	if (is_bpf_list_push_kfunc(meta.func_id) || is_bpf_rbtree_add_kfunc(meta.func_id)) {
		id = regs[BPF_REG_2].id;
		insn_aux->insert_off = regs[BPF_REG_2].var_off.value;
		insn_aux->kptr_struct_meta = btf_find_struct_meta(meta.arg_btf, meta.arg_btf_id);
		ref_convert_owning_non_owning(env, id);
	}

If the callback places a different allocated node pointer in its R2 before unwind_exit, wouldn't the revisit of check_kfunc_call() release the wrong reference ID in the verifier while bpf_rbtree_add_impl() drops the original node at runtime, leaving a dangling owning reference to the freed original node in the caller's unwind pad?

Should nregs be forced to 1 when callee->in_callback_fn is true (or should callbacks with ret_reg_pair be rejected)?

[ ... ]

kernel/bpf/verifier.c static int do_check(struct bpf_verifier_env *env) · line 20699

[Severity: Medium]
Can revisiting the call instruction with state->unwound set trigger verifier_bug_if() in do_check()'s precomputed constant register check?

Before do_check() runs, bpf_compute_const_regs() computes intraprocedural constant register values at each instruction. If the caller holds a known 32-bit constant C1 in R0 before a callback-calling helper or kfunc instruction (for instance, r0 = 0; call bpf_loop; unwind_goto +pad;), bpf_compute_const_regs() sets BIT(BPF_REG_0) in insn_aux->const_reg_mask and records C1 in insn_aux->const_reg_vals[BPF_REG_0].

When the callback unwinds with a different constant scalar C2 in R0, prepare_func_exit() sets state->unwound = true, copies C2 into caller->regs[BPF_REG_0], and sets *insn_idx = callee->callsite. On the revisit of the call instruction in do_check(), just below this hunk:

		/* Sanity check: precomputed constants must match verifier state */
		if (!state->speculative && insn_aux->const_reg_mask) {
			struct bpf_reg_state *regs = cur_regs(env);
			u16 mask = insn_aux->const_reg_mask;

			for (int r = 0; r < ARRAY_SIZE(insn_aux->const_reg_vals); r++) {
				u32 cval = insn_aux->const_reg_vals[r];

				if (!(mask & BIT(r)))
					continue;
				if (regs[r].type != SCALAR_VALUE)
					continue;
				if (!tnum_is_const(regs[r].var_off))
					continue;
				if (verifier_bug_if((u32)regs[r].var_off.value != cval,
						    env, "const R%d: %u != %llu",
						    r, cval, regs[r].var_off.value))
					return -EFAULT;
			}
		}

Because regs[BPF_REG_0] now holds the callback's unwound constant C2 rather than the caller's pre-call constant C1, wouldn't this fire verifier_bug_if() and fail verification with -EFAULT unless BPF_REG_0 is skipped when state->unwound is true?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

kernel/bpf/backtrack.c static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx, · line 580

This isn't a bug, but the comment at the top of the BPF_EXIT case says that the precision of the return registers is never propagated for an exit from a callback:

* case, we need to propagate the precision of the
* return registers, if necessary. In the latter we
* never do that.

With this change, unwind_exit of a callback sets cb_unwinds, and r0_precise = (from_subprog_call || cb_unwinds) && bt_is_reg_set(bt, BPF_REG_0) does propagate r0 to the callback frame.

The new comment above cb_unwinds describes this, but the old sentence is still there, so the same case has two comments that disagree.

Should the old comment say that the return registers of a callback are dropped unless the callback left with unwind_exit?

[ ... ]

kernel/bpf/verifier.c static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog) · line 11441

Can a callback that unwinds through one of these helpers end up on the fall-through path of the call instead of at the pad?

check_cb_unwind() accepts the unwind based on BPF_HAS_CB_PADS and insn_has_cb_pad(), which are static properties. At run time the pad is only reached if the return address slot of the helper still holds the address in the BPF program. bpf_unwind_leave() does this:

    unsigned long *slot = (unsigned long *)__builtin_dwarf_cfa() - 1;
    unsigned long to = bpf_unwind_target(*slot);

    WARN_ON_ONCE(to == *slot);
    *slot = to;

bpf_unwind_target() looks up *slot in the BPF and kernel extables, and several kinds of tracing replace or move that return address:

- function_graph: prepare_ftrace_return() does *parent = return_hooker
- kretprobe/rethook and fprobe exit handlers (kprobe.multi return, session)
- a BPF fexit/fmod_ret trampoline: with BPF_TRAMP_F_CALL_ORIG it calls orig_call from the trampoline, so the helper returns into the trampoline

None of the helpers that get a pad later in the series is protected against this. bpf_loop, bpf_for_each_map_elem and bpf_find_vma become plain noinline functions through BPF_CALL_x, and bpf_for_each_array_elem, bpf_rbtree_add_impl and the others are ordinary traceable functions. None of them is in btf_id_deny and none is notrace.

If the slot was redirected, bpf_unwind_target() finds no entry and returns the address unchanged. WARN_ON_ONCE() fires, the helper does a normal return, and the tracer then returns to the instruction after the BPF call.

That instruction is unwind_goto, for which the arch backends emit no branch. Execution continues on the fall-through path, which the verifier modelled as a normal return of the helper.

On that path r0 holds what the callback unwound with, and the helper has already done its pad-side work:

- bpf_loop: the verifier thinks r0 is the scalar returned by bpf_loop(), but it can be a kernel pointer, which could be stored to a map, or a referenced pointer whose reference is never released.

- bpf_rbtree_add: on the unwind path the pad has already freed the node with __bpf_obj_drop_impl(). On the fall-through path the verifier ran ref_convert_owning_non_owning() and treats the node as a live non-owning reference, so the program can read and write freed memory.

bpf_check_attach_target() already refuses anything but fentry on a subprog that can unwind ("unwind_exit would return to the trampoline that called the target"). Is there an equivalent for the kernel helpers with pads? The verifier has no say over function_graph or kretprobes at all.

This commit only adds the gate, since helper_has_cb_pad() and kfunc_has_cb_pad() return false for now. The path is opened later in the series by:

4e61c40fe5c0 ("bpf: Let the callback of bpf_loop unwind") 7a687425732b ("bpf: Let the callback of bpf_for_each_map_elem unwind") 59e3a600668d ("bpf: Let the callback of bpf_find_vma unwind") bee41eec3c39 ("bpf: Let the callback of bpf_user_ringbuf_drain unwind") ecd4c98df2ee ("bpf: Let the comparator of bpf_rbtree_add unwind")

No commit up to 5646f059f1bb adds a fix: no notrace or NOKPROBE marking, no attach restriction, and no return_to_handler or trampoline handling in bpf_unwind_target().

Would it work to make the pad helpers untraceable (notrace plus NOKPROBE, and deny them in btf_id_deny for fexit/fmod_ret), or to have bpf_unwind_leave() resolve the real return address through the ftrace/rethook shadow stacks?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40612543702

kernel/bpf/fixups.c static struct bpf_prog *inline_bpf_loop(struct bpf_verifier_env *env, · line 2861

[Severity: High]
Can this leave a live BPF_JMP32_A instruction targeting a dead caller landing pad that gets removed by bpf_opt_remove_dead_code(), leading to an out-of-bounds read in addrs[] or ctx->offset[] during JIT compilation?

In inline_bpf_loop(), pad is enabled without checking whether the target of the unwind_goto at position + 1 was actually visited during verification:

    caller_pad = position + 2 + env->prog->insnsi[position + 1].off;
    env->insn_aux_data[caller_pad].seen

The caller's landing pad can remain unseen while can_unwind and bpf_is_unwind_goto(&env->prog->insnsi[position + 1]) are both true, for example when bpf_loop() is called with 0 iterations (r1 == 0, which skips push_callback_call() in check_helper_call() while still marking the call site fit_for_inline in update_loop_inline_state()) or when unwind_exit in the callback lies on a branch not taken for this call site.

In bpf_check(), bpf_optimize_bpf_loop() runs before bpf_opt_hard_wire_dead_code_branches() and bpf_opt_remove_dead_code():

bpf_check()
bpf_optimize_bpf_loop()
inline_bpf_loop()
bpf_patch_insn_data()
adjust_insn_aux_data()
bpf_opt_hard_wire_dead_code_branches()
bpf_opt_remove_dead_code()

    verifier_remove_insns()
      bpf_remove_insns()
        bpf_adj_branches()
          bpf_adj_delta_to_off()

When bpf_patch_insn_data() patches the inlined loop, adjust_insn_aux_data() marks all patched instructions in insn_buf as seen, including the inlined pad and its trailing BPF_JMP32_A instruction, while the original unwind_goto is overwritten with BPF_JMP_A(0).

When bpf_opt_hard_wire_dead_code_branches() runs next, it preserves the inlined BPF_UNWIND_GOTO because its target (the inlined pad) is marked seen, and ignores the BPF_JMP32_A instruction because it is an unconditional jump.

When bpf_opt_remove_dead_code() then removes the unseen caller landing pad range [pos, end_old), bpf_adj_delta_to_off() only adjusts forward jumps when curr + off + 1 >= end_old. Because BPF_JMP32_A targets caller_pad inside the removed range [pos, end_old), its imm offset is not adjusted. If caller_pad is at the end of a subprogram and preceded by other dead instructions (or followed by a subprogram where bpf_do_misc_fixups() calls adjust_jmp_off() for may_goto), BPF_JMP32_A jumps past the end of the subprogram, causing do_jit() on x86-64 to read past addrs[] and bpf2a64_offset() on arm64 to read past ctx->offset[].

Should pad also require env->insn_aux_data[position + 2 + env->prog->insnsi[position + 1].off].seen to be true?

kernel/bpf/fixups.c static struct bpf_prog *inline_bpf_loop(struct bpf_verifier_env *env, · line 2940

[Severity: High]
What happens if bpf_is_unwind_goto(&new_prog->insnsi[position + cnt]) is true, but pad is false (for example, when env->subprog_info[callback_subprogno].can_unwind is false, or when the caller's landing pad was not reached from this call site)?

In check_unwind_insns() and check_cb_unwind(), an unwind_goto instruction is permitted after a bpf_loop() call even when the callback subprogram has can_unwind == false:

kernel/bpf/verifier.c:check_cb_unwind() {
    if (!env->subprog_info[subprog].can_unwind)
        return 0;
    ...
}

When inline_bpf_loop() runs with pad == false, it replaces the bpf_loop() call with 19 instructions ending in BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_10, r8_offset), and the error branch BPF_JMP_IMM(BPF_JA, 0, 0, 16) jumps directly to new_prog->insnsi[position + cnt].

Because the rewrite of *unwind_goto = BPF_JMP_A(0) is guarded by if (pad), an unwind_goto following the bpf_loop() call is left in place when !pad. If the landing pad targeted by that unwind_goto was marked seen via another unwinding call in the same subprogram, bpf_opt_hard_wire_dead_code_branches() sees:

    if (bpf_is_unwind_goto(insn) && aux_data[i + 1 + insn->off].seen)

and preserves the orphaned unwind_goto instruction. That leaves BPF_UNWIND_GOTO immediately following BPF_LDX_MEM instead of a call instruction, causing the x86-64 and arm64 JITs (which assume insn[i - 1] is a call instruction) to emit bogus EX_TYPE_BPF exception table entries and unwind stubs keyed inside the BPF_LDX_MEM instruction.

Should new_prog->insnsi[position + cnt] be rewritten to BPF_JMP_A(0) whenever bpf_is_unwind_goto(&new_prog->insnsi[position + cnt]) is true, regardless of whether pad is true?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

kernel/bpf/verifier.c static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog) · line 11414

[Severity: Critical]
When helper_has_cb_pad(BPF_FUNC_loop) returns true, can inline_bpf_loop() in kernel/bpf/fixups.c emit a live BPF_JMP32_A instruction that jumps to a dead landing pad which is subsequently removed by dead code elimination, or leave a stray BPF_UNWIND_GOTO after a load instruction?

In kernel/bpf/verifier.c:check_unwind_insns(), subprog_info[subprog].can_unwind is set statically whenever the callback contains an unwind_exit instruction, and check_cb_unwind() then requires an unwind_goto instruction at position + 1 immediately after the bpf_loop() call.

If a privileged program calls bpf_loop() with flags == 0 and a single callback target (so fit_for_inline is true), and the callback does not dynamically take its unwind_exit path during verification of that call (for example, when guarded by callback_ctx or loop index, or when nr_loops == 0), state->unwound is never set at position + 1. In check_cond_jmp_op(), the unwind_goto branch to the caller's landing pad (caller_pad = position + 2 + unwind_goto->off) is never taken, leaving env->insn_aux_data[caller_pad].seen == 0.

In kernel/bpf/verifier.c:bpf_check(), bpf_optimize_bpf_loop() runs before dead code elimination:

	if (ret == 0)
		ret = bpf_optimize_bpf_loop(env);

	if (is_priv) {
		if (ret == 0)
			bpf_opt_hard_wire_dead_code_branches(env);
		if (ret == 0)
			ret = bpf_opt_remove_dead_code(env);

Inside kernel/bpf/fixups.c:inline_bpf_loop(), pad is computed without checking whether the target of unwind_goto at position + 1 was actually visited:

	bool pad = env->subprog_info[callback_subprogno].can_unwind &&
		   bpf_is_unwind_goto(&env->prog->insnsi[position + 1]);
	...
	if (pad) {
		struct bpf_insn *unwind_goto = &new_prog->insnsi[position + cnt];

		/* the jump of the pad, one insn before, takes the target of the unwind_goto */
		new_prog->insnsi[position + cnt - 1].imm = unwind_goto->off + 1;
		/* and the unwind_goto after the call of bpf_loop follows no call anymore */
		*unwind_goto = BPF_JMP_A(0);
	}

Because adjust_insn_aux_data() copies the call instruction's non-zero seen count to all patched instructions, the inlined pad and its trailing BPF_JMP32_A are marked seen != 0. bpf_opt_hard_wire_dead_code_branches() does not fold the inlined BPF_UNWIND_GOTO (its target is the inlined pad, which has seen != 0) and ignores the unconditional BPF_JMP32_A.

When bpf_opt_remove_dead_code() then deletes the unvisited caller_pad via verifier_remove_insns() -> bpf_remove_insns() -> bpf_adj_branches() -> bpf_adj_delta_to_off() in kernel/bpf/core.c:

	if (curr < pos && curr + off + 1 >= end_old)
		off += delta;
	else if (curr >= end_new && curr + off + 1 < end_new)
		off -= delta;

BPF_JMP32_A's offset is not adjusted because its target falls inside the removed range [pos, end_old). If caller_pad was at the end of the subprogram, func[i]->len shrinks while BPF_JMP32_A still jumps past the end of the subprogram, causing out-of-bounds reads of addrs[i + insn->imm] in do_jit() in arch/x86/net/bpf_jit_comp.c and ctx->offset[bpf_insn + off] in bpf2a64_offset() in arch/arm64/net/bpf_jit_comp.c, or jumping into post-return int3 padding or subsequent instructions.

Additionally, if the callback has can_unwind == false and an unwind_goto follows the bpf_loop() call (which check_unwind_insns() and check_cb_unwind() allow), pad is false in inline_bpf_loop(), so *unwind_goto = BPF_JMP_A(0) is skipped, leaving BPF_UNWIND_GOTO immediately after BPF_LDX_MEM instead of a call instruction when the landing pad is shared (seen != 0) or !is_priv.

[Severity: Medium]
What happens in prepare_func_exit() in kernel/bpf/verifier.c when a bpf_loop() callback subprogram with a 9 to 16 byte BTF return type (ret_reg_pair == true) unwinds via unwind_exit?

In kernel/bpf/verifier.c:bpf_compute_subprog_ret_regs(), any static subprogram whose BTF return type has size in [9, 16] bytes gets ret_reg_pair = true, and neither check_func_args() nor btf_check_subprog_call() rejects such a subprogram when passed as ARG_PTR_TO_FUNC.

With helper_has_cb_pad(BPF_FUNC_loop) now returning true, the callee->in_callback_fn && unwinds path in prepare_func_exit() becomes reachable:

	callee = state->frame[state->curframe];
	r0 = &callee->regs[BPF_REG_0];
	nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1;
	for (i = 0; i < nregs; i++) {
		if (callee->regs[ret_regs[i]].type != PTR_TO_STACK)
			continue;
		verbose(env, "cannot return stack pointer to the caller\n");
		return -EINVAL;
	}
	...
		for (i = 0; i < nregs; i++) {
			u32 regno = ret_regs[i];

			bpf_diag_mod_begin(env, &caller->regs[regno], &callee->regs[regno],
					   BPF_DIAG_MOD_WRITE);
			caller->regs[regno] = callee->regs[regno];
			bpf_diag_mod_end(env);
		}

Even though bpf_call_cb() in bpf_loop() only propagates R0 to the landing pad, prepare_func_exit() uses nregs = 2 (ret_regs = { BPF_REG_0, BPF_REG_2 }) without checking !callee->in_callback_fn.

First, because set_loop_callback_state() initializes the callback's R2 to caller->regs[BPF_REG_3] (callback_ctx), passing a stack pointer as callback_ctx causes the PTR_TO_STACK check on callee->regs[BPF_REG_2] to falsely reject the unwind if the callback did not overwrite R2.

Second, prepare_func_exit() copies callee->regs[BPF_REG_2] into caller->regs[BPF_REG_2] before setting *insn_idx = callee->callsite to revisit the bpf_loop() call instruction. When check_helper_call() re-runs with env->cur_state->unwound == true, check_func_args() re-validates caller->regs[BPF_REG_1..5] before caller-saved registers are scrubbed. Because caller->regs[BPF_REG_2] (the callback_fn argument) was overwritten with the callback's R2, could check_func_args() fail verification or update meta.subprogno in update_loop_inline_state() to a different subprogram?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

kernel/bpf/bpf_iter.c BPF_CALL_4(bpf_loop, u32, nr_loops, void *, callback_fn, void *, callback_ctx, · line 784

Can bpf_unwind_leave() find the BPF caller's return address here when bpf_loop() is being traced?

bpf_unwind_leave() looks for the return address of the BPF caller in bpf_loop()'s own return-address slot (__builtin_dwarf_cfa() - 1 on x86, the frame record LR on arm64). But bpf_loop() is a regular traceable kernel function. bpf_iter.o is built with the ftrace flags (kernel/bpf/ Makefile only strips them for percpu_freelist, bpf_lru_list, queue_stack_maps, lpm_trie, ringbuf and rqspinlock), bpf_loop() has no NOKPROBE_SYMBOL, and it is not in btf_id_deny. Several tracers change that slot or add a frame:

- An fexit/fsession BPF trampoline calls the original function with emit_rsb_call(&prog, orig_call, ...) in arch/x86/net/bpf_jit_comp.c, so the return address of bpf_loop() points into the trampoline image.

- kretprobe/rethook: arch_rethook_prepare() does
stack[0] = arch_rethook_trampoline.

- The function_graph tracer and fprobe exit handlers replace the slot with return_to_handler.

In each case bpf_unwind_target(*slot) does not find a BPF address. search_bpf_extables() goes through bpf_prog_ksym_find(), which returns NULL for a trampoline ksym (ksym->prog is false), and the kernel and module tables have no EX_TYPE_BPF entry there, so the address comes back unchanged. Then WARN_ON_ONCE(to == *slot) fires, which panics with panic_on_warn, and bpf_loop() returns ret through the tracer to the instruction after "call bpf_loop". On x86 and arm64 the JIT emits no code for unwind_goto, so the program continues along the normal path.

The verifier checked that path only against callback states that returned normally, where r0 is RET_INTEGER and the stack and references are as left by a normal return. It checked the unwind_exit state only against the pad. So the BPF program can run in a state the verifier never checked. For example:

- r0 can be the pointer or reference the callback unwound with (b2f03cf6322f lets a pointer and its reference reach the pad), now seen as a scalar. The pointer leaks and the reference is never released.

- Callback writes through callback_ctx on the unwind path only (for example a scalar over a spilled pointer) are seen by code that assumes the old slot type.

The trigger is a bpf_loop() call that the verifier does not inline (flags not provably 0, or two callbacks at one call site) whose callback has unwind_exit, while kretprobe:bpf_loop, an fexit program on bpf_loop(), or the function_graph tracer is active.

bpf_check_attach_target() already treats this hazard as real for BPF targets:

    /* unwind_exit would return to the trampoline that called the target */
    ...
    "Only fentry can attach to %s, which can unwind"

Nothing equivalent covers bpf_loop(), which this patch turns into a frame that unwinds. No later patch in the series addresses this (no notrace, NOKPROBE_SYMBOL or btf_id_deny change, and no ftrace_graph_ret_addr() or rethook lookup in bpf_unwind_leave()). The helpers that get pads later in the series (bpf_for_each_map_elem and its ops, bpf_find_vma, bpf_user_ringbuf_drain, bpf_rbtree_add) look to have the same problem.

Could bpf_loop() and the other helpers with pads be made untraceable (notrace plus NOKPROBE_SYMBOL, and a btf_id_deny entry for fexit/fmod_ret), or could bpf_unwind_leave() resolve a hijacked return address?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40612543702

kernel/bpf/arraymap.c static long bpf_for_each_array_elem(struct bpf_map *map, bpf_callback_t callback · line 827

Can the PTR_TO_MAP_KEY handed to the callback here outlive the frame it points into?

&key is a local in the frame of bpf_for_each_array_elem(), and map_set_for_each_callback_args() types it as PTR_TO_MAP_KEY in R2 of the callback. With BPF_FUNC_for_each_map_elem now in helper_has_cb_pad(), the callback may leave with unwind_exit, and prepare_func_exit() from b2f03cf6322f hands the callback's r0 to the pad unchanged, except for PTR_TO_STACK, which it refuses.

A PTR_TO_MAP_KEY in r0 therefore reaches the pad after the call, where check_mem_access() still allows reads of key_size bytes through it. By then the helper frame has been popped (and, on the indirect path, the frame of bpf_for_each_map_elem() too), so the pointer refers to dead kernel stack that any later call or interrupt can overwrite. A program without allow_ptr_leaks/allow_uninit_stack would be able to read stale kernel stack through it.

The changelog of b2f03cf6322f mentions this case: "the key of the array iterator live in the frame of the helper ... the fix for that has to see to r0 too". Nothing between b2f03cf6322f and the end of this series adds that fix. Hash and rhash keys point into map elements rather than the helper frame, so only the array iterator is affected.

Should a PTR_TO_MAP_KEY r0 (or any pointer whose lifetime ends with the helper frame) be refused or turned into a scalar at the callback's unwind_exit before for_each_map_elem is opened up for array maps? Alternatively, could the key live somewhere that outlives the helper?

kernel/bpf/arraymap.c static long bpf_for_each_array_elem(struct bpf_map *map, bpf_callback_t callback · line 838

Does bpf_unwind_leave() work if the helper itself is being traced?

It reads the return-address slot of bpf_for_each_array_elem() and looks it up with bpf_unwind_target(), which only works if the slot still holds the address the BPF caller pushed. The same leave path is added to bpf_for_each_hash_elem(), bpf_each_rhash_elem() and bpf_for_each_map_elem(). All four are ordinary traceable functions: arraymap.o, hashtab.o and bpf_iter.o are not in the CFLAGS_REMOVE list of $(CC_FLAGS_FTRACE) in kernel/bpf/Makefile, none of the four is notrace, and none is in btf_id_deny.

If the function-graph tracer, a kretprobe/rethook, or a BPF fexit/fmod_ret trampoline is attached to one of them, the slot would hold return_to_handler, the rethook trampoline, or a trampoline address, and bpf_unwind_target() would find no entry. Then WARN_ON_ONCE(to == *slot) fires (fatal with panic_on_warn), and the exception comes back as a plain return. The BPF program carries on past the unwind_goto on the normal path, in a state the verifier only explored on the unwind edge.

bpf_check_attach_target() already guards the BPF-target version of this ("unwind_exit would return to the trampoline that called the target", "Only fentry can attach to %s, which can unwind"), but kernel-function targets that now carry pads get no equivalent check. bpf_loop() from 4e61c40fe5c0 has the same exposure.

Should these helpers be marked notrace and added to btf_id_deny, or should bpf_unwind_leave() recover the original return address (as ftrace_graph_ret_addr() and rethook do for unwinders) before the lookup?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40612543702

kernel/bpf/task_iter.c BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start, · line 842

[Severity: High]
Can the unwound pad end up calling mmap_read_unlock() and mmput_async() with hardware interrupts disabled?

When bpf_find_vma() is entered with interrupts enabled for a foreign task, the irqs_disabled() check passes, mmput_needed is set to true, and bpf_mmap_unlock_guard_get() returns NULL because !irqs_disabled():

kernel/bpf/mmap_unlock_work.h:bpf_mmap_unlock_guard_get() {
	struct mmap_unlock_irq_work *work;

	if (!irqs_disabled())
		return NULL;
    ...
}

Inside the callback, callback_ctx (R3) can point to a stack slot in the caller's frame. If the callback calls bpf_local_irq_save() on that caller stack slot and then executes unwind_exit, process_bpf_exit_full() calls check_resource_leak() with check_lock set to false because curframe > 0:

kernel/bpf/verifier.c:process_bpf_exit_full() {
	int err = check_resource_leak(env, exception_exit,
				      exception_exit || !env->cur_state->curframe,
				      exception_exit ? "bpf_throw" :
				      "BPF_EXIT instruction in main prog");
    ...
}

Unlike a normal callback return, unwind_exit sets state->unwound = true in prepare_func_exit(), so push_callback_call() returns 0 on revisit without verifying a second callback round and jumps via unwind_goto to the caller's pad where bpf_local_irq_restore() restores the caller's stack slot.

At runtime, execution enters the unwound label in bpf_find_vma() with hardware interrupts disabled while work is NULL and mmput_needed is true. bpf_mmap_unlock_mm(NULL, mm) then calls mmap_read_unlock(mm) directly with IRQs disabled instead of deferring via irq_work, and mmput_async(mm) is also called with IRQs disabled, risking a deadlock on rq_lock during reader wakeup and triggering an invalid wait context under CONFIG_PREEMPT_RT.

kernel/bpf/task_iter.c BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start, · line 845

[Severity: High]
What prevents the callback from escaping the vma pointer (or pointers walked from it, such as vma->vm_file, &vma->vm_file->f_path, or vma->vm_file->f_inode) in r0 via unwind_exit after mmap_lock, rcu_read_lock(), and the foreign mm reference have been dropped?

In set_find_vma_callback_state(), R2 (struct vm_area_struct *vma) is marked as a plain PTR_TO_BTF_ID:

kernel/bpf/verifier.c:set_find_vma_callback_state() {
    ...
	callee->regs[BPF_REG_2].type = PTR_TO_BTF_ID;
	__mark_reg_known_zero(&callee->regs[BPF_REG_2]);
	callee->regs[BPF_REG_2].btf =  btf_vmlinux;
	callee->regs[BPF_REG_2].btf_id = btf_tracing_ids[BTF_TRACING_TYPE_VMA];
    ...
}

When the callback leaves via unwind_exit, prepare_func_exit() skips the SCALAR_VALUE check on R0 and copies callee->regs[BPF_REG_0] into caller->regs[BPF_REG_0]:

kernel/bpf/verifier.c:prepare_func_exit() {
    ...
	if (callee->in_callback_fn && !unwinds) {
		if (r0->type != SCALAR_VALUE) {
			verbose(env, "R0 not a scalar value\n");
			return -EACCES;
		}
    ...
	} else {
		for (i = 0; i < nregs; i++) {
			u32 regno = ret_regs[i];

			bpf_diag_mod_begin(env, &caller->regs[regno], &callee->regs[regno],
					   BPF_DIAG_MOD_WRITE);
			caller->regs[regno] = callee->regs[regno];
			bpf_diag_mod_end(env);
		}
	}
    ...
}

Once the unwound pad in bpf_find_vma() runs rcu_read_unlock(), bpf_mmap_unlock_mm(work, mm), and mmput_async(mm), the caller's unwind pad receives the plain PTR_TO_BTF_ID in R0 outside the lock and RCU scope. If the VMA and its backing file or inode are concurrently unmapped and freed, passing that dangling R0 pointer to helpers accepting ARG_PTR_TO_BTF_ID (such as bpf_ima_file_hash(), bpf_ima_inode_hash(), bpf_d_path(), or bpf_inode_storage_get()) can trigger a use-after-free.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7

kernel/bpf/verifier.c static bool helper_has_cb_pad(enum bpf_func_id func_id) · line 11413

[Severity: Critical]
Can a callback for bpf_user_ringbuf_drain() smuggle its R1 dynptr (or a slice derived from it via bpf_dynptr_data() or bpf_dynptr_slice()) out to the caller's unwind pad in R0 when exiting via unwind_exit?

In bpf_user_ringbuf_drain(), struct bpf_dynptr_kern dynptr lives on the C stack frame of bpf_user_ringbuf_drain(). When the callback exits via unwind_exit and jumps to unwound_at, bpf_user_ringbuf_drain() releases the sample with __bpf_user_ringbuf_sample_release(), clears rb->busy, and returns ret (the callback's R0) to the caller's unwind pad after its stack frame is popped.

In the verifier, set_user_ringbuf_callback_state() initializes the callback's R1 using mark_dynptr_cb_reg(), which sets R1 to CONST_PTR_TO_DYNPTR with a fresh id that is not backed by a STACK_DYNPTR slot in the callee's BPF stack:

kernel/bpf/verifier.c:set_user_ringbuf_callback_state() {
	...
	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_0]);
	mark_dynptr_cb_reg(env, &callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL);
	callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3];
	...
}

When the callback executes unwind_exit, prepare_func_exit() only rejects PTR_TO_STACK in R0, copies callee->regs[BPF_REG_0] into caller->regs[BPF_REG_0], and calls destroy_dynptrs_in_stack_slots(), which only invalidates STACK_DYNPTR slots in the callee's stack frame:

kernel/bpf/verifier.c:prepare_func_exit() {
	...
			caller->regs[regno] = callee->regs[regno];
			bpf_diag_mod_end(env);
		}
	}

	/* Invalidate callee-local dynptrs and their slices before the frame goes away. */
	err = destroy_dynptrs_in_stack_slots(env, callee, 0,
					     callee->allocated_stack / BPF_REG_SIZE - 1);
	...
}

On the revisit of BPF_FUNC_user_ringbuf_drain in check_helper_call(), unwound_r0 is preserved across caller-saved register scrubbing without invalidating the callback's dynptr id:

kernel/bpf/verifier.c:check_helper_call() {
	...
	if (env->cur_state->unwound) {
		regs[BPF_REG_0] = unwound_r0;
		goto unwound;
	}
	...
}

Wouldn't this leave the caller's unwind pad with a live CONST_PTR_TO_DYNPTR pointing to the popped C stack frame of bpf_user_ringbuf_drain(), or a live PTR_TO_MEM slice pointing to the already-released user ringbuf sample?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8

kernel/bpf/helpers.c static int __bpf_rbtree_add(struct bpf_rb_root *root, · line 2813

[Severity: High]
Does the unwound path in __bpf_rbtree_add() leave node->owner poisoned with BPF_PTR_POISON when the object survives __bpf_obj_drop_impl()?

At the start of __bpf_rbtree_add() in kernel/bpf/helpers.c, node->owner is transitioned from NULL to BPF_PTR_POISON before the traversal loop:

	if (cmpxchg(&node->owner, NULL, BPF_PTR_POISON)) {
		/* Only called from BPF prog, no need to migrate_disable */
		__bpf_obj_drop_impl((void *)n - off, rec, false);
		return -EINVAL;
	}

When the comparator callback cb exits via unwind_exit during the while (*link) loop, control jumps to unwound after cmpxchg() has already set node->owner to BPF_PTR_POISON, and before WRITE_ONCE(node->owner, root) is reached.

If the object contains a bpf_refcount field and another active reference to it exists (for example via bpf_refcount_acquire() before calling bpf_rbtree_add() or inside the comparator before unwinding), __bpf_obj_drop_impl() decrements the refcount and returns without freeing the object:

	if (rec && rec->refcount_off >= 0 &&
	    !refcount_dec_and_test((refcount_t *)(p + rec->refcount_off))) {
		return;
	}

Unlike bpf_rb_root_free(), which clears node->owner back to NULL before dropping the reference:

	/* Ensure __bpf_rbtree_add() sees the node as unlinked. */
	smp_store_release(&node->owner, NULL);
	__bpf_obj_drop_impl(obj, field->graph_root.value_rec, false);

the unwound path in __bpf_rbtree_add() leaves node->owner set to BPF_PTR_POISON on the surviving unlinked object, causing any subsequent bpf_rbtree_add() or bpf_rbtree_add_impl() call on that object to fail the cmpxchg() check with -EINVAL. Should node->owner be reset to NULL before calling __bpf_obj_drop_impl() in the unwound path?

kernel/bpf/verifier.c static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, · line 15756

[Severity: Critical]
Can an unwinding comparator callback with a two-register return type clobber caller->regs[BPF_REG_2] in prepare_func_exit(), causing check_kfunc_call() to release the wrong reference ID and leave a dangling owning pointer to the dropped node?

During verifier setup, bpf_compute_subprog_ret_regs() sets ret_reg_pair = true for any subprogram whose BTF return type is a 9-16 byte struct, and btf_validate_return_type() allows static subprograms to return structs up to 16 bytes. Neither check_func_arg(), push_callback_call(), nor check_cb_unwind() prevents the less callback passed to bpf_rbtree_add() or bpf_rbtree_add_impl() from having ret_reg_pair == true.

When such a callback executes unwind_exit, prepare_func_exit() in kernel/bpf/verifier.c computes nregs = 2 (where ret_regs[] = { BPF_REG_0, BPF_REG_2 }) and copies both registers from the callee frame into the caller frame before rewinding *insn_idx to callee->callsite:

	callee = state->frame[state->curframe];
	r0 = &callee->regs[BPF_REG_0];
	nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1;
	...
	if (callee->in_callback_fn && !unwinds) {
		...
	} else {
		for (i = 0; i < nregs; i++) {
			u32 regno = ret_regs[i];

			bpf_diag_mod_begin(env, &caller->regs[regno], &callee->regs[regno],
					   BPF_DIAG_MOD_WRITE);
			caller->regs[regno] = callee->regs[regno];
			bpf_diag_mod_end(env);
		}
	}

If the caller passed &n1->node (with id = n1_id) in BPF_REG_2 while keeping a copy of n1 in a callee-saved register (R6-R9) or stack spill, and the comparator callback acquires an owning reference to a second node n2 of the same BTF type (for example via bpf_refcount_acquire() or bpf_list_pop_front()), places &n2->node (with id = n2_id) into BPF_REG_2, and executes unwind_exit:

1. prepare_func_exit() overwrites caller->regs[BPF_REG_2] with &n2->node (id = n2_id).

2. On the revisit of check_kfunc_call() at bpf_rbtree_add() with env->cur_state->unwound == true, check_func_args() succeeds because caller->regs[BPF_REG_2] holds a valid owning pointer of the expected node type, and release_reference(env, id) releases n2_id instead of n1_id.

3. At runtime, __bpf_rbtree_add() drops the caller's original node n1 via __bpf_obj_drop_impl(), while in the verifier state at the landing pad after unwind_goto, n1_id was never released and the caller's saved pointer to n1 remains a valid owning PTR_TO_BTF_ID | MEM_ALLOC pointer to freed memory.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

kernel/bpf/helpers.c static int __bpf_rbtree_add(struct bpf_rb_root *root, · line 2810

Does this path leave node->owner set to BPF_PTR_POISON? The function claims the node before the loop:

	if (cmpxchg(&node->owner, NULL, BPF_PTR_POISON)) {

The normal exit replaces that with WRITE_ONCE(node->owner, root), but the unwound exit never clears the claim. For a non-refcounted node the drop frees it, so that does not matter.

For a refcounted node with another live reference, __bpf_obj_drop_impl() only drops the count and the object lives on with owner still set to BPF_PTR_POISON. Nothing resets it later, since bpf_obj_free_fields() does nothing for BPF_RB_NODE and only bpf_rbtree_remove() or a successful add writes owner.

The other reference can come from m = bpf_refcount_acquire(n) before the add, or from the comparator calling bpf_refcount_acquire() on its argument (the kfunc is KF_SPINLOCK_SAFE). The verifier keeps that other reference as owning after the unwind, but every later bpf_rbtree_add() of the object fails at the cmpxchg, drops the caller's reference and returns -EINVAL.

The comment says "drop it, as the error path above does", but the error path above never took the claim and this path did. Would a WRITE_ONCE(node->owner, NULL) before the __bpf_obj_drop_impl() call be needed here?

Can this drop also run in a context where the verifier refuses bpf_obj_drop()? check_kfunc_call() rejects bpf_obj_drop() and bpf_percpu_obj_drop() when btf_record_has_nmi_unsafe_fields() is true for tracing program types, per commit 94c8d1c21be4 ("bpf: Reject bpf_obj_drop() from tracing progs"). Every rbtree node type is covered, because btf_field_is_nmi_safe() returns false for BPF_RB_NODE.

Before this patch, the only drop in bpf_rbtree_add() was the error path for a node already in a collection, and that collection holds a reference of its own. Now a comparator that ends in unwind_exit makes the kfunc call __bpf_obj_drop_impl() every time, and for a non-refcounted node that is the last reference, so bpf_obj_free_fields() runs inline.

Nothing restricts this by program type: BPF_PROG_TYPE_TRACING gets generic_kfunc_set, check_map_prog_compatibility() refuses BPF_SPIN_LOCK only for is_tracing_prog_type() (which does not include BPF_PROG_TYPE_TRACING), and neither check_cb_unwind() nor the new unwound branch in check_kfunc_call() tests btf_record_has_nmi_unsafe_fields() or the program type.

For example, a non-sleepable tp_btf or fentry program attached somewhere NMI-reachable could allocate:

	struct { struct bpf_rb_node node; struct task_struct __kptr *t; }

store an acquired task in t, take the lock and call bpf_rbtree_add() with a comparator that unwinds. The verifier accepts this although it refuses bpf_obj_drop() on the same object, and the pad then runs the task kptr destructor (put_task_struct()) from that context.

Should the same btf_record_has_nmi_unsafe_fields() and program type check be applied to a bpf_rbtree_add() whose comparator can unwind, for example in check_cb_unwind() or in the unwound branch of check_kfunc_call()?

kernel/bpf/helpers.c static int __bpf_rbtree_add(struct bpf_rb_root *root, · line 2813

Can this pad rely on the slot at the frame address of bpf_rbtree_add() or bpf_rbtree_add_impl() still holding the BPF caller's return address? Both are ordinary traceable functions: helpers.o is built with ftrace flags and btf_id_deny does not list them.

Several tracers that hook the return rewrite that slot:

- an fexit, fmod_ret or fsession trampoline, which uses BPF_TRAMP_F_CALL_ORIG and calls orig_call + X86_PATCH_SIZE, so the body returns into the trampoline
- a kretprobe or fprobe exit (the rethook trampoline)
- the function_graph tracer, where ftrace_graph_func() sets *parent = return_to_handler

In each case bpf_unwind_leave() reads that address. bpf_unwind_target() finds no BPF extable entry for it and no EX_TYPE_BPF kernel entry, and returns the address unchanged. WARN_ON_ONCE(to == *slot) fires (a panic with panic_on_warn), and the kfunc returns r0 the normal way.

The tracer then sends control to the BPF program's normal return address, and the JIT emits nothing for unwind_goto, so the program carries on along the path the verifier checked as "comparator returned". On that path ref_convert_owning_non_owning() has made the node a non-owning reference that is valid in the tree until unlock, but the kfunc has already called __bpf_obj_drop_impl() on it:

	__bpf_obj_drop_impl((void *)n - off, rec, false);
	bpf_unwind_leave();
	return r0;

Can the program then read and write a freed node? For example, it could bpf_kptr_xchg() an acquired reference into a kptr field of the dead object (kptr_xchg_dest_types allows NON_OWN_REF), and that reference is never released.

For helpers with pads, a missed unwind only loses the exception. bpf_rbtree_add() is the first one whose two exits leave the object in different states (in the tree, or freed), so for it a missed unwind leaves the verifier's state wrong.

bpf_check_attach_target() already refuses everything but fentry on a subprog that can unwind ("unwind_exit would return to the trampoline that called the target"), but kernel functions with a pad get no such restriction, and the rest of the series (through 5646f059f1bb) adds none. Should return-hooking tracers be kept off these kfuncs (notrace, a btf_id_deny-style list for fexit/fmod_ret/fsession, and no kretprobe or graph tracing), or should the pad tolerate a rewritten return address?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40612543702

tools/testing/selftests/bpf/prog_tests/unwind_drain.c line 55

The test skips only on -ENOTSUPP, which is what check_unwind_insns() returns when !bpf_jit_supports_unwind(). Is that enough on a kernel whose JIT supports unwinding but whose helpers have no callback pads?

On x86_64 or arm64 without CONFIG_CC_HAS_ASM_GOTO_OUTPUT (gcc < 11.5, 12.0-12.3 and 13.0-13.2, see GCC_ASM_GOTO_OUTPUT_BROKEN in scripts/Kconfig.toolchain), BPF_HAS_CB_PADS is 0 in include/linux/bpf_unwind.h while bpf_jit_supports_unwind() still returns true.

Loading drain then goes through:

push_callback_call()
check_cb_unwind()

    verbose(env, "callback cannot unwind\n");
    return -EINVAL;

unwind_drain__open_and_load() returns NULL with errno = EINVAL, so the -ENOTSUPP check is false and ASSERT_OK_PTR(skel, "open_and_load") fails the test where it should skip, since the kernel does not offer the feature in that configuration.

Should this also skip when the load fails with "callback cannot unwind" in the verifier log (for example via bpf_program__set_log_buf()), or should the kernel return -ENOTSUPP for that case so that this check catches it?

[ ... ]

tools/testing/selftests/bpf/progs/verifier_unwind.c static unsigned long cb_unwinds(void) · line 908

The new expected message is only what the verifier prints when BPF_HAS_CB_PADS is 1. In include/linux/bpf_unwind.h that is the case only for defined(CONFIG_X86_64) && defined(CONFIG_CC_HAS_ASM_GOTO_OUTPUT) and the arm64 equivalent; the #else branch sets it to 0 and calls the callback the plain way.

check_cb_unwind() in kernel/bpf/verifier.c tests that first:

    if (!BPF_HAS_CB_PADS || !insn_has_cb_pad(insn)) {
        verbose(env, "callback cannot unwind\n");
        return -EINVAL;
    }

so on such a kernel the log for this program still says "callback cannot unwind" and never reaches "no unwind_goto after the call of bpf_loop()".

CC_HAS_ASM_GOTO_OUTPUT depends on !GCC_ASM_GOTO_OUTPUT_BROKEN (scripts/Kconfig.toolchain), which is y for GCC_VERSION < 11.5, 12.0-12.3 and 13.0-13.2, while the minimum gcc is 8.1 (scripts/min-tool-version.sh). An x86_64 or arm64 kernel built with gcc 11.4 (Ubuntu 22.04), 12.2 (Debian 12) or 13.2 therefore still has bpf_jit_supports_unwind() == true.

check_unwind_insns() does not refuse the program there, so __arch_x86_64, __arch_arm64 and __load_if_JITed() still let it load, and verifier_unwind/callback_unwinds now fails. Before this hunk the expectation was correct on that configuration.

Nothing in the test can express the compiler dependency. The new verifier_unwind_cb.c tests in this commit depend on the compiler in the same way.

Can the expectation be made to hold in either configuration, for example with a regex that accepts either message, or can the test skip when the helpers have no pads?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40612543702