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

Daniel Borkmann
2025-09-19T19:28:23Z

On 9/19/25 8:26 PM, Alexei Starovoitov wrote:
> On Fri, Sep 19, 2025 at 12:12 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>> 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?
>
> No concerns.
> I don't remember why JIT calls it instead of the verifier.
>
> Daniel,
> do you recall? Any concern?

Hm, I think we did this in the JIT back then for couple of reasons iirc,
the constant blinding needs to work from native bpf(2) as well as from
cbpf->ebpf (seccomp-bpf, filters, etc), so the JIT was a natural location
to capture them all, and to fallback to interpreter with the non-blinded
BPF-insns when something went wrong during blinding or JIT process (e.g.
JIT hits some internal limits etc). Moving bpf_jit_blind_constants() out
from JIT to verifier.c:do_check() means constant blinding of cbpf->ebpf
are not covered anymore (and in this case its reachable from unpriv).

Cheers,
Daniel