Re: [PATCH v3 bpf-next 05/13] bpf: support instructions arrays with constants blinding

Eduard Zingerman
2025-09-19T07:12:41Z

On Fri, 2025-09-19 at 07:05 +0000, Anton Protopopov wrote:
> On 25/09/18 11:35PM, Eduard Zingerman wrote:
> > On Thu, 2025-09-18 at 09:38 +0000, Anton Protopopov wrote:
> >
> > [...]
> >
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index a7ad4fe756da..5c1e4e37d1f8 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -21578,6 +21578,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> > > struct bpf_insn *insn;
> > > void *old_bpf_func;
> > > int err, num_exentries;
> > > + int old_len, subprog_start_adjustment = 0;
> > >
> > > if (env->subprog_cnt <= 1)
> > > return 0;
> > > @@ -21652,7 +21653,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> > > func[i]->aux->func_idx = i;
> > > /* Below members will be freed only at prog->aux */
> > > func[i]->aux->btf = prog->aux->btf;
> > > - func[i]->aux->subprog_start = subprog_start;
> > > + func[i]->aux->subprog_start = subprog_start + subprog_start_adjustment;
> > > func[i]->aux->func_info = prog->aux->func_info;
> > > func[i]->aux->func_info_cnt = prog->aux->func_info_cnt;
> > > func[i]->aux->poke_tab = prog->aux->poke_tab;
> > > @@ -21705,7 +21706,15 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> > > func[i]->aux->might_sleep = env->subprog_info[i].might_sleep;
> > > if (!i)
> > > func[i]->aux->exception_boundary = env->seen_exception;
> > > +
> > > + /*
> > > + * To properly pass the absolute subprog start to jit
> > > + * all instruction adjustments should be accumulated
> > > + */
> > > + old_len = func[i]->len;
> > > func[i] = bpf_int_jit_compile(func[i]);
> > > + subprog_start_adjustment += func[i]->len - old_len;
> > > +
> > > if (!func[i]->jited) {
> > > err = -ENOTSUPP;
> > > goto out_free;
> >
> > This change makes sense, however, would it be possible to move
> > bpf_jit_blind_constants() out from jit to verifier.c:do_check,
> > somewhere after do_misc_fixups?
> > Looking at the source code, bpf_jit_blind_constants() is the first
> > thing any bpf_int_jit_compile() does.
> > Another alternative is to add adjust_subprog_starts() call to this
> > function. Wdyt?
>
> Yes, it makes total sense. Blinding was added to x86 jit initially and then
> every other jit copy-pasted it. I was considering to move blinding up some
> time back (see https://lore.kernel.org/bpf/20250318143318.656785-1-aspsk@isovalent.com/),
> but then I've decided to avoid this, as this requires to patch every JIT, and I
> am not sure what is the way to test such a change (any hints?)

We have the following covered by CI:
- arch/x86/net/bpf_jit_comp.c
- arch/s390/net/bpf_jit_comp.c
- arch/arm64/net/bpf_jit_comp.c

People work on these jits actively:
- arch/riscv/net/bpf_jit_core.c
- arch/loongarch/net/bpf_jit.c
- arch/powerpc/net/bpf_jit_comp.c

So, we can probably ask to test the patch-set.

The remaining are:
- arch/x86/net/bpf_jit_comp32.c
- arch/parisc/net/bpf_jit_core.c
- arch/mips/net/bpf_jit_comp.c
- arch/arm/net/bpf_jit_32.c
- arch/sparc/net/bpf_jit_comp_64.c
- arch/arc/net/bpf_jit_core.c

The change to each individual jit is not complicated, just removing
the transformation call. Idk, I'd just go for it.
Maybe Alexei has concerns?