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@vger.kernel.org Suggested-by: Fedor Pchelkin Signed-off-by: Dmitry Antipov --- 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