[PATCH] mshv: Fix error handling in mshv_region_populate_pages

[PATCH] mshv: Fix error handling in mshv_region_populate_pages

[Stanislav Kinsburskii]

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,

RE: [PATCH] mshv: Fix error handling in mshv_region_populate_pages

[Michael Kelley]

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,
> 
> 

Re: [PATCH] mshv: Fix error handling in mshv_region_populate_pages

[Wei Liu]

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,
> > 
> > 
> 

RE: [PATCH] mshv: Fix error handling in mshv_region_populate_pages

[Michael Kelley]

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,
> > >
> > >
> >

Re: [PATCH] mshv: Fix error handling in mshv_region_populate_pages

[Wei Liu]

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,
> > > >
> > > >
> > >
> 

Re: [PATCH] mshv: Fix error handling in mshv_region_populate_pages

[Stanislav Kinsburskii]

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,
> > > > >
> > > > >
> > > >
> >