Re: [PATCH v14 10/17] 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:

New issues:
- [Low] Stale function prototype for prepare_elf_headers() left in the arch header.

Pre-existing issues:
- [Critical] Retrying after load_other_segments() failure leads to Use-After-Free and Double Free of the ELF headers buffer.
--

commit 6831fa88a7516f58be581afc5bb7de8776b9c44c
Author: Jinjie Ruan <ruanjinjie@huawei.com>

arm64: kexec_file: Use crash_prepare_headers() helper to simplify code
    
Use the newly introduced crash_prepare_headers() function to replace
the existing prepare_elf_headers(), allocate cmem and exclude crash
kernel memory in the crash core, which reduce code duplication.

> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> index 884e446f08e34..770a4c7bf5737 100644
> --- a/arch/arm64/kernel/kexec_image.c
> +++ b/arch/arm64/kernel/kexec_image.c
> @@ -97,14 +96,11 @@ static void *image_load(struct kimage *image,
>  
>  #ifdef CONFIG_CRASH_DUMP
>  	if (image->type == KEXEC_TYPE_CRASH) {
> -		get_online_mems();
> -		ret = prepare_elf_headers(&headers, &headers_sz);
> +		ret = crash_prepare_headers_locked(true, &headers, &headers_sz, NULL);
>  		if (ret) {
>  			pr_err("Preparing elf core header failed\n");
> -			put_online_mems();
>  			return ERR_PTR(ret);
>  		}
> -		put_online_mems();
>  		image->elf_headers = headers;
>  		image->elf_headers_sz = headers_sz;
>  	}

[Severity: Critical]
This isn't a bug introduced by this patch, but it appears there's a
use-after-free and double-free risk in the retry loop below this code.

If load_other_segments() fails during the retry loop, it frees the
headers buffer:

arch/arm64/kernel/machine_kexec_file.c:load_other_segments() {
    ...
	if (image->type == KEXEC_TYPE_CRASH) {
		...
		ret = kexec_add_buffer(&kbuf);
		if (ret) {
			vfree(headers);
			goto out_err;
		}
    ...
}

If the retry loop in image_load() continues after this failure, won't the
next iteration pass the already freed headers pointer back into
load_other_segments()?

Additionally, since image->elf_headers stores this pointer, does
arch_kimage_file_post_load_cleanup() also call vfree(image->elf_headers)
upon failure, leading to a double-free?

[ ... ]

> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index c0ace89ded923..66fbfbaec1c67 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -40,51 +40,33 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>  }
>  
>  #ifdef CONFIG_CRASH_DUMP
> -int prepare_elf_headers(void **addr, unsigned long *sz)

[Severity: Low]
Does the function prototype for prepare_elf_headers() need to be
removed from arch/arm64/include/asm/kexec.h as well?

It appears to be left behind as dead code since this patch removes its
architecture-specific implementation.

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