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

[Pu Lehui <pulehui@huawei.com>]

On 2026/3/13 1:02, Xu Kuohai wrote:
> From: Xu Kuohai <xukuohai@huawei.com>
> 
> During the JIT stage, constants blinding rewrites instructions but only
> rewrites the private instruction copy of the JITed subprog, leaving the
> global instructions and insn_aux_data unchanged. This causes a mismatch
> between subprog instructions and the global state, making it difficult
> to look up the global insn_aux_data in the JIT.
> 
> To avoid this mismatch, and given that all arch-specific JITs already
> support constants blinding, move it to the generic verifier code, and
> switch to rewrite the global env->insnsi with the global states
> adjusted, as other rewrites in the verifier do.
> 
> This removes the constants blinding calls in each JIT, which are largely
> duplicated code across architectures.
> 
> Since constants blinding is only required for JIT, and there are two
> entry functions for JIT, jit_subprogs() and bpf_prog_select_runtime(),
> move the constants blinding invocation into the two functions.
> 
> If constants blinding fails, or if it succeeds but the subsequent JIT
> compilation fails, kernel falls back to running the BPF program with
> interpreter. To ensure a correct rollback, the program cloning before
> instruction rewriting in the constants blinding is preserved. During
> the blinding process, only the cloned instructions are patched, leaving
> the original program untouched.
> 
> Since bpf_patch_insn_data() is chosen for the constants blinding in the
> verifier path, and it adjusts the global auxiliary data in the verifier
> state, a key question is whether this auxiliary data should be restored
> when JIT fails?
> 
> Besides instructions, bpf_patch_insn_data() adjusts env->insn_aux_data,
> env->subprog_info, prog->aux->poke_tab and env->insn_array_maps. env->
> insn_aux_data and env->subprog_info are no longer used after JIT failure
> and are freed at the end of bpf_check(). prog->aux->poke_tab is only
> used by JIT. And when the JIT fails, programs using insn_array would be
> rejected by bpf_insn_array_ready() function since no JITed addresses
> available. This means env->insn_array_maps is only useful for JIT.
> Therefore, all the auxiliary data adjusted does not need to be restored.
> 
> For classic BPF programs, constants blinding works as before since it
> is still invoked from bpf_prog_select_runtime().
> 
> Reviewed-by: Anton Protopopov <a.s.protopopov@gmail.com>
> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
> ---
>   arch/arc/net/bpf_jit_core.c      | 39 ++++++-----------
>   arch/arm/net/bpf_jit_32.c        | 41 +++---------------
>   arch/arm64/net/bpf_jit_comp.c    | 72 +++++++++----------------------
>   arch/loongarch/net/bpf_jit.c     | 59 ++++++++------------------
>   arch/mips/net/bpf_jit_comp.c     | 20 +--------
>   arch/parisc/net/bpf_jit_core.c   | 73 +++++++++++++-------------------
>   arch/powerpc/net/bpf_jit_comp.c  | 68 +++++++++++------------------
>   arch/riscv/net/bpf_jit_core.c    | 61 ++++++++++----------------
>   arch/s390/net/bpf_jit_comp.c     | 59 +++++++++-----------------
>   arch/sparc/net/bpf_jit_comp_64.c | 61 +++++++++-----------------
>   arch/x86/net/bpf_jit_comp.c      | 43 +++----------------
>   arch/x86/net/bpf_jit_comp32.c    | 33 ++-------------
>   include/linux/filter.h           | 11 ++++-
>   kernel/bpf/core.c                | 66 +++++++++++++++++++++++++----
>   kernel/bpf/verifier.c            | 40 +++++++++++------
>   15 files changed, 281 insertions(+), 465 deletions(-)

[...]

