linux-kernel - RE: [PATCH v4 4/7] Drivers: hv: Use hv_setup

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:
 <BN7PR02MB414807A1A9174D1215AA365FD425A@BN7PR02MB4148.namprd02.prod.outlook.com>
Date: Tue, 29 Jul 2025 00:23:56 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Nuno Das Neves <nunodasneves@...ux.microsoft.com>, "kys@...rosoft.com"
	<kys@...rosoft.com>, "haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
	"wei.liu@...nel.org" <wei.liu@...nel.org>, "decui@...rosoft.com"
	<decui@...rosoft.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "hpa@...or.com"
	<hpa@...or.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kw@...ux.com" <kw@...ux.com>, "mani@...nel.org" <mani@...nel.org>,
	"robh@...nel.org" <robh@...nel.org>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "arnd@...db.de" <arnd@...db.de>
CC: "x86@...nel.org" <x86@...nel.org>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, "linux-arch@...r.kernel.org"
	<linux-arch@...r.kernel.org>
Subject: RE: [PATCH v4 4/7] Drivers: hv: Use hv_setup_*() to set up hypercall
 arguments

From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Monday, July 28, 2025 10:03 AM
> 
> On 7/17/2025 9:55 PM, mhkelley58@...il.com wrote:
> <snip>
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > index 2b4080e51f97..d9b569b204d2 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -1577,21 +1577,21 @@ static int hv_free_page_report(struct page_reporting_dev_info *pr_dev_info,
> >  {
> >  	unsigned long flags;
> >  	struct hv_memory_hint *hint;
> > -	int i, order;
> > +	int i, order, batch_size;
> >  	u64 status;
> >  	struct scatterlist *sg;
> >
> > -	WARN_ON_ONCE(nents > HV_MEMORY_HINT_MAX_GPA_PAGE_RANGES);
> >  	WARN_ON_ONCE(sgl->length < (HV_HYP_PAGE_SIZE << page_reporting_order));
> >  	local_irq_save(flags);
> > -	hint = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +	batch_size = hv_setup_in_array(&hint, sizeof(*hint), sizeof(hint->ranges[0]));
> >  	if (!hint) {
> >  		local_irq_restore(flags);
> >  		return -ENOSPC;
> >  	}
> > +	WARN_ON_ONCE(nents > batch_size);
> >
> 
> I don't think WARN_ON_ONCE is sufficient here... this looks like a bug in the current
> code. The loop below will go out of bounds of the input page if nents is too large.
> 
> Ideally this function would be refactored to batch the operation so that this isn't a
> problem.

Yes, I kept the existing functionality, which is slightly flawed. But there's not a
real problem, because "nents" is always PAGE_REPORTING_CAPACITY (which is
32) or smaller. See page_reporting_cycle(). Furthermore, the HV balloon driver
function enable_page_reporting() has a BUILD_BUG_ON to ensure everything fits.

Adding a batching loop around the hypercall here in hv_free_page_report() seems
like overkill unless PAGE_REPORTING_CAPACITY is changed to something larger.
Hyper-V has room for the value to be as large as 128. 

The virtio balloon driver does a similar check, though at runtime, and
virtio_balloon_probe() fails if its capacity isn't at least PAGE_REPORTING_CAPACITY.
The Hyper-V balloon driver could do the same. Tidying this up seems to me
to be a separate patch that's outside the scope of this series.

Michael

> 
> Nuno
> >  	hint->heat_type = HV_EXTMEM_HEAT_HINT_COLD_DISCARD;
> > -	hint->reserved = 0;
> >  	for_each_sg(sgl, sg, nents, i) {
> >  		union hv_gpa_page_range *range;
> >
> <snip>