Re: [PATCH v17 04/10] arm64: kexec_file: Use crash_prepare_headers() helper to simplify code

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition in arch_crash_populate_cmem() causes a heap buffer overflow in the flexible array cmem->ranges.
- [High] Memory leak of the vmalloc'd ELF headers buffer in the fallback retry loop of image_load().
--

commit 8b7f92744f5c8fa93faeb43fb72b1a59dcc427a4
Author: Jinjie Ruan <ruanjinjie@huawei.com>

arm64: kexec_file: Use crash_prepare_headers() helper to simplify code

This patch refactors the kexec_file code to use the newly introduced
crash_prepare_headers() helper, replacing prepare_elf_headers(). It
implements arch_get_system_nr_ranges() and arch_crash_populate_cmem()
to traverse memblock ranges and populate crash memory structures.

> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index e31fabed378a5..b019b31df48cb 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c

[ ... ]

> +int arch_crash_populate_cmem(struct crash_mem *cmem)
> +{
> +	phys_addr_t start, end;
> +	u64 i;
>  
> -	cmem->max_nr_ranges = nr_ranges;
> -	cmem->nr_ranges = 0;
>  	for_each_mem_range(i, &start, &end) {
>  		cmem->ranges[cmem->nr_ranges].start = start;
>  		cmem->ranges[cmem->nr_ranges].end = end - 1;
>  		cmem->nr_ranges++;

[Severity: High]
This is a pre-existing issue, but is it possible for memory hotplug events
to add new memblock regions concurrently?

If a concurrent memory hotplug event adds regions between the first
pass in arch_get_system_nr_ranges() and this second pass in
arch_crash_populate_cmem(), this loop blindly increments cmem->nr_ranges
without bounding it against cmem->max_nr_ranges.

Could this lead to an out-of-bounds write to the cmem->ranges array?

>  	}
>  
> -	/* Exclude crashkernel region */

[ ... ]

> @@ -109,7 +93,7 @@ int load_other_segments(struct kimage *image,
>  	void *headers;
>  	unsigned long headers_sz;
>  	if (image->type == KEXEC_TYPE_CRASH) {
> -		ret = prepare_elf_headers(&headers, &headers_sz);
> +		ret = crash_prepare_headers(true, &headers, &headers_sz, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but does this leak the vmalloc'd
headers buffer if a subsequent step in load_other_segments() fails?

If a later operation (like placing the initrd or dtb via kexec_add_buffer)
fails, the function jumps to out_err:

out_err:
	image->nr_segments = orig_segments;
	kvfree(dtb);
	return ret;

This doesn't free headers or clear image->elf_headers. The caller,
image_load(), catches this failure and loops to retry load_other_segments()
in a new memory hole. 

On the retry, a new headers buffer is allocated here, overwriting the
previous pointer in image->elf_headers and permanently leaking the old buffer.

>  		if (ret) {
>  			pr_err("Preparing elf core header failed\n");
>  			goto out_err;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629094746.191843-1-ruanjinjie@huawei.com?part=4