Re: [PATCH 05/14] block: add helpers to bounce buffer an iov_iter into bios

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Jan 23, 2026 at 06:51:28AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 22, 2026 at 09:25:56AM -0800, Darrick J. Wong wrote:
> > Hrm.  Should we combine this with the slightly different version that is
> > in xfs_healthmon?
> 
> Yes, but not now.  I'd rather not introduce a three-way cross tree
> dependency with bike shedding potential right now.  Let's look at this
> once we have the two versions in tree, and also look out for others.

<nod>

> > > +static void bio_free_folios(struct bio *bio)
> > > +{
> > > +	struct bio_vec *bv;
> > > +	int i;
> > > +
> > > +	bio_for_each_bvec_all(bv, bio, i) {
> > > +		struct folio *folio = page_folio(bv->bv_page);
> > > +
> > > +		if (!is_zero_folio(folio))
> > > +			folio_put(page_folio(bv->bv_page));
> > 
> > Isn't folio_put's argument just @folio again?
> 
> Yes, I'll clean this up.
> 
> > > +		if (this_len > PAGE_SIZE * 2)
> > > +			this_len = rounddown_pow_of_two(this_len);
> > > +
> > > +		if (bio->bi_iter.bi_size > UINT_MAX - this_len)
> > 
> > Now that I've seen UINT_MAX appear twice in terms of limiting bio size,
> > I wonder if that ought to be encoded as a constant somewhere?
> > 
> > #define BIO_ITER_MAX_SIZE	(UINT_MAX)
> > 
> > (apologies if I'm digging up some horrible old flamewar from the 1830s)
> 
> Heh.  I don't remember any flame wars, but maybe that's just because my
> memory sucks.

Well it's not like I'm highly incentivized to remember misinteractions
on the mailing lists... :D

>  I guess this would be more like:
> 
> define BVEC_ITER_MAX_SIZE	sizeof_field(struct bvec_iter, bi_size)
> 
> though.

Hrmm, that might be better.

> > > +	} while (len && bio->bi_vcnt < bio->bi_max_vecs - 1);
> > > +
> > > +	/*
> > > +	 * Set the folio directly here.  The above loop has already calculated
> > > +	 * the correct bi_size, and we use bi_vcnt for the user buffers.  That
> > > +	 * is safe as bi_vcnt is only for user by the submitter and not looked
> > 
> > "...for use by the submitter..." ?
> 
> Yes.
> 
> > > +	if (likely(!is_error)) {
> > > +		void *buf = bvec_virt(&bio->bi_io_vec[0]);
> > > +		struct iov_iter to;
> > > +
> > > +		iov_iter_bvec(&to, ITER_DEST, bio->bi_io_vec + 1, bio->bi_vcnt,
> > > +				len);
> > > +		WARN_ON_ONCE(copy_to_iter(buf, len, &to) != len);
> > 
> > I wonder, under what circumstances would the copy_to_iter come up short?
> > 
> > Something evil like $program initiates a directio read from a PI disk, a
> > BPF guy starts screaming in a datacenter to wobble the disk, and that
> > gives a compromised systemd enough time to attach to $program with
> > ptrace to unmap a page in the middle of the read buffer before
> > bio_iov_iter_unbounce_read gets called?
> 
> I don't think it can at all.   Remember, this is not directly copying
> to the user iter, but to the bvec array pointing to pinned pages,
> which are not going away. 

Ah, right.  How about adding a comment so that future me doesn't trip on
this again?

	/* copying to pinned pages should always work */
	WARN_ON_ONCE(copy_to_iter(...));

--D