Re: [PATCH v1 2/2] kho: unwind restored pages on kho_restore_vmalloc error

[Pratyush Yadav]

On Fri, Aug 07 2026, Chenghao Duan wrote:

> In kho_restore_vmalloc(), when kho_restore_pages() succeeds, the
> recovered pages are handed to the buddy allocator (via
> adjust_managed_page_count()). If any later step (e.g.
> __get_vm_area_node() or vmap_pages_range()) fails, the original error
> path only called kvfree(pages), leaking those folio pages.
>
> Fix by tracking how many folio groups have been restored with
> restored_idx. On any failure, use err_unwind_restored to walk
> restored_idx backwards and return each folio group to the buddy via
> __free_pages() before freeing the pages array.
>
> Signed-off-by: Chenghao Duan <duanchenghao@xxxxxxxxxx>
> ---
>  kernel/liveupdate/kexec_handover.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index ba03ff5baa9e..ed35405f59ab 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -1161,6 +1161,7 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  	struct vm_struct *area;
>  	struct page **pages;
>  	unsigned int idx = 0;
> +	unsigned int restored_idx = 0;
>  	int err;
>  
>  	vm_flags = kho_flags_to_vmalloc(preservation->flags);
> @@ -1183,11 +1184,11 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  			phys_addr_t phys = chunk->phys[i];
>  
>  			if (idx + contig_pages > total_pages)
> -				goto err_free_pages_array;
> +				goto err_unwind_restored;
>  
>  			page = kho_restore_pages(phys, contig_pages);
>  			if (!page)
> -				goto err_free_pages_array;
> +				goto err_unwind_restored;
>  
>  			for (int j = 0; j < contig_pages; j++)
>  				pages[idx++] = page + j;
> @@ -1195,13 +1196,14 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  
>  		page = kho_restore_pages(virt_to_phys(chunk), 1);
>  		if (!page)
> -			goto err_free_pages_array;
> +			goto err_unwind_restored;
> +		restored_idx = idx;
>  		chunk = KHOSER_LOAD_PTR(chunk->hdr.next);
>  		__free_page(page);
>  	}
>  
>  	if (idx != total_pages)
> -		goto err_free_pages_array;
> +		goto err_unwind_restored;
>  
>  	area = __get_vm_area_node(total_pages * PAGE_SIZE, align, shift,
>  				  vm_flags | VM_UNINITIALIZED,
> @@ -1209,7 +1211,7 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  				  NUMA_NO_NODE, GFP_KERNEL,
>  				  __builtin_return_address(0));
>  	if (!area)
> -		goto err_free_pages_array;
> +		goto err_unwind_restored;
>  
>  	addr = (unsigned long)area->addr;
>  	size = get_vm_area_size(area);
> @@ -1231,7 +1233,16 @@ void *kho_restore_vmalloc(const struct kho_vmalloc *preservation)
>  
>  err_free_vm_area:
>  	free_vm_area(area);
> -err_free_pages_array:
> +err_unwind_restored:
> +	/*
> +	 * Pages already restored via kho_restore_pages() have been given to
> +	 * the buddy allocator (via adjust_managed_page_count()). Return them
> +	 * to the buddy so that failure leaves the system in a clean state.
> +	 */
> +	while (restored_idx > 0) {
> +		restored_idx -= contig_pages;
> +		__free_pages(pages[restored_idx], order);
> +	}

This looks wrong. These are 0-order pages. You can't free them at order.

I sent a patch to do exactly this a while ago [0]. At the time it was
rejected with the below argument.

    Hm, I am not sure if KHO should be responsible for freeing the
    restored pages. We don't know the content of those pages, and what
    they are used for. They could be used by a hypervisor or a device.
    Therefore, it may be better to keep them leaked, and let the caller
    decide what to do next: i.e., boot into a maintenance mode, crash the
    kernel, or allow the leak until the next reboot.

Although thinking about this again, why would any device or hypervisor
use vmalloc buffers? They should only be used for metadata. So perhaps
we take another look at my patch? Pasha, what do you think?

[0] https://lore.kernel.org/all/20251118181811.47336-1-pratyush@xxxxxxxxxx/

>  	kvfree(pages);
>  	return NULL;
>  }

-- 
Regards,
Pratyush Yadav