Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Pratyush Yadav <pratyush@kernel.org>
To: Chenghao Duan <duanchenghao@kylinos.cn>
Cc: pasha.tatashin@soleen.com,  rppt@kernel.org,
	 pratyush@kernel.org, graf@amazon.com,
	 linux-kernel@vger.kernel.org,  linux-mm@kvack.org,
	kexec@lists.infradead.org,  jianghaoran@kylinos.cn
Subject: Re: [PATCH v1 2/2] kho: unwind restored pages on kho_restore_vmalloc error
Date: Tue, 11 Aug 2026 13:46:04 +0200	[thread overview]
Message-ID: <2vxzcxvo518j.fsf@kernel.org> (raw)
In-Reply-To: <20260807075128.542806-3-duanchenghao@kylinos.cn> (Chenghao Duan's message of "Fri, 7 Aug 2026 15:51:28 +0800")

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@kylinos.cn>
> ---
>  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@kernel.org/

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

-- 
Regards,
Pratyush Yadav


      reply	other threads:[~2026-08-11 11:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07  7:51 [PATCH v1 0/2] Optimize the code in kexec_handover.c Chenghao Duan
2026-08-07  7:51 ` [PATCH v1 1/2] kho: remove useless phys increment in inner loop Chenghao Duan
2026-08-11 11:39   ` Pratyush Yadav
2026-08-07  7:51 ` [PATCH v1 2/2] kho: unwind restored pages on kho_restore_vmalloc error Chenghao Duan
2026-08-11 11:46   ` Pratyush Yadav [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2vxzcxvo518j.fsf@kernel.org \
    --to=pratyush@kernel.org \
    --cc=duanchenghao@kylinos.cn \
    --cc=graf@amazon.com \
    --cc=jianghaoran@kylinos.cn \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pasha.tatashin@soleen.com \
    --cc=rppt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox