From 7904ac84244b59f536c2a5d1066a10f46df07b08 Mon Sep 17 00:00:00 2001 From: Earl Chew Date: Wed, 21 Mar 2012 16:33:43 -0700 Subject: [PATCH] seq_file: fix mishandling of consecutive pread() invocations. The following program illustrates the problem: char buf[8192]; int fd = open("/proc/self/maps", O_RDONLY); n = pread(fd, buf, sizeof(buf), 0); printf("%d\n", n); /* lseek(fd, 0, SEEK_CUR); */ /* Uncomment to work around */ n = pread(fd, buf, sizeof(buf), 0); printf("%d\n", n); The second printf() prints zero, but uncommenting the lseek() corrects its behaviour. To fix, make seq_read() mirror seq_lseek() when processing changes in *ppos. Restore m->version first, then if required traverse and update read_pos on success. Addresses https://bugzilla.kernel.org/show_bug.cgi?id=11856 Signed-off-by: Earl Chew Cc: Alexey Dobriyan Cc: Al Viro Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/seq_file.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/fs/seq_file.c b/fs/seq_file.c index 4023d6be939..aa242dc9937 100644 --- a/fs/seq_file.c +++ b/fs/seq_file.c @@ -140,21 +140,6 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) mutex_lock(&m->lock); - /* Don't assume *ppos is where we left it */ - if (unlikely(*ppos != m->read_pos)) { - m->read_pos = *ppos; - while ((err = traverse(m, *ppos)) == -EAGAIN) - ; - if (err) { - /* With prejudice... */ - m->read_pos = 0; - m->version = 0; - m->index = 0; - m->count = 0; - goto Done; - } - } - /* * seq_file->op->..m_start/m_stop/m_next may do special actions * or optimisations based on the file->f_version, so we want to @@ -167,6 +152,23 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) * need of passing another argument to all the seq_file methods. */ m->version = file->f_version; + + /* Don't assume *ppos is where we left it */ + if (unlikely(*ppos != m->read_pos)) { + while ((err = traverse(m, *ppos)) == -EAGAIN) + ; + if (err) { + /* With prejudice... */ + m->read_pos = 0; + m->version = 0; + m->index = 0; + m->count = 0; + goto Done; + } else { + m->read_pos = *ppos; + } + } + /* grab buffer if we didn't have one */ if (!m->buf) { m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);