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

[Caleb Sander Mateos]

On Tue, May 12, 2026 at 10:05 AM Dmitry Antipov <dmantipov at yandex.ru> wrote:
>
> 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;

The example is a bit over-complicated, this can just be:
struct page **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)

And these checks could be simplified to just if (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.

Clarify that "error" includes a length of 0 but not a positive length?

Also not sure it's necessary to say "freed if allocated"; from the
caller's perspective, it just looks like it was never allocated.

>   *
>   * 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;

I think a bool would suffice, as pages will only be allocated if the
initial *pages was NULL.

> +       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) {

The mismatch between ret <= 0 here and ret < 0 in
bio_integrity_map_user() would result in a use-after-free in the ret
== 0 case, no? I guess this should be fixed by the "block:
bio-integrity: Fix null-ptr-deref in bio_integrity_map_user()" patch
that landed today.

Best,
Caleb

> +               kvfree(*pages);
> +               *pages = oldpages;
> +       }
> +
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(iov_iter_extract_pages);
>
> --
> 2.54.0
>