[PATCH bpf-next v11 1/5] bpf: Move constants blinding out of arch-specific JITs

[bot+bpf-ci at kernel.org]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a431b7d50e1b..0d6bfccf0dd7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> +static int jit_subprogs(struct bpf_verifier_env *env)
> +{
> +	int err, i;
> +	bool blinded = false;
> +	struct bpf_insn *insn;
> +	struct bpf_prog *prog, *orig_prog;
> +	u32 *orig_subprog_starts;
> +
> +	if (env->subprog_cnt <= 1)
> +		return 0;
> +
> +	prog = orig_prog = env->prog;
> +	if (bpf_prog_need_blind(orig_prog)) {
> +		if (!IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON)) {
> +			orig_subprog_starts = dup_subprog_starts(env);
> +			if (!orig_subprog_starts) {
> +				err = -ENOMEM;
> +				goto out_cleanup;
> +			}
> +		}
> +		prog = bpf_jit_blind_constants(env, NULL);
> +		if (IS_ERR(prog)) {
> +			err = -ENOMEM;
> +			goto out_restore;

When bpf_jit_blind_constants() fails, prog holds an ERR_PTR value.
The goto out_restore falls through to out_cleanup which dereferences
prog:

    prog->jit_requested = 0;

That would be dereferencing ERR_PTR(-ENOMEM).

> +		}
> +		blinded = true;
> +	}
> +
> +	err = __jit_subprogs(env);
> +	if (blinded) {
> +		if (err) {
> +			bpf_jit_prog_release_other(orig_prog, prog);

Similarly, when blinding succeeds but __jit_subprogs() fails,
bpf_jit_prog_release_other() frees the clone through
bpf_prog_clone_free(). After this, prog points to freed memory,
and the goto out_restore falls through to out_cleanup where
prog->jit_requested is a use-after-free.

Both paths could use orig_prog (or env->prog, which was restored)
at out_cleanup instead of the stale prog local variable.

> +			/* roll back to the clean original prog */
> +			env->prog = orig_prog;
> +			goto out_restore;
> +		} else {
> +			bpf_jit_prog_release_other(prog, orig_prog);
> +			if (!IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON))
> +				kvfree(orig_subprog_starts);
> +		}
> +	} else if (err) {
> +		/* We will fall back to interpreter mode when err is not -EFAULT, before
> +		 * that, insn->off and insn->imm should be restored to their original values
> +		 * since they were modified by __jit_subprogs.
> +		 */
> +		if (err != EFAULT) {

The comment says "when err is not -EFAULT" but the code checks
err != EFAULT (positive 14).  Since __jit_subprogs() returns
negative error codes, this condition is always true.  Should this
be err != -EFAULT?

> +			for (i = 0, insn = prog->insnsi; i < prog->len; i++, insn++) {
> +				if (!bpf_pseudo_call(insn))
> +					continue;
> +				insn->off = 0;
> +				insn->imm = env->insn_aux_data[i].call_imm;
> +			}
> +		}
> +		goto out_cleanup;
> +	}
> +
> +	return 0;
> +
> +out_restore:
> +	if (!IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON)) {
> +		restore_subprog_starts(env, orig_subprog_starts);
> +		kvfree(orig_subprog_starts);
> +	}
> +out_cleanup:
> +	/* cleanup main prog to be interpreted */
> +	prog->jit_requested = 0;
> +	prog->blinding_requested = 0;
>  	return err;
>  }

[ ... ]

> @@ -26456,7 +26529,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
>  err_unlock:
>  	if (!is_priv)
>  		mutex_unlock(&bpf_verifier_lock);
> -	clear_insn_aux_data(env, 0, env->prog->len);
> +	clear_insn_aux_data(env, 0, env->blinded_insn_aux_len ? : env->prog->len);

When blinding succeeds but JIT fails and jit_subprogs() rolls back
env->prog to the original (shorter) program, blinded_insn_aux_len
is larger than env->prog->len.

clear_insn_aux_data() iterates to blinded_insn_aux_len and calls
bpf_is_ldimm64(&insns[i]) on each entry.  The insns array comes
from env->prog->insnsi, which only has env->prog->len entries.
Can this read past the end of the program instructions array?

>  	vfree(env->insn_aux_data);

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