Re: [PATCH bpf-next v6 2/7] riscv, bpf: Fix memory leak in bpf_jit_free

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] bpf_jit_free() ignores the error return value of bpf_jit_binary_pack_finalize(), leading to a Use-After-Free, Double-Free, and potential JIT memory charge limit corruption.
- [High] The identical memory leak fixed in RISC-V remains present in ARM64 and LoongArch BPF JIT implementations.
- [High] Memory charge limit (modmem) is permanently leaked when bpf_jit_binary_pack_finalize() fails in bpf_int_jit_compile().
--

--- Patch [2]: [PATCH bpf-next v6 2/7] riscv, bpf: Fix memory leak in bpf_jit_free ---
commit 9c17e0351f5dcd9f2cb6815fc5c5e2c47b738ba2
Author: Pu Lehui <pulehui@huawei.com>

riscv, bpf: Fix memory leak in bpf_jit_free

This commit fixes a memory leak in the RISC-V BPF JIT compiler. When a
subprogram fails to compile, it cleans up earlier subprograms but previously
failed to free the jit_data->ctx.offset array.

> diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
> --- a/arch/riscv/net/bpf_jit_core.c
> +++ b/arch/riscv/net/bpf_jit_core.c
> @@ -234,6 +234,7 @@ void bpf_jit_free(struct bpf_prog *prog)
>  		 */
>  		if (jit_data) {
>  			bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header);

[Severity: Critical]
This is a pre-existing issue, but does ignoring the return value of
bpf_jit_binary_pack_finalize() here cause a use-after-free and double-free?

If bpf_jit_binary_pack_finalize() fails (for example, due to a text copy
failure), it frees the ro_header memory chunk 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);
    }
    ...
}

Since bpf_jit_free() ignores this failure, it extracts the now-freed
ro_header via bpf_jit_binary_pack_hdr() below, and unconditionally passes
it to bpf_jit_binary_pack_free(). Inside that function, the freed ro_header
is dereferenced to read its size, leading to a use-after-free, double-free,
and uncharging the modmem with garbage data.

Can this be triggered by an unprivileged BPF program load?

[Severity: High]
This is also a pre-existing issue, but does bpf_jit_binary_pack_finalize()
fail to uncharge the modmem limit when it returns an error?

As shown above, bpf_jit_binary_pack_finalize() frees ro_header on failure
but does not call bpf_jit_uncharge_modmem(). In other callers like
bpf_int_jit_compile(), the error handler detects the failure, clears the
header pointers, and skips calling bpf_jit_binary_pack_free() completely:

arch/riscv/net/bpf_jit_core.c:bpf_int_jit_compile() {
    ...
    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;
        jit_data->header = NULL;
        goto out_free_hdr;
    }
    ...
}

Since out_free_hdr skips bpf_jit_binary_pack_free() when header is NULL,
the modmem uncharge is permanently bypassed. Does this leak the
bpf_jit_limit capacity?

> +			kfree(jit_data->ctx.offset);

[Severity: High]
This isn't a bug introduced by this patch, but do arm64 and loongarch have
the exact same memory leak that is being fixed here for riscv?

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

arch/arm64/net/bpf_jit_comp.c:bpf_jit_free() {
    ...
    if (jit_data) {
        bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header);
        kfree(jit_data);
    }
    ...
}

And similarly in arch/loongarch/net/bpf_jit.c:

arch/loongarch/net/bpf_jit.c:bpf_jit_free() {
    ...
    if (jit_data) {
        bpf_jit_binary_pack_finalize(jit_data->ro_header, jit_data->header);
        kfree(jit_data);
    }
    ...
}

Neither architecture frees the jit_data->ctx.offset array before freeing
jit_data. Should a similar fix be applied to those architectures?

>  			kfree(jit_data);
>  		}
>  		hdr = bpf_jit_binary_pack_hdr(prog);
>  		bpf_jit_binary_pack_free(hdr, NULL);

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