Re: Regression in 4.6.0-git - bisected to commit dd254f5a382c

From: Al Viro
Date: Tue May 24 2016 - 12:28:18 EST


On Tue, May 24, 2016 at 11:10:09AM -0500, Larry Finger wrote:

> For now, the following one-line hack allows my system to boot:
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 933b53a..d5d64d9 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -721,6 +721,7 @@ static ssize_t do_loop_readv_writev(struct file *filp,
> struct iov_iter *iter,
> ret += nr;
> if (nr != iovec.iov_len)
> break;
> + nr = max_t(ssize_t, nr, 1);
> iov_iter_advance(iter, nr);
> }
>
> I have no idea what subtle bug in do_loop_readv_writev() is causing nr to be
> zero, but it seems to have been exposed by commit dd254f5a382c.

This is definitely broken. What happens is that something calls readv()
or writev() with one of the iovecs in the middle of the vector having
zero ->iov_len. The call of iov_iter_advance(iter, 0) used to step to
the next iovec in such situation; the change in this commit leaves that to the
next primitive called, and most of the time that works just fine.
do_loop_readv_writev(), though, is picking the size to be passed to
->write()/->read() from the current iovec, and if it had been zero from
the very beginning... guess what's going to happen.

I'm somewhat curious about the source of that syscall - it's a good testcase
to add to ltp/xfstests, but it smells very odd for normal userland code.
It certainly needs to be fixed kernel-side, but the code issuing that is
probably worth looking into. Oddities are like mushrooms - found one, look
around for more and don't assume that the rest will be of the same species...

I'm looking into iterate_and_advance right now; hopefully will have a sane
replacement shortly...