Re: [PATCH 6.18.y 2/2] kho: free phys_bits bitmaps using free_page() in __kho_abort()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: use-after-free of physxa in __kho_preserve_order() racing with abort
- [Medium] mm: memory leak of kho_vmalloc_chunk pages on KHO abort
--

commit a7bcf7c6e605c88c110bdd87cec6fe96445bb703
Author: Federico Parola <fparola@amazon.de>

kho: free phys_bits bitmaps using free_page() in __kho_abort()

This commit fixes a memory leak in KHO abort by replacing kfree() with
free_page() for releasing bitmap pages allocated via the buddy allocator.
Using kfree() on non-kmalloc buddy pages previously hit a WARN_ON_ONCE() and
leaked the pages during teardown.

> diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> index 9fe5d14f1ec54..9691f6bc2fba7 100644
> --- a/kernel/kexec_handover.c
> +++ b/kernel/kexec_handover.c
> @@ -1054,7 +1054,7 @@ static int __kho_abort(void)
>  		unsigned long phys;
>  
>  		xa_for_each(&physxa->phys_bits, phys, bits)
> -			kfree(bits);
> +			free_page((unsigned long)bits);
>  
>  		xa_destroy(&physxa->phys_bits);
>  		kfree(physxa);

[Severity: High]
Does this code introduce a use-after-free? The physxa structure is freed
without being removed from the XArray first, leaving a dangling pointer
visible to lockless readers until xa_destroy(&kho_out.ser.track.orders)
eventually clears the array.

If a driver calls kho_preserve_folio() concurrently with another thread
calling kho_abort(), could __kho_preserve_order() load the physxa pointer
locklessly and dereference it just after it is freed here?

kernel/kexec_handover.c:__kho_preserve_order() {
    ...
    physxa = xa_load(&track->orders, order);
    if (!physxa) {
        ...
    }

    bits = xa_load_or_alloc(&physxa->phys_bits, pfn_high / PRESERVE_BITS);
    ...
}

[Severity: Medium]
Does this code leak kho_vmalloc_chunk pages? When __kho_abort() drops the KHO
state by destroying the XArrays, it leaves the driver's metadata chunk
pages untracked.

In kho_preserve_vmalloc(), these pages are allocated and linked only to the
driver's struct kho_vmalloc object:

kernel/kexec_handover.c:kho_preserve_vmalloc() {
    ...
    chunk = new_vmalloc_chunk(NULL);
    if (!chunk)
        return -ENOMEM;
    KHOSER_STORE_PTR(preservation->first, chunk);
    ...
}

Since kho_vmalloc_free_chunks() is static, and there is no exported
kho_vmalloc_unpreserve() API, drivers cannot free these chunks inside their
KEXEC_KHO_ABORT notifiers. Doesn't this mean these pages are permanently
leaked whenever KHO finalization is aborted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015144529.76911-1-fparola@amazon.de?part=2