[PATCH 0/2] vhost-vdpa: fix a pin leak and unpin mappings by range

[PATCH 0/2] vhost-vdpa: fix a pin leak and unpin mappings by range

[Tal Zussman]

While testing the second patch I ran into a pinned page leak in the
vhost_vdpa_pa_map() error path. The first patch fixes that and is
tagged for stable.

The second patch unpins each IOTLB mapping as a range with
unpin_user_page_range_dirty_lock(), which marks each folio dirty once
and drops its pins in one go, instead of a set_page_dirty_lock() and
unpin_user_page() per page. It removes the last use of
set_page_dirty_lock() in vhost.

---
Tal Zussman (2):
      vhost-vdpa: fix pinned page leak in vhost_vdpa_pa_map() error path
      vhost-vdpa: unpin mappings by range

 drivers/vhost/vdpa.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)
---
base-commit: c69023b5e16e5ffe9368603dca76d85bd544ee57
change-id: 20261015-vhost-vdpa-unpin-68ee785a019a

Best regards,
--  
Tal Zussman <tz2294@columbia.edu>

[PATCH 1/2] vhost-vdpa: fix pinned page leak in vhost_vdpa_pa_map() error path

[Tal Zussman]

When pin_user_pages() or vhost_vdpa_map() fails, vhost_vdpa_pa_map()
is supposed to unpin the pages pinned after the last successfully
mapped chunk, i.e. the range [map_pfn, last_pfn]. The unpin is guarded
by nchunks, but nchunks is reset to 0 every time a chunk is mapped from
inside the current page list, so a failure after any such boundary
leaves that pending range pinned forever. With physically scattered
memory nearly every page list contains a boundary, so the leak happens
on practically every failed IOTLB update.

The pending range is non-empty exactly when a page has been pinned,
which last_pfn already tells us, so drop nchunks and test last_pfn.

Fixes: ad89653f79f1 ("vhost-vdpa: fix page pinning leakage in error path (rework)")
Cc: stable@vger.kernel.org
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 drivers/vhost/vdpa.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index ff7d90719171..d9af728942b3 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -1151,7 +1151,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 	unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
 	unsigned int gup_flags = FOLL_LONGTERM;
 	unsigned long npages, cur_base, map_pfn, last_pfn = 0;
-	unsigned long lock_limit, sz2pin, nchunks, i;
+	unsigned long lock_limit, sz2pin, i;
 	unsigned long page_offset;
 	u64 start = iova;
 	long pinned;
@@ -1187,7 +1187,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 
 	cur_base = uaddr & PAGE_MASK;
 	iova &= PAGE_MASK;
-	nchunks = 0;
 
 	while (npages) {
 		sz2pin = min_t(unsigned long, npages, list_size);
@@ -1202,7 +1201,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 			}
 			goto out;
 		}
-		nchunks++;
 
 		if (!last_pfn)
 			map_pfn = page_to_pfn(page_list[0]);
@@ -1233,7 +1231,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 
 				map_pfn = this_pfn;
 				iova += csize;
-				nchunks = 0;
 			}
 
 			last_pfn = this_pfn;
