Re: [RFC PATCH 11/20] kho: return virtual address of mem_map

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The local variable `radix` in `kho_extend_scratch` is uninitialized, leading to skipped initialization and a subsequent kernel panic when walking the radix tree.
- [Low] kho_get_mem_map returns an integer 0 instead of a NULL pointer on an error path.
--

commit 80928d5942267d8e8edcbc91a1c5643fdd5aecee
Author: Pratyush Yadav (Google) <pratyush@kernel.org>

kho: return virtual address of mem_map

Changes kho_get_mem_map_phys() to kho_get_mem_map() and makes it return a
virtual address instead of a physical address to simplify its callers.

> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index a006a883ee94a..797ec285b698c 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -610,10 +610,11 @@ static int __init kho_preserved_memory_reserve(unsigned long key, void *data)
>  	return 0;
>  }
>  
> -/* Returns physical address of the preserved memory map from FDT */
> -static phys_addr_t __init kho_get_mem_map_phys(const void *fdt)
> +/* Returns virtual address of the preserved memory map from FDT */
> +static __init void *kho_get_mem_map(const void *fdt)
>  {
>  	const void *mem_ptr;
> +	phys_addr_t mem_map_phys;
>  	int len;
>  
>  	mem_ptr = fdt_getprop(fdt, 0, KHO_FDT_MEMORY_MAP_PROP_NAME, &len);
> @@ -622,7 +623,11 @@ static phys_addr_t __init kho_get_mem_map_phys(const void *fdt)
>  		return 0;
>  	}

[Severity: Low]
Since the return type has been changed to a pointer, should this return NULL
instead of the integer 0 to avoid a sparse warning?

> -	return get_unaligned((const u64 *)mem_ptr);
> +	mem_map_phys = get_unaligned((const u64 *)mem_ptr);
> +	if (!mem_map_phys)
> +		return NULL;
> +
> +	return phys_to_virt(mem_map_phys);
>  }
>  
[ ... ]
> @@ -917,15 +922,15 @@ void __init kho_extend_scratch(void)
>  		.key = kho_ext_mark_scratch,
>  	};
>  	struct kho_radix_tree radix;
> -	phys_addr_t prev_end = 0, mem_map_phys;
> +	phys_addr_t prev_end = 0;
>  	int err = 0;

[Severity: Critical]
Because radix is declared on the stack without being zeroed, it is
uninitialized. When kho_radix_init_tree() is called later, it checks if
tree->root is already set:

kernel/liveupdate/kexec_handover.c:kho_radix_init_tree() {
    ...
    if (tree->root)
        return 0;
    ...
}

Since radix.root contains uninitialized stack garbage, it is highly likely
to be non-zero. Could this cause kho_radix_init_tree() to return without
initializing the lock and setting up the root node, leading to a panic when
the tree is walked?

>  
>  	if (!is_kho_boot())
>  		return;
>  
>  	/* Make sure the KHO radix tree is initialized. */
> -	mem_map_phys = kho_get_mem_map_phys(kho_get_fdt());
> -	err = kho_radix_init_tree(&kho_in.radix_tree, phys_to_virt(mem_map_phys));
> +	err = kho_radix_init_tree(&kho_in.radix_tree,
> +				  kho_get_mem_map(kho_get_fdt()));
>  	if (err)
>  		goto print;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528004204.1484584-1-jloeser@linux.microsoft.com?part=11