On Thu 21-11-19 15:55:28, Darrick J. Wong wrote:
> 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@xxxxxxxxxxxxxxx
> > Reported-by: syzbot+991400e8eba7e00a26e1@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > ---
> > 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?
Correct.
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Thanks!
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
