linux-kernel - RE: [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <BN7PR02MB4148D3E7BEB015390E813910D4BA2@BN7PR02MB4148.namprd02.prod.outlook.com>
Date: Wed, 23 Apr 2025 20:14:31 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Long Li <longli@...rosoft.com>, "longli@...uxonhyperv.com"
	<longli@...uxonhyperv.com>, KY Srinivasan <kys@...rosoft.com>, Haiyang Zhang
	<haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui
	<decui@...rosoft.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages
 aligned to system page boundary

From: Long Li <longli@...rosoft.com> Sent: Wednesday, April 23, 2025 11:40 AM
> > >
> > > There are use cases that interrupt and monitor pages are mapped to
> > > user-mode through UIO, they need to be system page aligned. Some
> > > Hyper-V allocation APIs introduced earlier broke those requirements.
> > >
> > > Fix those APIs by always allocating Hyper-V page at system page boundaries.
> >
> > I'd suggest doing away with the hv_alloc/free_*() functions entirely since they are
> > now reduced to just being a wrapper around __get_free_pages(), which doesn't
> > add any value. Once all the arm64 support and CoCo VM code settled out, it
> > turned out that these functions to allocate Hyper-V size pages had dwindling
> > usage.
> 
> There is a BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE) in those functions, but it
> probably doesn't do anything.

You could move the BUILD_BUG_ON() to vmbus_connection() where
one of the calls to __get_free_pages() is made. That would codify the
assumption that __get_free_pages() returns memory at least as large as
HV_HYP_PAGE_SIZE.

Michael

> 
> If there is no objection, I can remove these functions.
> 
> Long
> 
> >
> > Allocation of the interrupt and monitor pages can use __get_free_pages() directly,
> > and that properly captures the need for those allocations to be a full page. Just
> > add a comment that this wastes space when PAGE_SIZE
> > > HV_HYP_PAGE_SIZE, but is necessary because the page may be mapped
> > into user space by uio_hv_generic.
> >
> > The only other use is in hv_kmsg_dump_register(), and it can do
> > kzalloc(HV_HYP_PAGE_SIZE), since that case really is tied to the Hyper-V page
> > size, not PAGE_SIZE. There's no need to waste space by allocating a full page.
> >
> > Michael
> >
> > >
> > > Cc: stable@...r.kernel.org
> > > Fixes: ca48739e59df ("Drivers: hv: vmbus: Move Hyper-V page allocator
> > > to arch neutral
> > > code")
> > > Signed-off-by: Long Li <longli@...rosoft.com>
> > > ---
> > >  drivers/hv/hv_common.c | 29 +++++++----------------------
> > >  1 file changed, 7 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index
> > > a7d7494feaca..f426aaa9b8f9 100644
> > > --- a/drivers/hv/hv_common.c
> > > +++ b/drivers/hv/hv_common.c
> > > @@ -106,41 +106,26 @@ void __init hv_common_free(void)  }
> > >
> > >  /*
> > > - * Functions for allocating and freeing memory with size and
> > > - * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> > > - * the guest page size may not be the same as the Hyper-V page
> > > - * size. We depend upon kmalloc() aligning power-of-two size
> > > - * allocations to the allocation size boundary, so that the
> > > - * allocated memory appears to Hyper-V as a page of the size
> > > - * it expects.
> > > + * A Hyper-V page can be used by UIO for mapping to user-space, it
> > > + should
> > > + * always be allocated on system page boundaries.
> > >   */
> > > -
> > >  void *hv_alloc_hyperv_page(void)
> > >  {
> > > -	BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> > > -
> > > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > > -		return (void *)__get_free_page(GFP_KERNEL);
> > > -	else
> > > -		return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > > +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> > > +	return (void *)__get_free_page(GFP_KERNEL);
> > >  }
> > >  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> > >
> > >  void *hv_alloc_hyperv_zeroed_page(void)
> > >  {
> > > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > > -		return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > > -	else
> > > -		return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > > +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> > > +	return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > >  }
> > >  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
> > >
> > >  void hv_free_hyperv_page(void *addr)
> > >  {
> > > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > > -		free_page((unsigned long)addr);
> > > -	else
> > > -		kfree(addr);
> > > +	free_page((unsigned long)addr);
> > >  }
> > >  EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
> > >
> > > --
> > > 2.34.1
> > >
>