>   /*
> diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
> index b3581e926436..527baa50dc68 100644
> --- a/arch/riscv/net/bpf_jit_core.c
> +++ b/arch/riscv/net/bpf_jit_core.c
> @@ -44,29 +44,19 @@ bool bpf_jit_needs_zext(void)
>   struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   {
>   	unsigned int prog_size = 0, extable_size = 0;
> -	bool tmp_blinded = false, extra_pass = false;
> -	struct bpf_prog *tmp, *orig_prog = prog;
> +	bool extra_pass = false;
>   	int pass = 0, prev_ninsns = 0, i;
>   	struct rv_jit_data *jit_data;
>   	struct rv_jit_context *ctx;
>   
>   	if (!prog->jit_requested)
> -		return orig_prog;
> -
> -	tmp = bpf_jit_blind_constants(prog);
> -	if (IS_ERR(tmp))
> -		return orig_prog;
> -	if (tmp != prog) {
> -		tmp_blinded = true;
> -		prog = tmp;
> -	}
> +		return prog;
>   
>   	jit_data = prog->aux->jit_data;
>   	if (!jit_data) {
>   		jit_data = kzalloc_obj(*jit_data);
>   		if (!jit_data) {
> -			prog = orig_prog;
> -			goto out;
> +			return prog;
>   		}
>   		prog->aux->jit_data = jit_data;
>   	}
> @@ -83,15 +73,11 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   	ctx->user_vm_start = bpf_arena_get_user_vm_start(prog->aux->arena);
>   	ctx->prog = prog;
>   	ctx->offset = kzalloc_objs(int, prog->len);
> -	if (!ctx->offset) {
> -		prog = orig_prog;
> +	if (!ctx->offset)
>   		goto out_offset;
> -	}
>   
> -	if (build_body(ctx, extra_pass, NULL)) {
> -		prog = orig_prog;
> +	if (build_body(ctx, extra_pass, NULL))
>   		goto out_offset;
> -	}
>   
>   	for (i = 0; i < prog->len; i++) {
>   		prev_ninsns += 32;
> @@ -105,10 +91,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   		bpf_jit_build_prologue(ctx, bpf_is_subprog(prog));
>   		ctx->prologue_len = ctx->ninsns;
>   
> -		if (build_body(ctx, extra_pass, ctx->offset)) {
> -			prog = orig_prog;
> +		if (build_body(ctx, extra_pass, ctx->offset))
>   			goto out_offset;
> -		}
>   
>   		ctx->epilogue_offset = ctx->ninsns;
>   		bpf_jit_build_epilogue(ctx);
> @@ -126,10 +110,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   							  &jit_data->ro_image, sizeof(u32),
>   							  &jit_data->header, &jit_data->image,
>   							  bpf_fill_ill_insns);
> -			if (!jit_data->ro_header) {
> -				prog = orig_prog;
> +			if (!jit_data->ro_header)
>   				goto out_offset;
> -			}
>   
>   			/*
>   			 * Use the image(RW) for writing the JITed instructions. But also save
> @@ -150,7 +132,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   
>   	if (i == NR_JIT_ITERATIONS) {
>   		pr_err("bpf-jit: image did not converge in <%d passes!\n", i);
> -		prog = orig_prog;
>   		goto out_free_hdr;
>   	}
>   
> @@ -163,26 +144,27 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   	ctx->nexentries = 0;
>   
>   	bpf_jit_build_prologue(ctx, bpf_is_subprog(prog));
> -	if (build_body(ctx, extra_pass, NULL)) {
> -		prog = orig_prog;
> +	if (build_body(ctx, extra_pass, NULL))
>   		goto out_free_hdr;
> -	}
>   	bpf_jit_build_epilogue(ctx);
>   
>   	if (bpf_jit_enable > 1)
>   		bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);
>   
> -	prog->bpf_func = (void *)ctx->ro_insns + cfi_get_offset();
> -	prog->jited = 1;
> -	prog->jited_len = prog_size - cfi_get_offset();
> -
>   	if (!prog->is_func || extra_pass) {
>   		if (WARN_ON(bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header))) {
>   			/* ro_header has been freed */
>   			jit_data->ro_header = NULL;
> -			prog = orig_prog;
> -			goto out_offset;
> +			jit_data->header = NULL;
> +			goto out_free_hdr;

Thank you for fixing this issue, and the riscv port looks good to me.

Reviewed-by: Pu Lehui <pulehui@huawei.com> # riscv

>   		}
> +	}
> +
> +	prog->bpf_func = (void *)ctx->ro_insns + cfi_get_offset();
> +	prog->jited = 1;
> +	prog->jited_len = prog_size - cfi_get_offset();
> +
> +	if (!prog->is_func || extra_pass) {
>   		/*
>   		 * The instructions have now been copied to the ROX region from
>   		 * where they will execute.
> @@ -198,14 +180,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>   		kfree(jit_data);
>   		prog->aux->jit_data = NULL;
>   	}
> -out:
>   
> -	if (tmp_blinded)
> -		bpf_jit_prog_release_other(prog, prog == orig_prog ?
> -					   tmp : orig_prog);
>   	return prog;
>   
>   out_free_hdr:
> +	if (extra_pass) {
> +		prog->bpf_func = NULL;
> +		prog->jited = 0;
> +		prog->jited_len = 0;
> +	}
>   	if (jit_data->header) {
>   		bpf_arch_text_copy(&jit_data->ro_header->size, &jit_data->header->size,
>   				   sizeof(jit_data->header->size));