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) <[email protected]> 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/[email protected]?part=11
