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

[Xu Kuohai]

On 4/13/2026 9:47 PM, bot+bpf-ci@xxxxxxxxxx wrote:
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index 066b86e7233c..fc9fb3c07866 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
>
> [ ... ]
>
> >   /**
> >    *	bpf_prog_select_runtime - select exec runtime for BPF program
> >    *	@fp: bpf_prog populated with BPF program
>
> [ ... ]
>
> > -	/* eBPF JITs can rewrite the program in case constant
> > -	 * blinding is active. However, in case of error during
> > -	 * blinding, bpf_int_jit_compile() must always return a
> > -	 * valid program, which in this case would simply not
> > -	 * be JITed, but falls back to the interpreter.
> > -	 */
> > +	/* eBPF JITs can rewrite the program in case constant
> > +	 * blinding is active. However, in case of error during
> > +	 * blinding, bpf_int_jit_compile() must always return a
> > +	 * valid program, which in this case would simply not
> > +	 * be JITed, but falls back to the interpreter.
> > +	 */
>
> This isn't a bug, but this comment still references bpf_int_jit_compile()
> even though the code now calls bpf_prog_jit_compile(). Should this be
> updated to match?

This comment is not touched by the patch, it was already there before
the patch, so I'd leave it as is.

> > -		fp = bpf_int_jit_compile(fp);
> > +		fp = bpf_prog_jit_compile(fp);
>
> > diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> > index 67c9b28767e1..ec8afd6a9369 100644
> > --- a/kernel/bpf/fixups.c
> > +++ b/kernel/bpf/fixups.c
>
> [ ... ]
>
> > +out_jit_err:
> > +	if (blinded) {
> > +		bpf_jit_prog_release_other(orig_prog, prog);
> > +		/* roll back to the clean original prog */
> > +		prog = env->prog = orig_prog;
> > +		goto out_restore;
> > +	} else {
> > +		if (err != -EFAULT) {
> > +			/*
> > +			 * 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.
> > +			 */
>
> The comment references __jit_subprogs, but the function is named
> jit_subprogs (no double underscore prefix). Should this be updated?

Yes, it should be jit_subprogs.

In addition, sashiko reported an issue that subprog_start of the fake
exit subprog is not restored on the rollback path. It said that
bpf_fixup_call_args() relies on this sentinel to detect the end of the
last subprog. I checked the bpf_fixup_call_args() function and its child
functions, but could not find any usage of the sentinel.

IIUC, the sentinel is only used to compute the length of the last subprog
and is not used after jit_subprogs fails, so not restoring it does not
to be a real issue.

However, leaving the sentinel unrestored seems error-prone, and for code
consistency, I'll restore it in the next version.

> > +			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;
> > +			}
> > +		}
>
>
> ---
> 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/24345549165