Re: [PATCH 1/2] iomap: Fix pipe page leakage during splicing

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Nov 21, 2019 at 05:15:34PM +0100, Jan Kara wrote:
> When splicing using iomap_dio_rw() to a pipe, we may leak pipe pages
> because bio_iov_iter_get_pages() records that the pipe will have full
> extent worth of data however if file size is not block size aligned
> iomap_dio_rw() returns less than what bio_iov_iter_get_pages() set up
> and splice code gets confused leaking a pipe page with the file tail.
> 
> Handle the situation similarly to the old direct IO implementation and
> revert iter to actually returned read amount which makes iter consistent
> with value returned from iomap_dio_rw() and thus the splice code is
> happy.
> 
> Fixes: ff6a9292e6f6 ("iomap: implement direct I/O")
> CC: stable@vger.kernel.org
> Reported-by: syzbot+991400e8eba7e00a26e1@syzkaller.appspotmail.com
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/iomap/direct-io.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 1fc28c2da279..30189652c560 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -497,8 +497,15 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		}
>  		pos += ret;
>  
> -		if (iov_iter_rw(iter) == READ && pos >= dio->i_size)
> +		if (iov_iter_rw(iter) == READ && pos >= dio->i_size) {
> +			/*
> +			 * We will report we've read data only upto i_size.

Nit: "up to"; will fix that on the way in.

> +			 * Revert iter to a state corresponding to that as
> +			 * some callers (such as splice code) rely on it.
> +			 */
> +			iov_iter_revert(iter, pos - dio->i_size);

Just to make sure I'm getting this right, iov_iter_revert walks the
iterator variables backwards through pipe buffers/bvec/iovec, which has
the effect of undoing whatever iterator walking we've just done.

In contrast, iov_iter_reexpand undoes a previous subtraction to
iov->count which was (presumably) done via iov_iter_truncate.

Or to put it another way, _revert walks the iteration pointer backwards,
whereas _truncate/_reexpand modify where the iteration ends.  Right?

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  			break;
> +		}
>  	} while ((count = iov_iter_count(iter)) > 0);
>  	blk_finish_plug(&plug);
>  
> -- 
> 2.16.4
>