Re: [PATCH bpf-next v5 08/16] bpf: Support stack arguments for bpf functions

bot+bpf-ci
2026-04-18T01:04:17Z

[-- Attachment #1: Type: text/plain, Size: 6540 bytes --]

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 29a8a2605a12..6223c055b028 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h

[ ... ]

> @@ -508,6 +513,17 @@ struct bpf_verifier_state {
> iter < frame->allocated_stack / BPF_REG_SIZE; \
> iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
>
> +#define bpf_get_spilled_stack_arg(slot, frame, mask) \
> + (((slot < frame->stack_arg_depth / BPF_REG_SIZE) && \
> + ((1 << frame->stack_arg_regs[slot].type) & (mask))) \
> + ? &frame->stack_arg_regs[slot] : NULL)
^^^^

The shift count in this macro appears to be incorrect. The macro uses
`1 << frame->stack_arg_regs[slot].type` where type is an `enum bpf_reg_type`.

Looking at bpf_get_spilled_reg (which this macro is modeled after), it uses
`1 << frame->stack[slot].slot_type[BPF_REG_SIZE - 1]` where slot_type is an
`enum bpf_stack_slot_type` with values in the range 0..7.

However, `enum bpf_reg_type` values frequently carry high bit flags like
PTR_MAYBE_NULL (BIT(8)), PTR_UNTRUSTED (BIT(14)), PTR_TRUSTED (BIT(20)),
and MEM_RCU (BIT(21)):

include/linux/bpf.h:
enum bpf_type_flag {
PTR_MAYBE_NULL = BIT(8),
...
PTR_UNTRUSTED = BIT(14),
...
PTR_TRUSTED = BIT(20),
MEM_RCU = BIT(21),
...
};

When a stack arg has type PTR_TO_BTF_ID | MEM_RCU, the value is around
0x200010, and `1 << 0x200010` is undefined behavior (shift count >= width
of int).

The concrete impact occurs where bpf_for_each_reg_in_vstate (line 541-554)
walks all registers including stack args:

include/linux/bpf_verifier.h:
#define bpf_for_each_reg_in_vstate_mask(...)
bpf_for_each_spilled_reg(___j, __state, __reg, __mask) {
...
}
bpf_for_each_spilled_stack_arg(___j, __state, __reg, __mask) {
if (!__reg)
continue;
(void)(__expr);
}

At kernel/bpf/verifier.c:13438, this iterator is used with
clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER) to strip MEM_RCU
from all surviving pointers after rcu_read_unlock. For a stack arg with
PTR_TO_BTF_ID | MEM_RCU, the mask test becomes
`(1 << (type & 31)) & 130` which does not match 130, so the macro returns
NULL and the MEM_RCU stripping code never runs. MEM_RCU pointers passed
as arguments 6+ remain tagged MEM_RCU across rcu_read_unlock, allowing
later dereferences without PTR_UNTRUSTED semantics.

Could the mask test be rewritten to avoid bit-shifting an enum bpf_reg_type
value? Perhaps using is_spillable_regtype() or a similar type predicate
rather than a bit pattern?

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f25a56cfabac..7a65b532e84a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1340,6 +1340,20 @@ static int copy_stack_state(struct bpf_func_state *dst, const struct bpf_func_st
> return -ENOMEM;
>
> dst->allocated_stack = src->allocated_stack;
> +
> + /* copy stack args state */
> + n = src->stack_arg_depth / BPF_REG_SIZE;
> + if (n) {
> + dst->stack_arg_regs = copy_array(dst->stack_arg_regs, src->stack_arg_regs, n,
> + sizeof(struct bpf_reg_state),
> + GFP_KERNEL_ACCOUNT);
> + if (!dst->stack_arg_regs)
> + return -ENOMEM;
> +
> + dst->stack_arg_depth = src->stack_arg_depth;
> + dst->incoming_stack_arg_depth = src->incoming_stack_arg_depth;
> + dst->out_stack_arg_mask = src->out_stack_arg_mask;
> + }
> return 0;
> }

Are dst->stack_arg_depth, dst->incoming_stack_arg_depth, and
dst->out_stack_arg_mask updated when src->stack_arg_depth is 0?

copy_stack_state only updates those fields when n != 0. When src has no
stack args, dst retains stale values from a prior copy.

This differs from the unconditional update of dst->allocated_stack a few
lines above, and from copy_reference_state which always updates
active_locks and acquired_refs.

The function copy_func_state() calls memcpy up to offsetof(struct
bpf_func_state, stack). Since stack_arg_depth, incoming_stack_arg_depth,
out_stack_arg_mask, and stack_arg_regs are all declared AFTER stack,
memcpy does not copy them. copy_stack_state is the only place those
fields are updated from src to dst.

The staleness is visible through pop_stack:

pop_stack() calls bpf_copy_verifier_state(cur, &head->st).
cur->frame[i] may have out_stack_arg_mask/stack_arg_depth populated
from a previously explored branch that called a stack-arg function.
head->st is the saved state from before the branch, with mask=0 and
stack_arg_depth=0.
For that frame, src->stack_arg_depth==0 so the copy is skipped, and
cur keeps the prior branch's mask and stack_arg_regs.

The commit message promises "A per-state bitmask out_stack_arg_mask
tracks which outgoing stack arg slots have been written on the current
path... if one branch of a conditional writes arg6 but another does not,
the mask correctly reflects the difference and the verifier rejects the
uninitialized path."

After pop_stack, the other branch is processed on top of cur, where the
mask still carries bits set by the taken branch.

Consequences in set_callee_state:

kernel/bpf/verifier.c:set_callee_state() {
...
if ((caller->out_stack_arg_mask & req_mask) != req_mask) {
for (i = 0; i < callee_incoming_slots; i++) {
if (caller->out_stack_arg_mask & BIT(i))
continue;
verbose(env, "stack arg#%d not properly initialized\n", ...);
return -EINVAL;
}
}
...
}

With stale bits, this check can pass on a path where the current branch
never wrote the required r11 offsets. The verifier then copies stale
caller->stack_arg_regs[caller_spi] into the callee's incoming slot.

Similarly in btf_check_func_arg_match:

kernel/bpf/verifier.c:btf_check_func_arg_match() {
...
if (spi >= (caller->stack_arg_depth / BPF_REG_SIZE)) {
verbose(env, "stack %s not found in caller state\n", ...);
return -EINVAL;
}
...
}

With stale caller->stack_arg_depth, this bound check can pass on paths
where the actual live stack_arg_depth is 0.

Should these fields be updated unconditionally, even when n==0?

---
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/24592562564