> diff --git a/kernel/kexec_bpf_loader.c b/kernel/kexec_bpf_loader.c
> index dc59e1389da9..bd6a47fc53ed 100644
> --- a/kernel/kexec_bpf_loader.c
> +++ b/kernel/kexec_bpf_loader.c
[ ... ]
> +static void decompress_mem_allocator_fini(struct decompress_mem_allocator *a)
> +{
> + vfree(a->chunk_base_addr);
> +}
Should decompress_mem_allocator_fini() also free the individual chunks stored
in chunk_base_addr[0..next_idx-1]? On the success path,
merge_decompressed_data()
frees them, but on error paths (decompressor failure or merge failure), the
chunks appear to be leaked since only the array pointer is freed here.
[ ... ]
> +static int parser_cmd_decompress(char *compressed_data, int image_gz_sz,
> + char **out_buf, int *out_sz, struct kexec_context *ctx)
> +{
> + struct decompress_mem_allocator *a = &dcmpr_allocator;
> + decompress_fn decompressor;
> + const char *name;
> + int ret;
> +
> + decompress_mem_allocator_init(a, CHUNK_SIZE);
Is the return value from decompress_mem_allocator_init() intentionally
ignored? If vmalloc fails inside that function, chunk_base_addr and the
other fields remain uninitialized, and subsequent code will use garbage
values when accessing the allocator.
> + decompressor = decompress_method(compressed_data, image_gz_sz, &name);
> + if (!decompressor) {
> + pr_err("Can not find decompress method\n");
> + return -1;
> + }
When decompress_method() returns NULL, should decompress_mem_allocator_fini()
be called before returning? The allocator was initialized above and allocated
both chunk_base_addr and the first chunk, which appear to leak on this path.
> + pr_debug("Find decompressing method: %s, compressed sz:0x%x\n",
> + name, image_gz_sz);
> + ret = decompressor(compressed_data, image_gz_sz, NULL, flush,
> + NULL, NULL, NULL);
> + if (!!ret)
> + goto err;
> + ret = merge_decompressed_data(a, out_buf, out_sz);
> +
> +err:
> + decompress_mem_allocator_fini(a);
> +
> + return ret;
> +}
[ ... ]
> + switch (cmd->cmd) {
> + case KEXEC_BPF_CMD_DECOMPRESS:
> + ret = parser_cmd_decompress(buf, cmd->payload_len,
> &decompressed_buf,
> + &decompressed_sz, ctx);
> + if (!ret) {
> + switch (cmd->subcmd) {
> + case KEXEC_BPF_SUBCMD_KERNEL:
> + vfree(ctx->kernel);
> + ctx->kernel = decompressed_buf;
> + ctx->kernel_sz = decompressed_sz;
> + break;
> + default:
> + break;
When decompression succeeds but subcmd is not KEXEC_BPF_SUBCMD_KERNEL (for
example INITRD or CMDLINE), decompressed_buf is allocated by
merge_decompressed_data() but not stored or freed. Is this intentional, or
should decompressed_buf be freed in the default case?
> + }
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21147860407
