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

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-03-13 at 01:02 +0800, Xu Kuohai wrote:

[...]

> diff --git a/arch/arc/net/bpf_jit_core.c b/arch/arc/net/bpf_jit_core.c
> index 1421eeced0f5..973ceae48675 100644
> --- a/arch/arc/net/bpf_jit_core.c
> +++ b/arch/arc/net/bpf_jit_core.c

[...]

> @@ -229,12 +211,19 @@ static void jit_ctx_cleanup(struct jit_context *ctx)
>  		ctx->bpf2insn_valid = false;
>  
>  	/* Freeing "bpf_header" is enough. "jit.buf" is a sub-array of it. */
> -	if (!ctx->success && ctx->bpf_header) {
> -		bpf_jit_binary_free(ctx->bpf_header);
> -		ctx->bpf_header = NULL;
> -		ctx->jit.buf    = NULL;
> -		ctx->jit.index  = 0;
> -		ctx->jit.len    = 0;
> +	if (!ctx->success) {
> +		if (ctx->bpf_header) {
> +			bpf_jit_binary_free(ctx->bpf_header);
> +			ctx->bpf_header = NULL;
> +			ctx->jit.buf    = NULL;
> +			ctx->jit.index  = 0;
> +			ctx->jit.len    = 0;
> +		}
> +		if (ctx->is_extra_pass) {

Nit: The idea is that for !ctx->is_extra_pass
     ctx->prog->bpf_func != NULL only when ctx->success is true, right?
     Maybe just drop the condition?

> +			ctx->prog->bpf_func = NULL;
> +			ctx->prog->jited = 0;
> +			ctx->prog->jited_len = 0;
> +		}
>  	}
>  
>  	ctx->emit = false;
> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
> index deeb8f292454..e6b1bb2de627 100644
> --- a/arch/arm/net/bpf_jit_32.c
> +++ b/arch/arm/net/bpf_jit_32.c
> @@ -2144,9 +2144,7 @@ bool bpf_jit_needs_zext(void)
>  
>  struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>  {
> -	struct bpf_prog *tmp, *orig_prog = prog;
>  	struct bpf_binary_header *header;
> -	bool tmp_blinded = false;
>  	struct jit_ctx ctx;
>  	unsigned int tmp_idx;
>  	unsigned int image_size;

The code in arch/arc is modified to do `... ctx->prog->jited = 0; ...`,
but for arm32 there is no such modification. Why is that so?

[...]

> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index adf84962d579..cd5a72fff500 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -2009,14 +2009,12 @@ struct arm64_jit_data {

[...]

> @@ -2245,13 +2211,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;
> +	}

Just for my understanding, is the following correct?
- Previously, a call bpf_jit_blind_constants() always cloned the prog.
- Jits only set prog->jited to true upon successful compilation.
- On error exit jits restored the original prog with it's prog->jited == 0.

What happened in case of an extra pass?
I'd expect that in case of an extra pass prog->jited would be true
even before program is cloned by blind_constants() (and that's what
arc code uses to figure out if the current pass is an extra pass).
If so, old code would preserve prog->jited as true even in case of
extra pass failure. Is that true or am I confused?

Just trying to understand why this patch has to deal with the above
snippet at all. In case it is indeed necessary, it seems the logic
should be similar for all jits, is there a way to push this snippet to
some common code? E.g. in verifier.c where the extra pass is initiated.

[...]