linux-kernel - [PATCH] fs/seq_file: clear private buffer when resetting iterator

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-next>] [day] [month] [year] [list]

Message-ID: <20171027084607.vwpl4py3hoedb4td@jaguar.localdomain>
Date:   Fri, 27 Oct 2017 01:46:07 -0700
From:   Eduard Sanou <sanou@...teo.net>, Dhole <dhole@...nmailbox.org>
To:     Alexander Viro <viro@...iv.linux.org.uk>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        sanou@...teo.net
Subject: [PATCH] fs/seq_file: clear private buffer when resetting iterator

Commit e522751d605d99a81508e58390a8f51ee96fb662 ("seq_file: reset iterator to
first record for zero offset") introduced a reset to the first iterator object
when reads have zero offset, but this can leave unflushed data from previous
reads in the 'private_data' buffer generating a repeated first object in the
result.  Clear the buffer when resetting the iterator.

If a seq_read() with size = 0 is followed by seq_read()s with size > 0, the
combined result may have the first line repeated twice.  This is observed in
the output of the 'mount' utility (which reads '/proc/self/mountinfo') under
musl libc because 'fgets' is implemented with the 'readv' system call using two
buffers, the first being of length zero.  This currently affects at least
Gentoo Linux with musl and Void Linux with musl.

The following C snippet reproduces the bug:

  #include <sys/uio.h>
  #include <fcntl.h>

  int main() {
          int fd;
          struct iovec iov[2];
          char buf[2][1024];
          iov[0].iov_base = buf[0];
          iov[0].iov_len = 0;
          iov[1].iov_base = buf[1];
          iov[1].iov_len = 1024;

          fd = open("/proc/self/mountinfo", O_RDONLY);
          readv(fd, iov, 2);
          writev(1, iov, 2);
          return 0;
  }

Fixes: e522751d605d ("seq_file: reset iterator to first record for zero offset")
Signed-off-by: Eduard Sanou <sanou@...teo.net>
---
 fs/seq_file.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/seq_file.c b/fs/seq_file.c
index dc7c2be963ed..1344cdd57cb5 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -179,9 +179,15 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 	/*
 	 * if request is to read from zero offset, reset iterator to first
 	 * record as it might have been already advanced by previous requests
-	 */
-	if (*ppos == 0)
+         * If this happens, clear buffer as it may have some remaining data
+         * from previous calls.
+         */
+	if (*ppos == 0) {
 		m->index = 0;
+		m->from = 0;
+		m->count = 0;
+	}
+

 	/* Don't assume *ppos is where we left it */
 	if (unlikely(*ppos != m->read_pos)) {
-- 
2.14.2