A slow path to a fast fix

No, I hadn't noticed at the time. The function in question had migrated into that commit on queue reordering - IOW, in my tree it got introduced (as a combination of iov_iter_copy_from_user() and iov_iter_advance()) a couple of months earlier. Early February, at a guess... Queue had been reordered and refactored many times, at some point it got used for pipe conversion to ->write_iter(), where it was obviously the right thing and simplified the logics.

iov_iter_copy_from_user() predates the whole series; it goes back to 2007 and used to live in mm/filemap.c - it's a replacement of filemap_copy_from_user() that doesn't mutilate iovecs; a part of the series by Nick Piggin that had introduced iov_iter in the first place. The whole pile (fault-in, try to use kmap_atomic, etc.) had originated there,
actually - in September 2002, series from Andrew Morton. In 2006 Jens Axboe had done something similar in fs/pipe.c. That's when the bug had been introduced.

iov_iter_copy_from_user() did the same song and dance (try to copy under kmap_atomic, etc.), but it didn't modify iovec *and* didn't advance the iov_iter. The latter was actually a bad idea - that's what copy_page_from_iter() had done differently, and that's what pretty much every caller wanted. However, pipe.c variant *did* modify iovecs, and what's more, it didn't do a self-contained fallback. So it had to be called twice. In a sense, that bug was a result of bad calling conventions...

Combination of iov_iter_copy_from_user() and advancing the iterator was fairly obvious from the look at its callers. What's more, it *did* the fallback, so it looked like it should've simplified the hell out of pipe_writev() - the thing there was obviously a homegrown analogue of that thing, with bloody inconvenient calling conventions.

So there it went. As it turned out, the homegrown analogue (with very obvious intended behaviour) wasn't correct. I really should have done the massage towards the open-coded copy_page_from_iter() - it would've caught the bogosity there and then, *and* it would have avoided compounding another bug in there with an embarrassing bug of my own. In case of failed copy_from_user() in the very beginning EFAULT had been lost - write() ended up returning 0 instead. Eric Biggers had caught that (and a very similar lost error nearby from all way back in 2006) last October...

As an aside, early enough it had become pretty clear that "take an array of iovecs, consume some data, adjust ->iov_{base,len} so that the next time it would point past the data consumed" was a source of recurring bugs waiting to happen. So systematic switch to iterators leaving the iovecs never modified was likely to reduce the likelyhood of that class of bugs. Some got noticed when fixing (e.g. CVE-2014-0069 got caught while doing similar conversion in cifs_iovec_write(); the fix got reordered in front of the queue, leaving the pure conversion to copy_page_from_iter() until the merge in April). Some didn't - bogus code with obvious intent got replaced with something that did what everyone assumed that bogus code to have been doing all along, without noticing a bug concealed from everyone (including the original author) by the convolutions in old variant...

And no, it wasn't a coverup - cifs bugs noticed at the same time got reported as such; I wouldn't have hesitated to do the same to pipe one had I noticed it. Code in cifs_iovec_write() was simpler, so the bogosity there got spotted...