@@ -1248,7 +1245,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 			     PFN_PHYS(map_pfn), perm, NULL);
 out:
 	if (ret) {
-		if (nchunks) {
+		if (last_pfn) {
 			unsigned long pfn;
 
 			/*
@@ -1260,7 +1257,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 			 * the corresponding unpinning will be handled by
 			 * vdpa_unmap().
 			 */
-			WARN_ON(!last_pfn);
 			for (pfn = map_pfn; pfn <= last_pfn; pfn++)
 				unpin_user_page(pfn_to_page(pfn));
 		}

-- 
2.39.5

Re: [PATCH 1/2] vhost-vdpa: fix pinned page leak in vhost_vdpa_pa_map() error path

[Si-Wei Liu]

Good catch! Thanks for the fix, it looks good to me.

On 10/31/2026 10:45 AM, Tal Zussman wrote:
> When pin_user_pages() or vhost_vdpa_map() fails, vhost_vdpa_pa_map()
> is supposed to unpin the pages pinned after the last successfully
> mapped chunk, i.e. the range [map_pfn, last_pfn]. The unpin is guarded
> by nchunks, but nchunks is reset to 0 every time a chunk is mapped from
> inside the current page list, so a failure after any such boundary
> leaves that pending range pinned forever. With physically scattered
> memory nearly every page list contains a boundary, so the leak happens
> on practically every failed IOTLB update.
>
> The pending range is non-empty exactly when a page has been pinned,
> which last_pfn already tells us, so drop nchunks and test last_pfn.
>
> Fixes: ad89653f79f1 ("vhost-vdpa: fix page pinning leakage in error path (rework)")
> Cc: stable@vger.kernel.org
Acked-by: Si-Wei Liu <si-wei.liu@oracle.com>

> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>   drivers/vhost/vdpa.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index ff7d90719171..d9af728942b3 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -1151,7 +1151,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
>   	unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
>   	unsigned int gup_flags = FOLL_LONGTERM;
>   	unsigned long npages, cur_base, map_pfn, last_pfn = 0;
> -	unsigned long lock_limit, sz2pin, nchunks, i;
> +	unsigned long lock_limit, sz2pin, i;
>   	unsigned long page_offset;
>   	u64 start = iova;
>   	long pinned;
> @@ -1187,7 +1187,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
>   
>   	cur_base = uaddr & PAGE_MASK;
>   	iova &= PAGE_MASK;
> -	nchunks = 0;
>   
>   	while (npages) {
>   		sz2pin = min_t(unsigned long, npages, list_size);
> @@ -1202,7 +1201,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
>   			}
>   			goto out;
>   		}
> -		nchunks++;
>   
>   		if (!last_pfn)
>   			map_pfn = page_to_pfn(page_list[0]);
> @@ -1233,7 +1231,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
>   
>   				map_pfn = this_pfn;
>   				iova += csize;
> -				nchunks = 0;
>   			}
>   
>   			last_pfn = this_pfn;
> @@ -1248,7 +1245,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
>   			     PFN_PHYS(map_pfn), perm, NULL);
>   out:
>   	if (ret) {
> -		if (nchunks) {
> +		if (last_pfn) {
>   			unsigned long pfn;
>   
>   			/*
> @@ -1260,7 +1257,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
>   			 * the corresponding unpinning will be handled by
>   			 * vdpa_unmap().
>   			 */
> -			WARN_ON(!last_pfn);
>   			for (pfn = map_pfn; pfn <= last_pfn; pfn++)
>   				unpin_user_page(pfn_to_page(pfn));
>   		}
>
> -- 
> 2.39.5

[PATCH 2/2] vhost-vdpa: unpin mappings by range

[Tal Zussman]

vhost_vdpa_pa_unmap() unpins each mapping a page at a time, dirtying
each page first if the mapping is writable. Each mapping is physically
contiguous, so use unpin_user_page_range_dirty_lock() instead, which
marks every folio dirty once and drops its pins in one go. Do the same
for the pages left pinned in the vhost_vdpa_pa_map() error path.

This removes the last use of set_page_dirty_lock() in vhost.

Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 drivers/vhost/vdpa.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index d9af728942b3..cd5da522fdc0 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -972,18 +972,11 @@ static void vhost_vdpa_pa_unmap(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
 {
 	struct vhost_dev *dev = &v->vdev;
 	struct vhost_iotlb_map *map;
-	struct page *page;
-	unsigned long pfn, pinned;
 
 	while ((map = vhost_iotlb_itree_first(iotlb, start, last)) != NULL) {
-		pinned = PFN_DOWN(map->size);
-		for (pfn = PFN_DOWN(map->addr);
-		     pinned > 0; pfn++, pinned--) {
-			page = pfn_to_page(pfn);
-			if (map->perm & VHOST_ACCESS_WO)
-				set_page_dirty_lock(page);
-			unpin_user_page(page);
-		}
+		unpin_user_page_range_dirty_lock(pfn_to_page(PFN_DOWN(map->addr)),
+						 PFN_DOWN(map->size),
+						 map->perm & VHOST_ACCESS_WO);
 		atomic64_sub(PFN_DOWN(map->size), &dev->mm->pinned_vm);
 		vhost_vdpa_general_unmap(v, map, asid);
 		vhost_iotlb_map_free(iotlb, map);
@@ -1246,8 +1239,6 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 out:
 	if (ret) {
 		if (last_pfn) {
-			unsigned long pfn;
-
 			/*
 			 * Unpin the outstanding pages which are yet to be
 			 * mapped but haven't due to vdpa_map() or
@@ -1257,8 +1248,9 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 			 * the corresponding unpinning will be handled by
 			 * vdpa_unmap().
 			 */
-			for (pfn = map_pfn; pfn <= last_pfn; pfn++)
-				unpin_user_page(pfn_to_page(pfn));
+			unpin_user_page_range_dirty_lock(pfn_to_page(map_pfn),
+							 last_pfn - map_pfn + 1,
+							 false);
 		}
 		vhost_vdpa_unmap(v, iotlb, start, size);
 	}

-- 
2.39.5