Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] kho_radix_alloc_node() incorrectly uses slab_is_available() to determine if the buddy allocator can be used, leading to double-allocation and memory corruption during boot. - [High] kho_radix_free_node() conditionally frees memory based on the current global boot state rather than how the memory was originally allocated, leading to page state corruption or crashes. --
commit 470d80274c16904cd71b8b9c878c7272d6b58647 Author: Pratyush Yadav (Google) <[email protected]> kho: allow early-boot usage of the KHO radix tree This commit updates the KHO radix tree to support allocations during early boot when the buddy allocator is not yet available. It introduces kho_radix_alloc_node() and kho_radix_free_node() which use slab_is_available() to toggle between memblock and buddy allocators. > diff --git a/kernel/liveupdate/kexec_handover.c > b/kernel/liveupdate/kexec_handover.c > index f6de6bf632261..5c201e605b969 100644 > --- a/kernel/liveupdate/kexec_handover.c > +++ b/kernel/liveupdate/kexec_handover.c [ ... ] > @@ -143,6 +143,26 @@ static unsigned long kho_radix_get_table_index(unsigned > long key, > return (key >> s) % (1 << KHO_TABLE_SIZE_LOG2); > } > > +static void __ref *kho_radix_alloc_node(void) > +{ > + struct kho_radix_node *node; > + > + if (slab_is_available()) > + node = (struct kho_radix_node *)get_zeroed_page(GFP_KERNEL); > + else > + node = memblock_alloc(PAGE_SIZE, PAGE_SIZE); [Severity: High] Will this check cause memory corruption during the window between buddy initialization and slab initialization? During boot, memblock_free_all() hands over all unreserved pages to the buddy allocator. However, slab_is_available() remains false until kmem_cache_init() finishes later in mm_core_init(). If kho_radix_alloc_node() is called in this window, it evaluates slab_is_available() as false and falls back to memblock_alloc(). This means it will reserve physical memory that the buddy allocator is already actively managing, potentially leading to a silent double-allocation where KHO radix tree nodes alias with other kernel allocations. > + > + return node; > +} > + > +static void __ref kho_radix_free_node(struct kho_radix_node *node) > +{ > + if (slab_is_available()) > + free_page((unsigned long)node); > + else > + memblock_free(node, PAGE_SIZE); > +} [Severity: High] Could checking slab_is_available() at the time of freeing cause crashes or memory leaks for nodes allocated during early boot? If a node is allocated early via memblock_alloc(), it is backed by a PageReserved physical page. If this same node is later freed after the slab allocator has been initialized, slab_is_available() will be true, causing the free path to incorrectly call free_page() on the reserved page. Calling free_page() on a memblock-reserved page fails in __free_pages_prepare() because PAGE_FLAGS_CHECK_AT_FREE includes PG_reserved. This triggers a bad_page() warning, dumps the stack, and leaks the memory. Should this code determine the deallocation method based on the node's actual allocation origin, such as checking PageReserved(virt_to_page(node)), rather than checking the current global boot state? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
