Re: [PATCHv2 1/4] block: bio-integrity: directly map user buffers

Keith Busch
2023-10-30T21:25:18Z

On Tue, Oct 31, 2023 at 02:32:48AM +0530, Kanchan Joshi wrote:
> On 10/27/2023 11:49 PM, Keith Busch wrote:
> > +
> > + bip_for_each_vec(bv, bip, iter) {
> > + if (dirty && !PageCompound(bv.bv_page))
> > + set_page_dirty_lock(bv.bv_page);
> > + unpin_user_page(bv.bv_page);
> > + }
> > +}
>
> Leak here, page-unpinning loop will not execute for the common (i.e.,
> no-copy) case...
>
> > + bip = bio_integrity_alloc(bio, GFP_KERNEL, folios);
> > + if (IS_ERR(bip)) {
> > + ret = PTR_ERR(bip);
> > + goto release_pages;
> > + }
> > +
> > + memcpy(bip->bip_vec, bvec, folios * sizeof(*bvec));
>
> Because with this way of copying, bip->bip_iter.bi_size will remain zero.

Good catch.

> Second, is it fine not to have those virt-alignment checks that are done
> by bvec_gap_to_prev() when the pages are added using
> bio_integrity_add_page()?

We're mapping a single user buffer. It's guaranteed to be virtually
congiguous, so no gaps.