Re: [block.git conflicts] Re: [PATCH 37/44] block: convert to advancing variants of iov_iter_get_pages{,_alloc}()

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Jul 01, 2022 at 01:56:53PM -0600, Keith Busch wrote:

> Validating user requests gets really messy if we allow arbitrary segment
> lengths. This particular patch just enables arbitrary address alignment, but
> segment size is still required to be a block size. You found the commit that
> enforces that earlier, "iov: introduce iov_iter_aligned", two commits prior.

BTW, where do you check it for this caller?
	fs/zonefs/super.c:786:  ret = bio_iov_iter_get_pages(bio, from);
Incidentally, we have an incorrect use of iov_iter_truncate() in that one (compare
with iomap case, where we reexpand it afterwards)...

I still don't get the logics of those round-downs.  You've *already* verified
that each segment is a multiple of logical block size.  And you are stuffing
as much as you can into bio, covering the data for as many segments as you
can.  Sure, you might end up e.g. running into an unmapped page at wrong
offset (since your requirements for initial offsets might be milder than
logical block size).  Or you might run out of pages bio would take.  Either
might put the end of bio at the wrong offset.

So why not trim it down *after* you are done adding pages into it?  And do it
once, outside of the loop.  IDGI...  Validation is already done; I'm not
suggesting to allow weird segment lengths or to change behaviour of your
iov_iter_is_aligned() in any other way.

Put it another way, is there any possibility for __bio_iov_iter_get_pages() to
do a non-trivial round-down on anything other than the last iteration of that
loop?