Re: [PATCH v12 5/8] powerpc64/bpf: fix percpu private stack leak on JIT failure

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing error handling flaw in `bpf_jit_free()` leads to a double-free and use-after-free of the BPF binary pack if `bpf_jit_binary_pack_finalize()` fails.
- [High] The exact same per-CPU private stack leak fixed in this patch for PowerPC also exists in the ARM64 BPF JIT compiler.
--

--- Patch [5]: [PATCH v12 5/8] powerpc64/bpf: fix percpu private stack leak on JIT failure ---
commit 7f3490460ab3b86fd0696381ed6fb1a49e89ac48
Author: Abhishek Dubey <[email protected]>

powerpc64/bpf: fix percpu private stack leak on JIT failure

The existing conditional statement in bpf_int_jit_compile() frees the
percpu private stack at out_addrs only when the image buffer was never
allocated.

[ ... ]

Closes: https://lore.kernel.org/bpf/[email protected]

> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index 38acef44c0979..b45fa80271068 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -404,7 +404,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
>  				(void *)fimage + FUNCTION_DESCR_SIZE);
>  
>  out_addrs:
> -		if (!image && priv_stack_ptr) {
> +		if (!fp->jited && priv_stack_ptr) {

[Severity: High]
This isn't a bug introduced by this patch, but does the arm64 BPF JIT have the
exact same leak this patch fixes?

Looking at bpf_int_jit_compile() in arch/arm64/net/bpf_jit_comp.c:

	out_off:
		if (!ro_header && priv_stack_ptr) {
			free_percpu(priv_stack_ptr);
			prog->aux->priv_stack_ptr = NULL;
		}

If JIT compilation fails late (e.g., validate_ctx fails), it jumps to the
out_free_hdr block which frees the header but doesn't set ro_header to NULL.
Execution then falls through to out_off. Since ro_header is non-NULL, it skips
freeing the stack. bpf_jit_free() also skips it because prog->jited is 0.

Should the arm64 code also be updated to use !prog->jited?


[Severity: High]
This is a pre-existing issue, but can ignoring the return value of
bpf_jit_binary_pack_finalize() in bpf_jit_free() cause a double-free?

In arch/powerpc/net/bpf_jit_comp.c:bpf_jit_free():

	if (jit_data) {
		bpf_jit_binary_pack_finalize(jit_data->fhdr, jit_data->hdr);
		// ...
	}
	hdr = bpf_jit_binary_pack_hdr(fp);
	bpf_jit_binary_pack_free(hdr, NULL);

If bpf_jit_binary_pack_finalize() fails, it frees ro_header and returns an
error:

kernel/bpf/core.c:bpf_jit_binary_pack_finalize() {
	...
	if (IS_ERR(ptr)) {
		bpf_prog_pack_free(ro_header, ro_header->size);
		return PTR_ERR(ptr);
	}
	...
}

Because bpf_jit_free() ignores this return value, it immediately re-fetches
the freed header and calls bpf_jit_binary_pack_free() again.

Does this lead to a use-after-free read of the garbage size and a double free
of the pack?

>  			fp->aux->priv_stack_ptr = NULL;
>  			free_percpu(priv_stack_ptr);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5