From: Nuno Das Neves <nunodasne...@linux.microsoft.com> Sent: Monday, July 28,
2025 10:03 AM
>
> On 7/17/2025 9:55 PM, mhkelle...@gmail.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>
