* [PATCH] mshv: Fix error handling in mshv_region_populate_pages
@ 2026-03-17 15:04 Stanislav Kinsburskii
2026-03-17 21:56 ` Michael Kelley
0 siblings, 1 reply; 6+ messages in thread
From: Stanislav Kinsburskii @ 2026-03-17 15:04 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli; +Cc: linux-hyperv, linux-kernel
The current error handling has two issues:
First, pin_user_pages_fast() can return a short pin count (less than
requested but greater than zero) when it cannot pin all requested pages.
This is treated as success, leading to partially pinned regions being
used, which causes memory corruption.
Second, when an error occurs mid-loop, already pinned pages from the
current batch are not released before calling mshv_region_evict_pages(),
causing a page reference leak.
Fix by treating short pins as errors and explicitly unpinning the
partial batch before cleanup.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
drivers/hv/mshv_regions.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
index c28aac0726de..fdffd4f002f6 100644
--- a/drivers/hv/mshv_regions.c
+++ b/drivers/hv/mshv_regions.c
@@ -314,15 +314,17 @@ int mshv_region_pin(struct mshv_mem_region *region)
ret = pin_user_pages_fast(userspace_addr, nr_pages,
FOLL_WRITE | FOLL_LONGTERM,
pages);
- if (ret < 0)
+ if (ret != nr_pages)
goto release_pages;
}
return 0;
release_pages:
+ if (ret > 0)
+ done_count += ret;
mshv_region_invalidate_pages(region, 0, done_count);
- return ret;
+ return ret < 0 ? ret : -ENOMEM;
}
static int mshv_region_chunk_unmap(struct mshv_mem_region *region,
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] mshv: Fix error handling in mshv_region_populate_pages
2026-03-17 15:04 [PATCH] mshv: Fix error handling in mshv_region_populate_pages Stanislav Kinsburskii
@ 2026-03-17 21:56 ` Michael Kelley
2026-03-18 6:20 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Michael Kelley @ 2026-03-17 21:56 UTC (permalink / raw)
To: Stanislav Kinsburskii, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, longli@microsoft.com
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, March 17, 2026 8:05 AM
>
> The current error handling has two issues:
>
> First, pin_user_pages_fast() can return a short pin count (less than
> requested but greater than zero) when it cannot pin all requested pages.
> This is treated as success, leading to partially pinned regions being
> used, which causes memory corruption.
>
> Second, when an error occurs mid-loop, already pinned pages from the
> current batch are not released before calling mshv_region_evict_pages(),
> causing a page reference leak.
There's now an online LLM-based tool that is automatically reviewing
kernel patches. For this patch, the results are here:
https://sashiko.dev/#/patchset/177375989324.25621.6532741522672582851.stgit%40skinsburskii-cloud-desktop.internal.cloudapp.net
It has flagged the commit message as incorrectly referencing the
function mshv_region_evict_pages(), which doesn't exist.
FWIW, the announcement about sashiko.dev is here:
https://lore.kernel.org/lkml/7ia4o6kmpj5s.fsf@castle.c.googlers.com/
Other than the commit message reference, this looks good to me.
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
>
> Fix by treating short pins as errors and explicitly unpinning the
> partial batch before cleanup.
>
> Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> ---
> drivers/hv/mshv_regions.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
> index c28aac0726de..fdffd4f002f6 100644
> --- a/drivers/hv/mshv_regions.c
> +++ b/drivers/hv/mshv_regions.c
> @@ -314,15 +314,17 @@ int mshv_region_pin(struct mshv_mem_region *region)
> ret = pin_user_pages_fast(userspace_addr, nr_pages,
> FOLL_WRITE | FOLL_LONGTERM,
> pages);
> - if (ret < 0)
> + if (ret != nr_pages)
> goto release_pages;
> }
>
> return 0;
>
> release_pages:
> + if (ret > 0)
> + done_count += ret;
> mshv_region_invalidate_pages(region, 0, done_count);
> - return ret;
> + return ret < 0 ? ret : -ENOMEM;
> }
>
> static int mshv_region_chunk_unmap(struct mshv_mem_region *region,
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mshv: Fix error handling in mshv_region_populate_pages
2026-03-17 21:56 ` Michael Kelley
@ 2026-03-18 6:20 ` Wei Liu
2026-03-18 14:38 ` Michael Kelley
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2026-03-18 6:20 UTC (permalink / raw)
To: Michael Kelley
Cc: Stanislav Kinsburskii, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, longli@microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
On Tue, Mar 17, 2026 at 09:56:07PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, March 17, 2026 8:05 AM
> >
> > The current error handling has two issues:
> >
> > First, pin_user_pages_fast() can return a short pin count (less than
> > requested but greater than zero) when it cannot pin all requested pages.
> > This is treated as success, leading to partially pinned regions being
> > used, which causes memory corruption.
> >
> > Second, when an error occurs mid-loop, already pinned pages from the
> > current batch are not released before calling mshv_region_evict_pages(),
> > causing a page reference leak.
>
> There's now an online LLM-based tool that is automatically reviewing
> kernel patches. For this patch, the results are here:
>
> https://sashiko.dev/#/patchset/177375989324.25621.6532741522672582851.stgit%40skinsburskii-cloud-desktop.internal.cloudapp.net
>
> It has flagged the commit message as incorrectly referencing the
> function mshv_region_evict_pages(), which doesn't exist.
>
> FWIW, the announcement about sashiko.dev is here:
>
> https://lore.kernel.org/lkml/7ia4o6kmpj5s.fsf@castle.c.googlers.com/
>
> Other than the commit message reference, this looks good to me.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
The second point is written as if the code here should release the
already pinned pages before calling mshv_region_invalidate_pages(), but
the code actually relies on mshv_mem_region_invalidate_pages() to
release the pages. The change here fixes the accounting.
Second, when an error occurs mid-loop, already pinned pages from the
current batch are not accounted for before calling
mshv_region_invalidate_pages(), causing a page reference leak.
And queued up the patch to hyperv-fixes.
Wei
>
> >
> > Fix by treating short pins as errors and explicitly unpinning the
> > partial batch before cleanup.
> >
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > ---
> > drivers/hv/mshv_regions.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
> > index c28aac0726de..fdffd4f002f6 100644
> > --- a/drivers/hv/mshv_regions.c
> > +++ b/drivers/hv/mshv_regions.c
> > @@ -314,15 +314,17 @@ int mshv_region_pin(struct mshv_mem_region *region)
> > ret = pin_user_pages_fast(userspace_addr, nr_pages,
> > FOLL_WRITE | FOLL_LONGTERM,
> > pages);
> > - if (ret < 0)
> > + if (ret != nr_pages)
> > goto release_pages;
> > }
> >
> > return 0;
> >
> > release_pages:
> > + if (ret > 0)
> > + done_count += ret;
> > mshv_region_invalidate_pages(region, 0, done_count);
> > - return ret;
> > + return ret < 0 ? ret : -ENOMEM;
> > }
> >
> > static int mshv_region_chunk_unmap(struct mshv_mem_region *region,
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] mshv: Fix error handling in mshv_region_populate_pages
2026-03-18 6:20 ` Wei Liu
@ 2026-03-18 14:38 ` Michael Kelley
2026-03-18 16:20 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Michael Kelley @ 2026-03-18 14:38 UTC (permalink / raw)
To: Wei Liu, Michael Kelley
Cc: Stanislav Kinsburskii, kys@microsoft.com, haiyangz@microsoft.com,
decui@microsoft.com, longli@microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
From: Wei Liu <wei.liu@kernel.org> Sent: Tuesday, March 17, 2026 11:20 PM
>
> On Tue, Mar 17, 2026 at 09:56:07PM +0000, Michael Kelley wrote:
> > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, March 17, 2026 8:05 AM
> > >
> > > The current error handling has two issues:
> > >
> > > First, pin_user_pages_fast() can return a short pin count (less than
> > > requested but greater than zero) when it cannot pin all requested pages.
> > > This is treated as success, leading to partially pinned regions being
> > > used, which causes memory corruption.
> > >
> > > Second, when an error occurs mid-loop, already pinned pages from the
> > > current batch are not released before calling mshv_region_evict_pages(),
> > > causing a page reference leak.
> >
> > There's now an online LLM-based tool that is automatically reviewing
> > kernel patches. For this patch, the results are here:
> >
> >
> https://sashiko.dev/#/patchset/177375989324.25621.6532741522672582851.stgit
> %40skinsburskii-cloud-desktop.internal.cloudapp.net
> >
> > It has flagged the commit message as incorrectly referencing the
> > function mshv_region_evict_pages(), which doesn't exist.
> >
> > FWIW, the announcement about sashiko.dev is here:
> >
> > https://lore.kernel.org/lkml/7ia4o6kmpj5s.fsf@castle.c.googlers.com/
> >
> > Other than the commit message reference, this looks good to me.
> >
> > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
>
> The second point is written as if the code here should release the
> already pinned pages before calling mshv_region_invalidate_pages(), but
> the code actually relies on mshv_mem_region_invalidate_pages() to
> release the pages. The change here fixes the accounting.
>
> Second, when an error occurs mid-loop, already pinned pages from the
> current batch are not accounted for before calling
> mshv_region_invalidate_pages(), causing a page reference leak.
>
> And queued up the patch to hyperv-fixes.
One other thing I noticed: The "Subject" of the patch is wrong. It
mentions mshv_region_populate_pages(), but the function being
modified is actually mshv_region_pin().
Michael
>
> Wei
>
> >
> > >
> > > Fix by treating short pins as errors and explicitly unpinning the
> > > partial batch before cleanup.
> > >
> > > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > > ---
> > > drivers/hv/mshv_regions.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
> > > index c28aac0726de..fdffd4f002f6 100644
> > > --- a/drivers/hv/mshv_regions.c
> > > +++ b/drivers/hv/mshv_regions.c
> > > @@ -314,15 +314,17 @@ int mshv_region_pin(struct mshv_mem_region *region)
> > > ret = pin_user_pages_fast(userspace_addr, nr_pages,
> > > FOLL_WRITE | FOLL_LONGTERM,
> > > pages);
> > > - if (ret < 0)
> > > + if (ret != nr_pages)
> > > goto release_pages;
> > > }
> > >
> > > return 0;
> > >
> > > release_pages:
> > > + if (ret > 0)
> > > + done_count += ret;
> > > mshv_region_invalidate_pages(region, 0, done_count);
> > > - return ret;
> > > + return ret < 0 ? ret : -ENOMEM;
> > > }
> > >
> > > static int mshv_region_chunk_unmap(struct mshv_mem_region *region,
> > >
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mshv: Fix error handling in mshv_region_populate_pages
2026-03-18 14:38 ` Michael Kelley
@ 2026-03-18 16:20 ` Wei Liu
2026-03-23 16:09 ` Stanislav Kinsburskii
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2026-03-18 16:20 UTC (permalink / raw)
To: Michael Kelley
Cc: Wei Liu, Stanislav Kinsburskii, kys@microsoft.com,
haiyangz@microsoft.com, decui@microsoft.com, longli@microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
On Wed, Mar 18, 2026 at 02:38:49PM +0000, Michael Kelley wrote:
> From: Wei Liu <wei.liu@kernel.org> Sent: Tuesday, March 17, 2026 11:20 PM
> >
> > On Tue, Mar 17, 2026 at 09:56:07PM +0000, Michael Kelley wrote:
> > > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, March 17, 2026 8:05 AM
> > > >
> > > > The current error handling has two issues:
> > > >
> > > > First, pin_user_pages_fast() can return a short pin count (less than
> > > > requested but greater than zero) when it cannot pin all requested pages.
> > > > This is treated as success, leading to partially pinned regions being
> > > > used, which causes memory corruption.
> > > >
> > > > Second, when an error occurs mid-loop, already pinned pages from the
> > > > current batch are not released before calling mshv_region_evict_pages(),
> > > > causing a page reference leak.
> > >
> > > There's now an online LLM-based tool that is automatically reviewing
> > > kernel patches. For this patch, the results are here:
> > >
> > >
> > https://sashiko.dev/#/patchset/177375989324.25621.6532741522672582851.stgit
> > %40skinsburskii-cloud-desktop.internal.cloudapp.net
> > >
> > > It has flagged the commit message as incorrectly referencing the
> > > function mshv_region_evict_pages(), which doesn't exist.
> > >
> > > FWIW, the announcement about sashiko.dev is here:
> > >
> > > https://lore.kernel.org/lkml/7ia4o6kmpj5s.fsf@castle.c.googlers.com/
> > >
> > > Other than the commit message reference, this looks good to me.
> > >
> > > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> >
> > The second point is written as if the code here should release the
> > already pinned pages before calling mshv_region_invalidate_pages(), but
> > the code actually relies on mshv_mem_region_invalidate_pages() to
> > release the pages. The change here fixes the accounting.
> >
> > Second, when an error occurs mid-loop, already pinned pages from the
> > current batch are not accounted for before calling
> > mshv_region_invalidate_pages(), causing a page reference leak.
> >
> > And queued up the patch to hyperv-fixes.
>
> One other thing I noticed: The "Subject" of the patch is wrong. It
> mentions mshv_region_populate_pages(), but the function being
> modified is actually mshv_region_pin().
Good catch. I have updated the subject line and pushed to hyperv-fixes.
Wei
>
> Michael
>
> >
> > Wei
> >
> > >
> > > >
> > > > Fix by treating short pins as errors and explicitly unpinning the
> > > > partial batch before cleanup.
> > > >
> > > > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > > > ---
> > > > drivers/hv/mshv_regions.c | 6 ++++--
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
> > > > index c28aac0726de..fdffd4f002f6 100644
> > > > --- a/drivers/hv/mshv_regions.c
> > > > +++ b/drivers/hv/mshv_regions.c
> > > > @@ -314,15 +314,17 @@ int mshv_region_pin(struct mshv_mem_region *region)
> > > > ret = pin_user_pages_fast(userspace_addr, nr_pages,
> > > > FOLL_WRITE | FOLL_LONGTERM,
> > > > pages);
> > > > - if (ret < 0)
> > > > + if (ret != nr_pages)
> > > > goto release_pages;
> > > > }
> > > >
> > > > return 0;
> > > >
> > > > release_pages:
> > > > + if (ret > 0)
> > > > + done_count += ret;
> > > > mshv_region_invalidate_pages(region, 0, done_count);
> > > > - return ret;
> > > > + return ret < 0 ? ret : -ENOMEM;
> > > > }
> > > >
> > > > static int mshv_region_chunk_unmap(struct mshv_mem_region *region,
> > > >
> > > >
> > >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mshv: Fix error handling in mshv_region_populate_pages
2026-03-18 16:20 ` Wei Liu
@ 2026-03-23 16:09 ` Stanislav Kinsburskii
0 siblings, 0 replies; 6+ messages in thread
From: Stanislav Kinsburskii @ 2026-03-23 16:09 UTC (permalink / raw)
To: Wei Liu
Cc: Michael Kelley, kys@microsoft.com, haiyangz@microsoft.com,
decui@microsoft.com, longli@microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
On Wed, Mar 18, 2026 at 04:20:03PM +0000, Wei Liu wrote:
> On Wed, Mar 18, 2026 at 02:38:49PM +0000, Michael Kelley wrote:
> > From: Wei Liu <wei.liu@kernel.org> Sent: Tuesday, March 17, 2026 11:20 PM
> > >
> > > On Tue, Mar 17, 2026 at 09:56:07PM +0000, Michael Kelley wrote:
> > > > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, March 17, 2026 8:05 AM
> > > > >
> > > > > The current error handling has two issues:
> > > > >
> > > > > First, pin_user_pages_fast() can return a short pin count (less than
> > > > > requested but greater than zero) when it cannot pin all requested pages.
> > > > > This is treated as success, leading to partially pinned regions being
> > > > > used, which causes memory corruption.
> > > > >
> > > > > Second, when an error occurs mid-loop, already pinned pages from the
> > > > > current batch are not released before calling mshv_region_evict_pages(),
> > > > > causing a page reference leak.
> > > >
> > > > There's now an online LLM-based tool that is automatically reviewing
> > > > kernel patches. For this patch, the results are here:
> > > >
> > > >
> > > https://sashiko.dev/#/patchset/177375989324.25621.6532741522672582851.stgit
> > > %40skinsburskii-cloud-desktop.internal.cloudapp.net
> > > >
> > > > It has flagged the commit message as incorrectly referencing the
> > > > function mshv_region_evict_pages(), which doesn't exist.
> > > >
> > > > FWIW, the announcement about sashiko.dev is here:
> > > >
> > > > https://lore.kernel.org/lkml/7ia4o6kmpj5s.fsf@castle.c.googlers.com/
> > > >
> > > > Other than the commit message reference, this looks good to me.
> > > >
> > > > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> > >
> > > The second point is written as if the code here should release the
> > > already pinned pages before calling mshv_region_invalidate_pages(), but
> > > the code actually relies on mshv_mem_region_invalidate_pages() to
> > > release the pages. The change here fixes the accounting.
> > >
> > > Second, when an error occurs mid-loop, already pinned pages from the
> > > current batch are not accounted for before calling
> > > mshv_region_invalidate_pages(), causing a page reference leak.
> > >
> > > And queued up the patch to hyperv-fixes.
> >
> > One other thing I noticed: The "Subject" of the patch is wrong. It
> > mentions mshv_region_populate_pages(), but the function being
> > modified is actually mshv_region_pin().
>
> Good catch. I have updated the subject line and pushed to hyperv-fixes.
>
Thank you Michael and Wei.
Thanks,
Stanislav
> Wei
>
> >
> > Michael
> >
> > >
> > > Wei
> > >
> > > >
> > > > >
> > > > > Fix by treating short pins as errors and explicitly unpinning the
> > > > > partial batch before cleanup.
> > > > >
> > > > > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > > > > ---
> > > > > drivers/hv/mshv_regions.c | 6 ++++--
> > > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
> > > > > index c28aac0726de..fdffd4f002f6 100644
> > > > > --- a/drivers/hv/mshv_regions.c
> > > > > +++ b/drivers/hv/mshv_regions.c
> > > > > @@ -314,15 +314,17 @@ int mshv_region_pin(struct mshv_mem_region *region)
> > > > > ret = pin_user_pages_fast(userspace_addr, nr_pages,
> > > > > FOLL_WRITE | FOLL_LONGTERM,
> > > > > pages);
> > > > > - if (ret < 0)
> > > > > + if (ret != nr_pages)
> > > > > goto release_pages;
> > > > > }
> > > > >
> > > > > return 0;
> > > > >
> > > > > release_pages:
> > > > > + if (ret > 0)
> > > > > + done_count += ret;
> > > > > mshv_region_invalidate_pages(region, 0, done_count);
> > > > > - return ret;
> > > > > + return ret < 0 ? ret : -ENOMEM;
> > > > > }
> > > > >
> > > > > static int mshv_region_chunk_unmap(struct mshv_mem_region *region,
> > > > >
> > > > >
> > > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-23 16:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 15:04 [PATCH] mshv: Fix error handling in mshv_region_populate_pages Stanislav Kinsburskii
2026-03-17 21:56 ` Michael Kelley
2026-03-18 6:20 ` Wei Liu
2026-03-18 14:38 ` Michael Kelley
2026-03-18 16:20 ` Wei Liu
2026-03-23 16:09 ` Stanislav Kinsburskii
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox