Re: [PATCH v2 3/4] kho: rename KHO scratch to KHO bootmem

[sashiko-bot@kernel.org]

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

New issues:
- [High] Mismatch in device tree property names for KHO bootmem causes KHO initialization failure on OF platforms.
- [Medium] UAPI breakage: renamed fields in `struct kho_data`.

Pre-existing issues:
- [High] Kernel panic or Use-After-Free during early boot if KHO bootmem allocation fails.
--

commit 3dec172ac499ac601fb7ed89fbed34f0bd01b393
Author: Pratyush Yadav (Google) <pratyush@kernel.org>

kho: rename KHO scratch to KHO bootmem

This commit renames the term "KHO scratch" to "KHO boot memory" (or bootmem)
across function names, variable names, comments, and documentation. The change
clarifies that the memory is provided by the previous kernel for early boot
allocations, while the discovered allocatable areas are now called "noprsrv".

> diff --git a/arch/x86/include/uapi/asm/setup_data.h b/arch/x86/include/uapi/asm/setup_data.h
> --- a/arch/x86/include/uapi/asm/setup_data.h
> +++ b/arch/x86/include/uapi/asm/setup_data.h
[ ... ]
> @@ -85,8 +85,8 @@ struct ima_setup_data {
>  struct kho_data {
>  	__u64 fdt_addr;
>  	__u64 fdt_size;
> -	__u64 scratch_addr;
> -	__u64 scratch_size;
> +	__u64 bootmem_addr;
> +	__u64 bootmem_size;
>  } __attribute__((packed));

[Severity: Medium]
Will renaming these fields in a UAPI header break source compatibility for
userspace tools (like kexec-tools) that are compiled against this structure?
If userspace references scratch_addr and scratch_size, compiling against the
new headers will fail.

> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
[ ... ]
> @@ -285,8 +285,8 @@ static int kho_add_chosen(const struct kimage *image, void *fdt, int chosen_node
>  				       fdt_mem, fdt_len);
>  	if (ret)
>  		return ret;
> -	ret = fdt_appendprop_addrrange(fdt, 0, chosen_node, "linux,kho-scratch",
> -				       scratch_mem, scratch_len);
> +	ret = fdt_appendprop_addrrange(fdt, 0, chosen_node, "linux,kho-bootmem",
> +				       bootmem_arr, bootmem_len);

[Severity: High]
Does this create a mismatch with the FDT reader? Looking at the boot-time
initialization in early_init_dt_check_kho() in drivers/of/fdt.c, the new
kernel still appears to search for the old linux,kho-scratch property.

If the reader side is not updated to match, will this cause the new kernel
to silently discard the handed-over memory during kexec on OF platforms?

> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
[ ... ]
> @@ -837,73 +837,73 @@ static void __init kho_reserve_bootmem(void)
>  	if (!kho_enable)
>  		return;
>  
> -	scratch_size_update();
> +	bootmem_size_update();
>  
>  	/* FIXME: deal with node hot-plug/remove */
> -	kho_scratch_cnt = nodes_weight(node_states[N_MEMORY]) + 2;
> -	size = kho_scratch_cnt * sizeof(*kho_scratch);
> -	kho_scratch = memblock_alloc(size, PAGE_SIZE);
> -	if (!kho_scratch) {
> -		pr_err("Failed to reserve scratch array\n");
> +	kho_bootmem_cnt = nodes_weight(node_states[N_MEMORY]) + 2;
> +	size = kho_bootmem_cnt * sizeof(*kho_bootmem);
> +	kho_bootmem = memblock_alloc(size, PAGE_SIZE);
> +	if (!kho_bootmem) {
> +		pr_err("Failed to reserve bootmem array\n");
>  		goto err_disable_kho;
>  	}
[ ... ]
> -err_free_scratch_areas:
> +err_free_bootmem_areas:
>  	for (i--; i >= 0; i--)
> -		memblock_phys_free(kho_scratch[i].addr, kho_scratch[i].size);
> -err_free_scratch_desc:
> -	memblock_free(kho_scratch, kho_scratch_cnt * sizeof(*kho_scratch));
> +		memblock_phys_free(kho_bootmem[i].addr, kho_bootmem[i].size);
> +err_free_bootmem_desc:
> +	memblock_free(kho_bootmem, kho_bootmem_cnt * sizeof(*kho_bootmem));
>  err_disable_kho:
> -	pr_warn("Failed to reserve scratch area, disabling kexec handover\n");
> +	pr_warn("Failed to reserve bootmem, disabling kexec handover\n");
>  	kho_enable = false;
>  }

[Severity: High]
This is a pre-existing issue, but if the initial memblock_alloc() fails in
kho_reserve_bootmem(), the error path sets kho_enable = false but fails to
clear kho_bootmem_cnt back to zero.

If kho_bootmem_cnt is greater than zero and kho_bootmem is a NULL or freed
pointer, could this cause a panic later in boot? Code like
kho_bootmem_migratetype() unconditionally calls kho_bootmem_overlap(), which
iterates up to kho_bootmem_cnt and blindly dereferences kho_bootmem:

bool kho_bootmem_overlap(phys_addr_t phys, size_t size)
{
        ...
        for (i = 0; i < kho_bootmem_cnt; i++) {
                bootmem_start = kho_bootmem[i].addr;
        ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908155639.2496551-1-pratyush@kernel.org?part=3