[PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[Sungwoo Kim]

pin_user_pages_fast() can partially succeed and return the number of
pages that were actually pinned. However, the bio_integrity_map_user()
does not handle this partial pinning. This leads to a general protection
fault since bvec_from_pages() dereferences an unpinned page address,
which is 0.

To fix this, add a check to verify that all requested memory is pinned.

KASAN splat:

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
RIP: 0010:_compound_head home/wukong/fuzznvme/linux/./include/linux/page-flags.h:240 [inline]
RIP: 0010:bvec_from_pages home/wukong/fuzznvme/linux/block/bio-integrity.c:290 [inline]

Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
Acked-by: Chao Shi <cshi008@fiu.edu>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Acked-by: Dave Tian <daveti@purdue.edu>
Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
V1: https://lore.kernel.org/linux-block/20260308001358.1675543-2-iam@sung-woo.kim/T/#u
V1->V2:
- v1 incorrectly assumed pin_user_pages_fast() returns bytes. Fixed.

 block/bio-integrity.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 20f5d301d32d..992ce39e8ab9 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -338,6 +338,15 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 					extraction_flags, &offset);
 	if (unlikely(ret < 0))
 		goto free_bvec;
+	if (unlikely(ret != nr_vecs)) {
+		for (int i = 0; i < ret; i++)
+			unpin_user_page(pages[i]);
+
+		if (pages != stack_pages)
+			kvfree(pages);
+		ret = -EFAULT;
+		goto free_bvec;
+	}
 
 	nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset,
 				   &is_p2p);
-- 
2.47.3

