On Mon, Jun 13, 2022 at 11:28:34PM +0100, Al Viro wrote: > On Mon, Jun 13, 2022 at 10:54:36AM -0700, Linus Torvalds wrote: > > On Sun, Jun 12, 2022 at 5:10 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > > > > Unlike other copying operations on ITER_PIPE, copy_mc_to_iter() can > > > result in a short copy. In that case we need to trim the unused > > > buffers, as well as the length of partially filled one - it's not > > > enough to set ->head, ->iov_offset and ->count to reflect how > > > much had we copied. Not hard to fix, fortunately... > > > > > > I'd put a helper (pipe_discard_from(pipe, head)) into pipe_fs_i.h, > > > rather than iov_iter.c - > > > > Actually, since this "copy_mc_xyz()" stuff is going to be entirely > > impossible to debug and replicate for any normal situation, I would > > suggest we take the approach that we (long ago) used to take with > > copy_from_user(): zero out the destination buffer, so that developers > > that can't test the faulting behavior don't have to worry about it. > > > > And then the existing code is fine: it will break out of the loop, but > > it won't do the odd revert games and the "randomnoise.len -= rem" > > thing that I can't wrap my head around. > > > > Hmm? > > Not really - we would need to zero the rest of those pages somehow. > They are already allocated and linked into pipe; leaving them > there (and subsequent ones hadn't seen any stores whatsoever - they > are fresh out of alloc_page(GFP_USER)) is a non-starter. > > We could do allocation as we go, but that's a much more intrusive > change... FWIW, I've got quite a bit of cleanups in the local tree; reordering and cleaning that queue up at the moment, will post tonight or tomorrow. I've looked into doing allocations page-by-page (instead of single push_pipe(), followed by copying into those). Doable, but it ends up being much messier. IMO this "truncate on failure" approach is saner.
