Re: [PATCH bpf-next 4/4] bpf: Fix double-free RO header in bpf_jit_free

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] When `bpf_jit_binary_pack_finalize` fails, `bpf_jit_binary_pack_free` reads an uninitialized/garbage `size` from `ro_header->size`, leading to a kernel panic in `vfree()` and module memory uncharge counter corruption.
--

--- Patch [4]: [PATCH bpf-next 4/4] bpf: Fix double-free RO header in bpf_jit_free ---
commit 4586888c771a3a31208f8ce31608697cc4147140
Author: Pu Lehui <pulehui@huawei.com>

bpf: Fix double-free RO header in bpf_jit_free

This commit modifies bpf_jit_binary_pack_finalize() to drop the implicit free
of ro_header on failure. Architectures are updated to explicitly call
bpf_jit_binary_pack_free() in their error paths instead.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index de7515ea1beae..d017bb0397b88 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -3988,7 +3988,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
>  			 * Both cases are serious bugs and justify WARN_ON.
>  			 */
>  			if (WARN_ON(bpf_jit_binary_pack_finalize(header, rw_header))) {
> -				/* header has been freed */
> +				bpf_jit_binary_pack_free(header, NULL);
>  				header = NULL;
>  				goto out_image;
>  			}

[ ... ]

> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 47fe047ad30b8..77195911a3b37 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1237,10 +1237,9 @@ int bpf_jit_binary_pack_finalize(struct bpf_binary_header *ro_header,
>  
>  	kvfree(rw_header);
>  
> -	if (IS_ERR(ptr)) {
> -		bpf_prog_pack_free(ro_header, ro_header->size);
> +	if (IS_ERR(ptr))
>  		return PTR_ERR(ptr);
> -	}
> +
>  	return 0;
>  }

[Severity: Critical]
This is a pre-existing issue, but does reading ro_header->size cause a panic
when bpf_arch_text_copy() fails?

If bpf_arch_text_copy() returns an error, the RO memory was not written, so
ro_header->size contains uninitialized garbage or illegal instructions. Because
kvfree(rw_header) is called unconditionally just before this check, the only
header holding the correct allocation size is destroyed before the caller can
properly free the resource.

When the architecture fallback explicitly calls bpf_jit_binary_pack_free()
(or previously when this function called bpf_prog_pack_free directly), it
reads the garbage size from ro_header->size:

kernel/bpf/core.c:bpf_jit_binary_pack_free() {
    u32 size = ro_header->size; 
    bpf_prog_pack_free(ro_header, size);
    ...
    bpf_jit_uncharge_modmem(size); 
}

Since this garbage size typically exceeds BPF_PROG_PACK_SIZE, the allocator
falls back to vfree(ro_header). As ro_header is a pointer to the interior of
a vmalloc'd pack, vfree() will trigger a kernel panic. Additionally,
bpf_jit_uncharge_modmem() drastically underflows the module memory tracking
counter.

Could the size be preserved or passed differently to avoid reading from the
uninitialized RO header during the error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722112754.1615201-1-pulehui@huaweicloud.com?part=4