Re: [PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[Christoph Hellwig]

On Mon, Mar 30, 2026 at 07:02:56PM -0400, Sungwoo Kim wrote:
> pin_user_pages_fast() can partially succeed and return the number of
> pages that were actually pinned. However, the bio_integrity_map_user()
> does not handle this partial pinning. This leads to a general protection
> fault since bvec_from_pages() dereferences an unpinned page address,
> which is 0.

Can you share the reproducer, or even better wire it up to blktests?

> 
> To fix this, add a check to verify that all requested memory is pinned.
> 
> KASAN splat:
> 
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> RIP: 0010:_compound_head home/wukong/fuzznvme/linux/./include/linux/page-flags.h:240 [inline]
> RIP: 0010:bvec_from_pages home/wukong/fuzznvme/linux/block/bio-integrity.c:290 [inline]
> 
> Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
> Acked-by: Chao Shi <cshi008@fiu.edu>
> Acked-by: Weidong Zhu <weizhu@fiu.edu>
> Acked-by: Dave Tian <daveti@purdue.edu>
> Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> ---
> V1: https://lore.kernel.org/linux-block/20260308001358.1675543-2-iam@sung-woo.kim/T/#u
> V1->V2:
> - v1 incorrectly assumed pin_user_pages_fast() returns bytes. Fixed.
> 
>  block/bio-integrity.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 20f5d301d32d..992ce39e8ab9 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -338,6 +338,15 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>  					extraction_flags, &offset);
>  	if (unlikely(ret < 0))
>  		goto free_bvec;
> +	if (unlikely(ret != nr_vecs)) {
> +		for (int i = 0; i < ret; i++)
> +			unpin_user_page(pages[i]);

I guess this works fine even for a negative ret, but it looks really
odd.

> +		if (pages != stack_pages)
> +			kvfree(pages);
> +		ret = -EFAULT;
> +		goto free_bvec;

This now loses the original return value if it alredy was
negative.

I think the better fix here would be to switch to
iov_iter_extract_bvecs, but that might be a bit too big for
a backportable bugfix, so I guess we should merge your patch
first once it is fixed up.

Re: [PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[Sungwoo Kim]

On Tue, Mar 31, 2026 at 4:16 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Mar 30, 2026 at 07:02:56PM -0400, Sungwoo Kim wrote:
> > pin_user_pages_fast() can partially succeed and return the number of
> > pages that were actually pinned. However, the bio_integrity_map_user()
> > does not handle this partial pinning. This leads to a general protection
> > fault since bvec_from_pages() dereferences an unpinned page address,
> > which is 0.
>
> Can you share the reproducer, or even better wire it up to blktests?

Thank you for letting me know about blktests. I'm learning this and
finding a way to reproduce this bug.
If blktests is not suitable for reproducing this - it might be too
early to think about this though - I have another idea to reproduce
this.
I can modify the pin_user_pages_fast() source to make it always
partial success, so there are always some unpinned pages, which is a
precondition of this bug.
I'd like to ask for comments on this if it doesn't make sense.

>
> >
> > To fix this, add a check to verify that all requested memory is pinned.
> >
> > KASAN splat:
> >
> > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
> > KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> > RIP: 0010:_compound_head home/wukong/fuzznvme/linux/./include/linux/page-flags.h:240 [inline]
> > RIP: 0010:bvec_from_pages home/wukong/fuzznvme/linux/block/bio-integrity.c:290 [inline]
> >
> > Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
> > Acked-by: Chao Shi <cshi008@fiu.edu>
> > Acked-by: Weidong Zhu <weizhu@fiu.edu>
> > Acked-by: Dave Tian <daveti@purdue.edu>
> > Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> > ---
> > V1: https://lore.kernel.org/linux-block/20260308001358.1675543-2-iam@sung-woo.kim/T/#u
> > V1->V2:
> > - v1 incorrectly assumed pin_user_pages_fast() returns bytes. Fixed.
> >
> >  block/bio-integrity.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> > index 20f5d301d32d..992ce39e8ab9 100644
> > --- a/block/bio-integrity.c
> > +++ b/block/bio-integrity.c
> > @@ -338,6 +338,15 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
> >                                       extraction_flags, &offset);
> >       if (unlikely(ret < 0))
> >               goto free_bvec;
> > +     if (unlikely(ret != nr_vecs)) {
> > +             for (int i = 0; i < ret; i++)
> > +                     unpin_user_page(pages[i]);
>
> I guess this works fine even for a negative ret, but it looks really
> odd.
>
> > +             if (pages != stack_pages)
> > +                     kvfree(pages);
> > +             ret = -EFAULT;
> > +             goto free_bvec;
>
> This now loses the original return value if it alredy was
> negative.
>
> I think the better fix here would be to switch to
> iov_iter_extract_bvecs, but that might be a bit too big for
> a backportable bugfix, so I guess we should merge your patch
> first once it is fixed up.
>

Okay. I'll find a reproducer first. And then let's discuss about a fix.

Thanks again for your review.
Sungwoo.

Re: [PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[Christoph Hellwig]

On Fri, Apr 03, 2026 at 03:03:00AM +0900, Sungwoo Kim wrote:
> Thank you for letting me know about blktests. I'm learning this and
> finding a way to reproduce this bug.
> If blktests is not suitable for reproducing this - it might be too
> early to think about this though - I have another idea to reproduce
> this.
> I can modify the pin_user_pages_fast() source to make it always
> partial success, so there are always some unpinned pages, which is a
> precondition of this bug.
> I'd like to ask for comments on this if it doesn't make sense.

Using the failure injection framework to reproduce this sounds like a
good idea.

Re: [PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[Kanchan Joshi]

On 3/31/2026 4:32 AM, Sungwoo Kim wrote:
> pin_user_pages_fast() can partially succeed and return the number of
> pages that were actually pinned. However, the bio_integrity_map_user()
> does not handle this partial pinning. This leads to a general protection
> fault since bvec_from_pages() dereferences an unpinned page address,
> which is 0.
> 
> To fix this, add a check to verify that all requested memory is pinned.
> 
> KASAN splat:
> 
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> RIP: 0010:_compound_head home/wukong/fuzznvme/linux/./include/linux/page-flags.h:240 [inline]
> RIP: 0010:bvec_from_pages home/wukong/fuzznvme/linux/block/bio-integrity.c:290 [inline]
> 
> Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
> Acked-by: Chao Shi<cshi008@fiu.edu>
> Acked-by: Weidong Zhu<weizhu@fiu.edu>
> Acked-by: Dave Tian<daveti@purdue.edu>
> Signed-off-by: Sungwoo Kim<iam@sung-woo.kim>
> ---
> V1:https://lore.kernel.org/linux-block/20260308001358.1675543-2-iam@sung- 
> woo.kim/T/#u
> V1->V2:
> - v1 incorrectly assumed pin_user_pages_fast() returns bytes. Fixed.

But this function does not call pin_user_pages_fast(). It calls 
iov_iter_extract_pages() which returns in bytes. So v1 maybe better than 
this patch?

> 
>   block/bio-integrity.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 20f5d301d32d..992ce39e8ab9 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -338,6 +338,15 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
>   					extraction_flags, &offset);
>   	if (unlikely(ret < 0))
>   		goto free_bvec;
> +	if (unlikely(ret != nr_vecs)) {

ret is in bytes and nr_vecs is in pages. Almost always we will go inside 
and throw failures for perfectly valid case. No?

> +		for (int i = 0; i < ret; i++)
> +			unpin_user_page(pages[i]);

And out-of-bounds access here.

> +
> +		if (pages != stack_pages)
> +			kvfree(pages);
> +		ret = -EFAULT;

Re: [PATCH v2] block: Fix general protection fault in bio_integrity_map_user()

[Sungwoo Kim]

[snip]
> > ---
> > V1:https://lore.kernel.org/linux-block/20260308001358.1675543-2-iam@sung-
> > woo.kim/T/#u
> > V1->V2:
> > - v1 incorrectly assumed pin_user_pages_fast() returns bytes. Fixed.
>
> But this function does not call pin_user_pages_fast(). It calls
> iov_iter_extract_pages() which returns in bytes. So v1 maybe better than
> this patch?

Thank you for your review.  If iov_iter_extract_pages() returns bytes,
this patch is completely wrong.

> >
> >   block/bio-integrity.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> > index 20f5d301d32d..992ce39e8ab9 100644
> > --- a/block/bio-integrity.c
> > +++ b/block/bio-integrity.c
> > @@ -338,6 +338,15 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
> >                                       extraction_flags, &offset);
> >       if (unlikely(ret < 0))
> >               goto free_bvec;
> > +     if (unlikely(ret != nr_vecs)) {
>
> ret is in bytes and nr_vecs is in pages. Almost always we will go inside
> and throw failures for perfectly valid case. No?
>

Right.

> > +             for (int i = 0; i < ret; i++)
> > +                     unpin_user_page(pages[i]);
>
> And out-of-bounds access here.

I assume blktests can catch this. So, it might be a good idea to
confirm the patch with blktests.
I will do this in v3.

Thank you again for your review.
Sungwoo.

>
> > +
> > +             if (pages != stack_pages)
> > +                     kvfree(pages);
> > +             ret = -EFAULT;
>
>