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