folio_add_pins() can fail after memfd_pin_folios() succeeded pinning a folio. Upcoming folio refcount limit checks will make this easier to trigger. Let's fix three problems in the error handling: 1. pfn_reader_release_pins() uses the allocated array size instead of the number of folios returned by memfd_pin_folios(). With large folios, cleanup can unpin uninitialized entries. Remember the number of returned folios and use that during cleanup. 2. pin_memfd_pages() returns on failure without releasing the pins acquired by the current call. Release the per-page pins for folios already expanded into the page array, and the single pins for the remaining folios. 3. pfn_reader_next() returns on failure before updating batch_end_index, even if the batch already contains successfully expanded folios. Update it before propagating the error so cleanup releases those pins as well. Fixes: 92687c793644 ("iommufd: pfn_reader for file mappings") Cc: stable@vger.kernel.org Reviewed-by: Lance Yang Reviewed-by: Jason Gunthorpe Signed-off-by: David Hildenbrand (Arm) --- drivers/iommu/iommufd/pages.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c index 2f6153108add7..fcdbde007a9ab 100644 --- a/drivers/iommu/iommufd/pages.c +++ b/drivers/iommu/iommufd/pages.c @@ -781,6 +781,7 @@ struct pfn_reader_user { struct file *file; struct folio **ufolios; size_t ufolios_len; + unsigned long ufolios_count; unsigned long ufolios_offset; struct folio **ufolios_next; }; @@ -800,6 +801,7 @@ static void pfn_reader_user_init(struct pfn_reader_user *user, user->file = (pages->type == IOPT_ADDRESS_FILE) ? pages->file : NULL; user->ufolios = NULL; user->ufolios_len = 0; + user->ufolios_count = 0; user->ufolios_next = NULL; user->ufolios_offset = 0; } @@ -843,6 +845,7 @@ static long pin_memfd_pages(struct pfn_reader_user *user, unsigned long start, offset >>= PAGE_SHIFT; user->ufolios_next = user->ufolios; + user->ufolios_count = nfolios; user->ufolios_offset = offset; for (i = 0; i < nfolios; i++) { @@ -859,8 +862,12 @@ static long pin_memfd_pages(struct pfn_reader_user *user, unsigned long start, } else { int rc = folio_add_pins(folio, npin - 1); - if (rc) + if (rc) { + unpin_user_pages(user->upages, + upages - user->upages); + unpin_folios(user->ufolios + i, nfolios - i); return rc; + } while (npin--) *upages++ = folio_page(folio, offset++); @@ -1251,14 +1258,14 @@ static int pfn_reader_next(struct pfn_reader *pfns) return -EINVAL; rc = pfn_reader_fill_span(pfns); + pfns->batch_end_index = pfns->batch_start_index + + pfns->batch.total_pfns; if (rc) return rc; if (WARN_ON(!pfns->batch.total_pfns)) return -EINVAL; - pfns->batch_end_index = - pfns->batch_start_index + pfns->batch.total_pfns; if (pfns->batch_end_index == pfns->span.last_used + 1) interval_tree_double_span_iter_next(&pfns->span); @@ -1320,7 +1327,7 @@ static void pfn_reader_release_pins(struct pfn_reader *pfns) if (!user->file) { unpin_user_pages(user->upages + start_index, npages); } else { - long n = user->ufolios_len / sizeof(*user->ufolios); + const unsigned long n = user->ufolios_count; unpin_folios(user->ufolios_next, user->ufolios + n - user->ufolios_next); -- 2.56.0