Re: [PATCH v2 6/7] block: unpin all pages of a bvec in bio_iov_iter_align_down()

[Tal Zussman]

On 8/28/26 4:49 PM, Tal Zussman wrote:
> bio_iov_iter_align_down() drops trailing bvecs with unpin_user_page(),
> but a bvec built by iov_iter_extract_bvecs() can span several pages of
> one folio, each with its own pin. All but the first pin leak.
> 
> The partially trimmed bvec has the same problem. Shrinking bv_len does
> not release the pins for the pages cut off by the trim, and
> __bio_release_pages() only unpins the pages bv_len still covers at
> completion.
> 
> Both issues occur only with a logical block size above PAGE_SIZE and a
> large folio backing the user buffer. On a device with a 64K logical
> block size, an O_DIRECT pwritev() from a hugetlb mapping that ends 16K
> past a block boundary leaks one huge page per call, whether the
> remainder is its own bvec or the tail of a larger one.
> 
> Unpin all pages of a dropped bvec with unpin_user_folio(), as
> __bio_release_pages() does, and unpin the pages trimmed off the last
> bvec as well.
> 
> Fixes: 20a0e6276edb ("block: align the bio after building it")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
> ---
>  block/bio.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 898b2f5ef8c8..48fa6b9a6dba 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1196,6 +1196,11 @@ bool bio_iov_iter_set(struct bio *bio, const struct iov_iter *iter)
>  	return true;
>  }
>  
> +static unsigned int bvec_nr_pages(const struct bio_vec *bv)
> +{
> +	return DIV_ROUND_UP(bv->bv_offset + bv->bv_len, PAGE_SIZE);
> +}
> +
>  /*
>   * Aligns the bio size to the len_align_mask, releasing excessive bio vecs that
>   * __bio_iov_iter_get_pages may have inserted, and reverts the trimmed length
> @@ -1205,6 +1210,7 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter,
>  				   struct bio_vec *bv, unsigned len_align_mask)
>  {
>  	size_t nbytes = bio->bi_iter.bi_size & len_align_mask;
> +	unsigned int npages;
>  
>  	if (!nbytes)
>  		return 0;
> @@ -1213,14 +1219,24 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter,
>  	bio->bi_iter.bi_size -= nbytes;
>  	while (nbytes >= bv->bv_len) {
>  		if (bio_flagged(bio, BIO_PAGE_PINNED))
> -			unpin_user_page(bv->bv_page);
> +			unpin_user_folio(bvec_folio(bv),
> +					 bvec_nr_pages(bv));

And right after sending this, I noticed bvec_unpin()...

I think a cleaner solution is to move bvec_unpin() up, make bvec_nr_pages()
handle bv_offset > PAGE_SIZE, use bvec_nr_pages() in bvec_unpin(), and then
use bvec_unpin() here.

>  		if (!--bio->bi_vcnt)
>  			return -EFAULT;
>  		nbytes -= bv->bv_len;
>  		bv--;
>  	}
> +
> +	/*
> +	 * __bio_release_pages() only unpins the pages still covered by
> +	 * bv_len, so drop the pins for the pages trimmed off here.
> +	 */
> +	npages = bvec_nr_pages(bv);
>  	bv->bv_len -= nbytes;
> +	npages -= bvec_nr_pages(bv);
> +	if (npages && bio_flagged(bio, BIO_PAGE_PINNED))
> +		unpin_user_folio(bvec_folio(bv), npages);
>  	return 0;
>  }
>  
>