Re: [PATCH v4] drivers/hv: remove deposited pages from direct map

[Wei Liu <wei.liu@kernel.org>]

On Wed, Oct 28, 2026 at 07:28:15PM +0100, Thorsten Leemhuis wrote:
> On 9/30/26 14:10, Magnus Kulke wrote:
> > hv_call_deposit_pages() donates pages (deposit) to hypervisor for L2
> > guest on L1VH systems via HVCALL_DEPOSIT_MEMORY. The hypervisor takes
> > ownership of those pages and per contract revokes root partition
> > access to them, raising a #GP on access from the L1VH root partition.
> > 
> > However, the pages remain mapped in the kernel direct map, so kernel
> > code may still access them even though the hypervisor has revoked
> > access.
> 
> Wei, Magnus, this patch showed up in -next today and broke my daily
> Fedora builds on x86_64 and arm64 which use the rawhide config as base.
> Reverting it made the build succeed.
> 

Can you share the kconfig?

Thanks,
Wei

> """
> > drivers/hv/hv_proc.c: In function ‘hv_deposit_update_direct_map’:
> > drivers/hv/hv_proc.c:32:23: error: implicit declaration of function ‘set_direct_map_valid_noflush’; did you mean ‘set_direct_map_invalid_noflush’? [-Wimplicit-function-declaration]
> >    32 |                 err = set_direct_map_valid_noflush(pfn_to_page(pfns[i]),
> >       |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >       |                       set_direct_map_invalid_noflush
> > make[4]: *** [scripts/Makefile.build:316: drivers/hv/hv_proc.o] Error 1
> > make[4]: *** Waiting for unfinished jobs....
> > make[3]: *** [scripts/Makefile.build:598: drivers/hv] Error 2
> > make[3]: *** Waiting for unfinished jobs....
> > make[2]: *** [scripts/Makefile.build:598: drivers] Error 2
> > make[2]: *** Waiting for unfinished jobs....
> > make[1]: *** [/builddir/build/BUILD/kernel-7.4.0-build/kernel-next-20261028/linux-7.4.0-0.0.next.20261028.509.vanilla.fc46.x86_64/Makefile:2252: .] Error 2
> > make: *** [Makefile:256: __sub-make] Error 2
> """
> Full log:
> https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/11219444-next-next-all/builder-live.log.gz
> 
> Ciao, Thorsten
> 
> > Helpers such as "load_unaligned_zeropad()" deliberately read past the
> > end of a buffer and across page boundaries. A read into an unmapped
> > page is tolerated and triggers a #PF, for which the kernel executes
> > a fixup in the exception table.
> > 
> > If such a call steps into a page that has been deposited, the access
> > raises a #GP by the hypervisor from which the above handler cannot
> > recover and the kernel panics:
> > 
> >   Oops: general protection fault, maybe for address 0xff1100941a3dfffc
> >   RIP: 0010:csum_partial+0xe5/0x110
> > 
> > This condition will appear on L1VH system that have created L2
> > partitions (and hence deposited pages) and exercise networking code
> > paths such as csum_partial() can trigger this condition when a buffer
> > ends close to a page boundary (e.g fffc in the above example).
> > 
> > The fix is to remove the deposited pages from the direct map before
> > they are passed to the hypervisor, and restore them when the hypervisor
> > returns them again.
> > 
> > We want to avoid flushing the TLB in the loop, so we use the _noflush()
> > variant of set_direct_map_valid() when marking a deposited page invalid
> > and flush the affected page ranges in one go ourselves. In the opposite
> > direction this is not required:
> > 
> >   > If a paging-structure entry is modified to change the P flag from
> >   > 0 to 1, no invalidation is necessary. This is because no TLB entry
> >   > or paging-structure cache entry is created with information from a
> >   > paging-structure entry in which the P flag is 0.
> > 
> > (Intel SDM Vol. 3, 4.10.4.3)
> > 
> > Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
> > ---
> > Changes since v3:
> > - Batch contiguous pages in the restore path
> > - Adjusted comment about failure modes
> > 
> > Changes since v2:
> > - Checkpatch format fix
> > 
> > Changes since RFC:
> > - Handle direct-map restoration failures without returning unmapped
> >   pages to the allocator.
> > - Move freeing of withdrawn pages into the restoration helper.
> > ---
> >  drivers/hv/hv_proc.c           | 83 +++++++++++++++++++++++++++++++++-
> >  drivers/hv/mshv_root_hv_call.c |  4 +-
> >  include/asm-generic/mshyperv.h |  4 ++
> >  3 files changed, 87 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
> > index 57b2c64197cb..b056051a5ac5 100644
> > --- a/drivers/hv/hv_proc.c
> > +++ b/drivers/hv/hv_proc.c
> > @@ -7,7 +7,9 @@
> >  #include <linux/cpuhotplug.h>
> >  #include <linux/minmax.h>
> >  #include <linux/export.h>
> > +#include <linux/set_memory.h>
> >  #include <asm/mshyperv.h>
> > +#include <asm/tlbflush.h>
> >  
> >  /*
> >   * See struct hv_deposit_memory. The first u64 is partition ID, the rest
> > @@ -15,6 +17,35 @@
> >   */
> >  #define HV_DEPOSIT_MAX (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
> >  
> > +/*
> > + * Add or remove a set of physically contiguous page runs from the kernel
> > + * direct map. Once a page has been deposited the hypervisor owns it and
> > + * revokes root partition access to it.
> > + */
> > +static int hv_deposit_update_direct_map(struct page **pages, int *counts,
> > +					int num_allocations, bool valid)
> > +{
> > +	int i, err, ret = 0;
> > +
> > +	for (i = 0; i < num_allocations; ++i) {
> > +		err = set_direct_map_valid_noflush(pages[i], counts[i], valid);
> > +		if (err && !ret)
> > +			ret = err;
> > +	}
> > +
> > +	if (valid)
> > +		return ret;
> > +
> > +	for (i = 0; i < num_allocations; ++i) {
> > +		unsigned long addr = (unsigned long)page_address(pages[i]);
> > +		unsigned long size = (unsigned long)counts[i] << PAGE_SHIFT;
> > +
> > +		flush_tlb_kernel_range(addr, addr + size);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  /* Deposits exact number of pages. Must be called with interrupts enabled.  */
> >  int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> >  {
> > @@ -72,6 +103,10 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> >  	}
> >  	num_allocations = i;
> >  
> > +	ret = hv_deposit_update_direct_map(pages, counts, num_allocations, false);
> > +	if (ret)
> > +		goto err_restore_direct_map;
> > +
> >  	local_irq_save(flags);
> >  
> >  	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > @@ -90,12 +125,23 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> >  	if (!hv_result_success(status)) {
> >  		hv_status_err(status, "\n");
> >  		ret = hv_result_to_errno(status);
> > -		goto err_free_allocations;
> > +		goto err_restore_direct_map;
> >  	}
> >  
> >  	ret = 0;
> >  	goto free_buf;
> >  
> > +err_restore_direct_map:
> > +	/*
> > +	 * We don't want to return pages to the allocator if weren't able to
> > +	 * mark them valid in the direct map.
> > +	 */
> > +	if (hv_deposit_update_direct_map(pages, counts, num_allocations, true)) {
> > +		WARN(1, "leaking %d page block(s) that could not be set to valid\n",
> > +		     num_allocations);
> > +		goto free_buf;
> > +	}
> > +
> >  err_free_allocations:
> >  	for (i = 0; i < num_allocations; ++i) {
> >  		base_pfn = page_to_pfn(pages[i]);
> > @@ -110,6 +156,41 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> >  }
> >  EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
> >  
> > +/*
> > + * Put withdrawn pages back in the direct map. Counterpart to the direct map
> > + * removal done by hv_call_deposit_pages().
> > + */
> > +void hv_restore_withdrawn_pages(const u64 *pfns, int count)
> > +{
> > +	int i, j, end, n, ret;
> > +	struct page *page;
> > +
> > +	for (i = 0; i < count; i = end) {
> > +		page = pfn_to_page(pfns[i]);
> > +		/* find consecutive runs of pfns */
> > +		for (end = i + 1; end < count; ++end) {
> > +			if (pfns[end] != pfns[end - 1] + 1)
> > +				break;
> > +		}
> > +		n = end - i;
> > +		ret = set_direct_map_valid_noflush(page, n, true);
> > +
> > +		for (j = i; j < end; ++j) {
> > +			page = pfn_to_page(pfns[j]);
> > +			/*
> > +			 * Retry individually on failed batch. We don't want
> > +			 * to return pages to the allocator that we couldn't mark
> > +			 * valid in the direct map.
> > +			 */
> > +			if (ret &&
> > +			    WARN_ON_ONCE(set_direct_map_valid_noflush(page, 1, true)))
> > +				continue;
> > +			__free_page(page);
> > +		}
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(hv_restore_withdrawn_pages);
> > +
> >  int hv_deposit_memory_node(int node, u64 partition_id,
> >  			   u64 hv_status)
> >  {
> > diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> > index cb55d4d4be2e..150a0c63ebc8 100644
> > --- a/drivers/hv/mshv_root_hv_call.c
> > +++ b/drivers/hv/mshv_root_hv_call.c
> > @@ -46,7 +46,6 @@ int hv_call_withdraw_memory(u64 count, int node, u64 partition_id)
> >  	struct page *page;
> >  	u16 completed;
> >  	u64 status, withdrawn = 0;
> > -	int i;
> >  	unsigned long flags;
> >  
> >  	page = alloc_page(GFP_KERNEL);
> > @@ -69,8 +68,7 @@ int hv_call_withdraw_memory(u64 count, int node, u64 partition_id)
> >  
> >  		completed = hv_repcomp(status);
> >  
> > -		for (i = 0; i < completed; i++)
> > -			__free_page(pfn_to_page(output_page->gpa_page_list[i]));
> > +		hv_restore_withdrawn_pages(output_page->gpa_page_list, completed);
> >  
> >  		if (!hv_result_success(status)) {
> >  			if (hv_result(status) == HV_STATUS_NO_RESOURCES)
> > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> > index bf601d67cecb..397c8ec0ce9a 100644
> > --- a/include/asm-generic/mshyperv.h
> > +++ b/include/asm-generic/mshyperv.h
> > @@ -346,6 +346,7 @@ static inline bool hv_parent_partition(void)
> >  bool hv_result_needs_memory(u64 status);
> >  int hv_deposit_memory_node(int node, u64 partition_id, u64 status);
> >  int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
> > +void hv_restore_withdrawn_pages(const u64 *pfns, int count);
> >  int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
> >  int hv_call_notify_all_processors_started(void);
> >  bool hv_lp_exists(u32 lp_index);
> > @@ -364,6 +365,9 @@ static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_page
> >  {
> >  	return -EOPNOTSUPP;
> >  }
> > +
> > +static inline void hv_restore_withdrawn_pages(const u64 *pfns, int count) { }
> > +
> >  static inline int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id)
> >  {
> >  	return -EOPNOTSUPP;
>