[PATCH] block: bio-integrity: fix memory leak in bio_integrity_map_user()

[PATCH] block: bio-integrity: fix memory leak in bio_integrity_map_user()

[Dmitry Antipov]

Since 'iov_iter_extract_pages()' may allocate new array of pages
even when it returns non-zero error value, matching cleanup with
'vfree()' should be performed on all return paths afterwards. So
adjust 'bio_integrity_map_user()' to ensure that both 'pages' and
'bvec' arrays are always freed on return.

Fixes: 8582792cf23b ("block: bio-integrity: Fix null-ptr-deref in bio_integrity_map_user()")
Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 block/bio-integrity.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e796de1a749e..53fb04adb09b 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -400,7 +400,7 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 	ret = iov_iter_extract_pages(iter, &pages, bytes, nr_vecs,
 					extraction_flags, &offset);
 	if (unlikely(ret < 0))
-		goto free_bvec;
+		goto out_free;
 
 	/*
 	 * Handle partial pinning. This can happen when pin_user_pages_fast()
@@ -414,16 +414,12 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 			for (i = 0; i < npinned; i++)
 				unpin_user_page(pages[i]);
 		}
-		if (pages != stack_pages)
-			kvfree(pages);
 		ret = -EFAULT;
-		goto free_bvec;
+		goto out_free;
 	}
 
 	nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset,
 				   &is_p2p);
-	if (pages != stack_pages)
-		kvfree(pages);
 	if (nr_bvecs > queue_max_integrity_segments(q))
 		copy = true;
 	if (is_p2p)
@@ -434,15 +430,10 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 	else
 		ret = bio_integrity_init_user(bio, bvec, nr_bvecs, bytes);
 	if (ret)
-		goto release_pages;
-	if (bvec != stack_vec)
-		kfree(bvec);
-
-	return 0;
-
-release_pages:
-	bio_integrity_unpin_bvec(bvec, nr_bvecs);
-free_bvec:
+		bio_integrity_unpin_bvec(bvec, nr_bvecs);
+out_free:
+	if (pages != stack_pages)
+		kvfree(pages);
 	if (bvec != stack_vec)
 		kfree(bvec);
 	return ret;
-- 
2.54.0

Re: [PATCH] block: bio-integrity: fix memory leak in bio_integrity_map_user()

[Caleb Sander Mateos]

On Wed, May 13, 2026 at 12:05 AM Dmitry Antipov <dmantipov@yandex.ru> wrote:
>
> Since 'iov_iter_extract_pages()' may allocate new array of pages
> even when it returns non-zero error value, matching cleanup with
> 'vfree()' should be performed on all return paths afterwards. So
> adjust 'bio_integrity_map_user()' to ensure that both 'pages' and
> 'bvec' arrays are always freed on return.

I preferred your previous patch changing iov_iter_extract_pages() to
free the page array on error. (I left some minor comments on the code
change, but the general approach seems good.) Even though
bio_integrity_map_user() is the only current caller that requests
iov_iter_extract_pages() allocate a page array, others could be added
in the future and forget to do this freeing. And it's conventional for
functions to release any allocated resources on failure. Is there a
reason you decided to modify bio_integrity_map_user() instead?

Best,
Caleb

>
> Fixes: 8582792cf23b ("block: bio-integrity: Fix null-ptr-deref in bio_integrity_map_user()")
> Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  block/bio-integrity.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index e796de1a749e..53fb04adb09b 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -400,7 +400,7 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>         ret = iov_iter_extract_pages(iter, &pages, bytes, nr_vecs,
>                                         extraction_flags, &offset);
>         if (unlikely(ret < 0))
> -               goto free_bvec;
> +               goto out_free;
>
>         /*
>          * Handle partial pinning. This can happen when pin_user_pages_fast()
> @@ -414,16 +414,12 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>                         for (i = 0; i < npinned; i++)
>                                 unpin_user_page(pages[i]);
>                 }
> -               if (pages != stack_pages)
> -                       kvfree(pages);
>                 ret = -EFAULT;
> -               goto free_bvec;
> +               goto out_free;
>         }
>
>         nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset,
>                                    &is_p2p);
> -       if (pages != stack_pages)
> -               kvfree(pages);
>         if (nr_bvecs > queue_max_integrity_segments(q))
>                 copy = true;
>         if (is_p2p)
> @@ -434,15 +430,10 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>         else
>                 ret = bio_integrity_init_user(bio, bvec, nr_bvecs, bytes);
>         if (ret)
> -               goto release_pages;
> -       if (bvec != stack_vec)
> -               kfree(bvec);
> -
> -       return 0;
> -
> -release_pages:
> -       bio_integrity_unpin_bvec(bvec, nr_bvecs);
> -free_bvec:
> +               bio_integrity_unpin_bvec(bvec, nr_bvecs);
> +out_free:
> +       if (pages != stack_pages)
> +               kvfree(pages);
>         if (bvec != stack_vec)
>                 kfree(bvec);
>         return ret;
> --
> 2.54.0
>

Re: [PATCH] block: bio-integrity: fix memory leak in bio_integrity_map_user()

[Dmitry Antipov]

On Wed, 2026-05-13 at 07:32 -0700, Caleb Sander Mateos wrote:

> I preferred your previous patch changing iov_iter_extract_pages() to
> free the page array on error.

Me too.

> Even though bio_integrity_map_user() is the only current caller that
> requests iov_iter_extract_pages() allocate a page array, others could
> be added in the future and forget to do this freeing.

It was in the past. Again, the whole thing was started from 6.12, see
https://lore.kernel.org/stable/20260505094529.406783-1-dmantipov@yandex.ru
(the commit message is somewhat messy BTW), and I'm definitely interested
in supporting that longterm too.

> Is there a reason you decided to modify bio_integrity_map_user() instead?

Sure. Such an ad-hoc quick fixes are much more likely to be integrated
within a reasonable time and effort from the maintainer. OTOH better wait for
Jens' answer before doing anything else.

Dmitry

Re: [PATCH] block: bio-integrity: fix memory leak in bio_integrity_map_user()

[Christoph Hellwig]

On Wed, May 13, 2026 at 10:05:15AM +0300, Dmitry Antipov wrote:
> Since 'iov_iter_extract_pages()' may allocate new array of pages
> even when it returns non-zero error value, matching cleanup with
> 'vfree()' should be performed on all return paths afterwards. So
> adjust 'bio_integrity_map_user()' to ensure that both 'pages' and
> 'bvec' arrays are always freed on return.
> 
> Fixes: 8582792cf23b ("block: bio-integrity: Fix null-ptr-deref in bio_integrity_map_user()")
> Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

The change looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

We'd still be much better off converting it to iov_iter_extract_bvecs
to share common code, and make it more efficient by no needing the
separate pages allocation.

> ---
>  block/bio-integrity.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index e796de1a749e..53fb04adb09b 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -400,7 +400,7 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>  	ret = iov_iter_extract_pages(iter, &pages, bytes, nr_vecs,
>  					extraction_flags, &offset);
>  	if (unlikely(ret < 0))
> -		goto free_bvec;
> +		goto out_free;
>  
>  	/*
>  	 * Handle partial pinning. This can happen when pin_user_pages_fast()
> @@ -414,16 +414,12 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>  			for (i = 0; i < npinned; i++)
>  				unpin_user_page(pages[i]);
>  		}
> -		if (pages != stack_pages)
> -			kvfree(pages);
>  		ret = -EFAULT;
> -		goto free_bvec;
> +		goto out_free;
>  	}
>  
>  	nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset,
>  				   &is_p2p);
> -	if (pages != stack_pages)
> -		kvfree(pages);
>  	if (nr_bvecs > queue_max_integrity_segments(q))
>  		copy = true;
>  	if (is_p2p)
> @@ -434,15 +430,10 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>  	else
>  		ret = bio_integrity_init_user(bio, bvec, nr_bvecs, bytes);
>  	if (ret)
> -		goto release_pages;
> -	if (bvec != stack_vec)
> -		kfree(bvec);
> -
> -	return 0;
> -
> -release_pages:
> -	bio_integrity_unpin_bvec(bvec, nr_bvecs);
> -free_bvec:
> +		bio_integrity_unpin_bvec(bvec, nr_bvecs);
> +out_free:
> +	if (pages != stack_pages)
> +		kvfree(pages);
>  	if (bvec != stack_vec)
>  		kfree(bvec);
>  	return ret;
> -- 
> 2.54.0
---end quoted text---