seq_file: fix mishandling of consecutive pread() invocations.
authorEarl Chew <echew@ixiacom.com>
Wed, 21 Mar 2012 23:33:43 +0000 (16:33 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 22 Mar 2012 00:54:54 +0000 (17:54 -0700)
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 <echew@ixiacom.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/seq_file.c

index 4023d6be939bc8521e9afca4094ad96ef8cf3c9f..aa242dc99373bec0fe981d22508921454759bd14 100644 (file)
@@ -140,9 +140,21 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
 
        mutex_lock(&m->lock);
 
+       /*
+        * 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
+        * pass the file->f_version to those methods.
+        *
+        * seq_file->version is just copy of f_version, and seq_file
+        * methods can treat it simply as file version.
+        * It is copied in first and copied out after all operations.
+        * It is convenient to have it as  part of structure to avoid the
+        * 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)) {
-               m->read_pos = *ppos;
                while ((err = traverse(m, *ppos)) == -EAGAIN)
                        ;
                if (err) {
@@ -152,21 +164,11 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
                        m->index = 0;
                        m->count = 0;
                        goto Done;
+               } else {
+                       m->read_pos = *ppos;
                }
        }
 
-       /*
-        * 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
-        * pass the file->f_version to those methods.
-        *
-        * seq_file->version is just copy of f_version, and seq_file
-        * methods can treat it simply as file version.
-        * It is copied in first and copied out after all operations.
-        * It is convenient to have it as  part of structure to avoid the
-        * need of passing another argument to all the seq_file methods.
-        */
-       m->version = file->f_version;
        /* grab buffer if we didn't have one */
        if (!m->buf) {
                m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);