[lvc-project] [PATCH v2] lib: free pagelist on error in iov_iter_extract_pages()

[Dmitry Antipov]

Users of 'iov_iter_extract_pages()' may provide small, likely
stack-allocated, array of pages by itself and then reject to
use it if it's considered too small. In such a case, passing
NULL pointer means that 'iov_iter_extract_pages()' should
allocate array of pages internally (via 'want_pages_array()').
An overall scenario may be:

...
struct page *stack_pages[SMALL];
struct page **pages = stack_pages;
...
if (not_enough_pages(SMALL))
        pages = NULL;
...
if (iov_iter_extract_pages(..., &pages, ...) <= 0) {
        /* Even in case of error, new array of pages may be allocated */
        if (pages != stack_pages)
                kvfree(pages);                                  [1]
        /* The rest of error handling and return */
}
/* Regular flow */
...
if (pages != stack_pages)
        kvfree(pages);
...

That is, if you're unlucky so SMALL amount of pages wasn't enough and
new array of pages was allocated, missing [1] causes the memory leak.

Currently 'bio_integrity_map_user()' seems the only place where such
a leak looks possible. Older kernels may have more. In particular,
6.12.x has this type of leak in 'bio_map_user_iov()', and it was
found with syzkaller and reproduced experimentally.

So adjust 'iov_iter_extract_pages()' to make cleanup [1] itself rather
than rely on caller's handling on error paths.

Fixes: 7d58fe731028 ("iov_iter: Add a function to extract a page list from an iterator")
Cc: stable at vger.kernel.org
Suggested-by: Fedor Pchelkin <pchelkin at ispras.ru>
Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>
---
v2: fix commit message and issues observed by Sashiko
---
 lib/iov_iter.c | 54 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 243662af1af7..30c5baccc6a9 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1807,7 +1807,8 @@ static ssize_t iov_iter_extract_user_pages(struct iov_iter *i,
  *  (*) Use with ITER_DISCARD is not supported as that has no content.
  *
  * On success, the function sets *@pages to the new pagelist, if allocated, and
- * sets *offset0 to the offset into the first page.
+ * sets *offset0 to the offset into the first page. On error, new pagelist
+ * is freed if was allocated, and *@pages sets back to its original value.
  *
  * It may also return -ENOMEM and -EFAULT.
  */
@@ -1818,31 +1819,42 @@ ssize_t iov_iter_extract_pages(struct iov_iter *i,
 			       iov_iter_extraction_t extraction_flags,
 			       size_t *offset0)
 {
+	struct page **oldpages = *pages;
+	ssize_t ret;
+
 	maxsize = min_t(size_t, min_t(size_t, maxsize, i->count), MAX_RW_COUNT);
 	if (!maxsize)
 		return 0;
 
 	if (likely(user_backed_iter(i)))
-		return iov_iter_extract_user_pages(i, pages, maxsize,
-						   maxpages, extraction_flags,
-						   offset0);
-	if (iov_iter_is_kvec(i))
-		return iov_iter_extract_kvec_pages(i, pages, maxsize,
-						   maxpages, extraction_flags,
-						   offset0);
-	if (iov_iter_is_bvec(i))
-		return iov_iter_extract_bvec_pages(i, pages, maxsize,
-						   maxpages, extraction_flags,
-						   offset0);
-	if (iov_iter_is_folioq(i))
-		return iov_iter_extract_folioq_pages(i, pages, maxsize,
-						     maxpages, extraction_flags,
-						     offset0);
-	if (iov_iter_is_xarray(i))
-		return iov_iter_extract_xarray_pages(i, pages, maxsize,
-						     maxpages, extraction_flags,
-						     offset0);
-	return -EFAULT;
+		ret = iov_iter_extract_user_pages(i, pages, maxsize,
+						  maxpages, extraction_flags,
+						  offset0);
+	else if (iov_iter_is_kvec(i))
+		ret = iov_iter_extract_kvec_pages(i, pages, maxsize,
+						  maxpages, extraction_flags,
+						  offset0);
+	else if (iov_iter_is_bvec(i))
+		ret = iov_iter_extract_bvec_pages(i, pages, maxsize,
+						  maxpages, extraction_flags,
+						  offset0);
+	else if (iov_iter_is_folioq(i))
+		ret = iov_iter_extract_folioq_pages(i, pages, maxsize,
+						    maxpages, extraction_flags,
+						    offset0);
+	else if (iov_iter_is_xarray(i))
+		ret = iov_iter_extract_xarray_pages(i, pages, maxsize,
+						    maxpages, extraction_flags,
+						    offset0);
+	else
+		ret = -EFAULT;
+
+	if (unlikely(ret <= 0) && *pages && *pages != oldpages) {
+		kvfree(*pages);
+		*pages = oldpages;
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(iov_iter_extract_pages);
 
-- 
2.54.0