Re: [PATCH bpf v2 1/3] bpf: Add validation for bpf_set_retval argument

[Xu Kuohai]

On 5/30/2026 3:04 PM, bot+bpf-ci@xxxxxxxxxx wrote:
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 7fb88e1cd7c4..e82dedf871e8 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -10460,6 +10460,19 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> >   		}
> >   		break;
> >   	case BPF_FUNC_set_retval:
> > +	{
> > +		struct bpf_retval_range range = {
> > +			.minval = -MAX_ERRNO,
> > +			.maxval = 0,
> > +			.return_32bit = true
> > +		};
> > +		struct bpf_reg_state *r1 = &regs[BPF_REG_1];
> > +
> > +		if (r1->type != SCALAR_VALUE) {
> > +			verbose(env, "R1 is not a scalar\n");
> > +			return -EINVAL;
> > +		}
> > +
> >   		if (prog_type == BPF_PROG_TYPE_LSM &&
> >   		    env->prog->expected_attach_type == BPF_LSM_CGROUP) {
> >   			if (!env->prog->aux->attach_func_proto->type) {
> > @@ -10469,8 +10482,16 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> >   				verbose(env, "BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value!\n");
> >   				return -EINVAL;
> >   			}
> > +			bpf_lsm_get_retval_range(env->prog, &range);
> >   		}
> > +
> > +		if (!retval_range_within(range, r1)) {
> > +			verbose_invalid_scalar(env, r1, range, "At bpf_set_retval", "R1");
> > +			return -EINVAL;
> > +		}
> > +
>
> Because bpf_set_retval() takes ARG_ANYTHING, does this bounds check need
> a call to mark_chain_precision(env, BPF_REG_1) to prevent a state pruning
> regression?
>
> Since ARG_ANYTHING doesn't automatically mark the register as precise in
> check_func_arg(), it looks like the verifier's state pruning logic in
> regsafe() might treat this register as equivalent to any other scalar
> value if it remains imprecise.
>
> Could this allow a program with two branches to bypass this bounds check?
> If a safe branch (e.g., passing 0) is explored first and saved with an
> imprecise R1, the verifier might prune an unsafe branch (e.g., passing 1)
> because it matches the imprecise scalar state.
>
> This would skip the bounds check on the unsafe branch and potentially
> allow the exact bypass issue this patch is trying to fix.
>
> Reference: https://lore.kernel.org/bpf/20260530063315.7C5D41F00893@xxxxxxxxxxxxxxx/

Confirmed with the following PoC:

SEC("scgroup/sock_create")
__naked int sock_create_prog(void *ctx)
{
	asm volatile(
		"call %[bpf_get_prandom_u32];"
		"if r0 != 0 goto 1f;"
		"r0 = r0;"
		"r0 = r0;"
		"r0 = r0;"
		"r0 = r0;"
		"r0 = r0;"
		"goto 2f;"
	"1:"
		"call %[bpf_get_prandom_u32];"
	"2:"
		"r1 = r0;"
		"call %[bpf_set_retval];"
		"r0 = 1;"
		"exit;"
		: : __imm(bpf_get_prandom_u32), __imm(bpf_set_retval)
	);
}

Will fix it and add the PoC to the test in the next version.

> >   		break;
> > +	}
>
> [ ... ]
>
>
> ---
> 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/26677028117