Re: [PATCH bpf-next v5 06/16] bpf: Limit the scope of BPF_REG_PARAMS usage

sashiko-bot
2026-04-17T04:50:15Z

Sashiko AI review found 2 potential issue(s):
- [Critical] Early returns in dataflow tracking passes skip necessary state updates for the destination register during BPF_LDX using BPF_REG_PARAMS.
- [Critical] Permitting BPF_REG_PARAMS causes severe out-of-bounds memory accesses to the `regs` array during the main verifier loop (`do_check_main`).
--

commit d68722297de575b838cc2f76141cf9c052e63448
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Limit the scope of BPF_REG_PARAMS usage

This commit restricts the usage of BPF_REG_PARAMS (r11) to specific memory
load and store instruction forms. It also adds early returns in dataflow
tracking passes to prevent out-of-bounds accesses for stack arguments.

> diff --git a/kernel/bpf/const_fold.c b/kernel/bpf/const_fold.c
> index db73c4740b1e1..09db7fdb370fd 100644
> --- a/kernel/bpf/const_fold.c
> +++ b/kernel/bpf/const_fold.c

[ ... ]

> @@ -51,13 +51,18 @@ static void const_reg_xfer(struct bpf_verifier_env *env, struct const_arg_info *
> struct bpf_insn *insn, struct bpf_insn *insns, int idx)
> {
> struct const_arg_info unknown = { .state = CONST_ARG_UNKNOWN, .val = 0 };
> - struct const_arg_info *dst = &ci_out[insn->dst_reg];
> - struct const_arg_info *src = &ci_out[insn->src_reg];
> + struct const_arg_info *dst, *src;
> u8 class = BPF_CLASS(insn->code);
> u8 mode = BPF_MODE(insn->code);
> u8 opcode = BPF_OP(insn->code) | BPF_SRC(insn->code);
> int r;
>
> + /* Stack arguments using BPF_REG_PARAMS are outside the tracked register set. */
> + if (insn->dst_reg >= MAX_BPF_REG || insn->src_reg >= MAX_BPF_REG)
> + return;

If we return early here during a BPF_LDX instruction where src_reg is
BPF_REG_PARAMS (11), doesn't this skip necessary state updates for dst_reg
(e.g., registers 0 through 9)?

Since dst_reg is not marked as unknown, won't it retain its stale tracking
state? Could bpf_prune_dead_branches() rely on this stale constant to
incorrectly rewrite conditional jumps, allowing verifier safety checks to be
bypassed?

> +
> + dst = &ci_out[insn->dst_reg];
> + src = &ci_out[insn->src_reg];

[ ... ]

> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index 332e6e003f270..0c7eef3385513 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
> @@ -1068,11 +1068,16 @@ static void arg_track_xfer(struct bpf_verifier_env *env, struct bpf_insn *insn,
> int depth = instance->depth;
> u8 class = BPF_CLASS(insn->code);
> u8 code = BPF_OP(insn->code);
> - struct arg_track *dst = &at_out[insn->dst_reg];
> - struct arg_track *src = &at_out[insn->src_reg];
> + struct arg_track *dst, *src;
> struct arg_track none = { .frame = ARG_NONE };
> int r;
>
> + /* Stack arguments using BPF_REG_PARAMS are outside the tracked register set. */
> + if (insn->dst_reg >= MAX_BPF_REG || insn->src_reg >= MAX_BPF_REG)
> + return;

Similarly, does this early return skip clearing the arg_track state for the
destination register when loading from a stack argument?

> +
> + dst = &at_out[insn->dst_reg];
> + src = &at_out[insn->src_reg];

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ff0c55d803117..f25a56cfabaca 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18487,13 +18487,22 @@ static int check_and_resolve_insns(struct bpf_verifier_env *env)
> return err;
>
> for (i = 0; i < insn_cnt; i++, insn++) {
> + u8 class = BPF_CLASS(insn->code);
> + u8 mode = BPF_MODE(insn->code);
> +
> if (insn->dst_reg >= MAX_BPF_REG) {
> - verbose(env, "R%d is invalid\n", insn->dst_reg);
> - return -EINVAL;
> + if (insn->dst_reg != BPF_REG_PARAMS ||
> + !((class == BPF_ST || class == BPF_STX) && mode == BPF_MEM)) {
> + verbose(env, "R%d is invalid\n", insn->dst_reg);
> + return -EINVAL;
> + }
> }

By permitting BPF_REG_PARAMS (which equals MAX_BPF_REG) to pass this validation
check, won't this cause out-of-bounds memory accesses to the regs array
(e.g., struct bpf_reg_state regs[MAX_BPF_REG]) during the main verifier loop?

When do_check_main() processes these instructions, functions like
check_reg_arg() will attempt to access regs[11], which overlays verifier
internal state.

I note this regression is fixed later in the patch series in commit
6ec4a3fcb5e83 ("bpf: Support stack arguments for bpf functions"), but it
appears to introduce an issue in this intermediate patch.

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260417034658.2625353-1-yonghong.song@linux.dev?part=6