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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 24 May 2016 17:28:12 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Larry Finger <Larry.Finger@...inger.net>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: Regression in 4.6.0-git - bisected to commit dd254f5a382c

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...