pstore: pstore_ftrace_seq_next should increase position index
authorVasily Averin <vvs@virtuozzo.com>
Tue, 25 Feb 2020 08:11:20 +0000 (11:11 +0300)
committerKees Cook <keescook@chromium.org>
Thu, 27 Feb 2020 16:04:59 +0000 (08:04 -0800)
In Aug 2018 NeilBrown noticed
commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
"Some ->next functions do not increment *pos when they return NULL...
Note that such ->next functions are buggy and should be fixed.
A simple demonstration is

 dd if=/proc/swaps bs=1000 skip=1

Choose any block size larger than the size of /proc/swaps. This will
always show the whole last line of /proc/swaps"

/proc/swaps output was fixed recently, however there are lot of other
affected files, and one of them is related to pstore subsystem.

If .next function does not change position index, following .show function
will repeat output related to current position index.

There are at least 2 related problems:
- read after lseek beyond end of file, described above by NeilBrown
  "dd if=<AFFECTED_FILE> bs=1000 skip=1" will generate whole last list
- read after lseek on in middle of last line will output expected rest of
  last line but then repeat whole last line once again.

If .show() function generates multy-line output (like
pstore_ftrace_seq_show() does ?) following bash script cycles endlessly

 $ q=;while read -r r;do echo "$((++q)) $r";done < AFFECTED_FILE

Unfortunately I'm not familiar enough to pstore subsystem and was unable
to find affected pstore-related file on my test node.

If .next function does not change position index, following .show function
will repeat output related to current position index.

Cc: stable@vger.kernel.org
Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code ...")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206283
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Link: https://lore.kernel.org/r/4e49830d-4c88-0171-ee24-1ee540028dad@virtuozzo.com
[kees: with robustness tweak from Joel Fernandes <joelaf@google.com>]
Signed-off-by: Kees Cook <keescook@chromium.org>
fs/pstore/inode.c

index 7fbe8f0582205a3cdb31514d27cd94708c9b8e82..d99b5d39aa90e04a8822196973bda309018c902e 100644 (file)
@@ -87,11 +87,11 @@ static void *pstore_ftrace_seq_next(struct seq_file *s, void *v, loff_t *pos)
        struct pstore_private *ps = s->private;
        struct pstore_ftrace_seq_data *data = v;
 
+       (*pos)++;
        data->off += REC_SIZE;
        if (data->off + REC_SIZE > ps->total_size)
                return NULL;
 
-       (*pos)++;
        return data;
 }
 
@@ -101,6 +101,9 @@ static int pstore_ftrace_seq_show(struct seq_file *s, void *v)
        struct pstore_ftrace_seq_data *data = v;
        struct pstore_ftrace_record *rec;
 
+       if (!data)
+               return 0;
+
        rec = (struct pstore_ftrace_record *)(ps->record->buf + data->off);
 
        seq_printf(s, "CPU:%d ts:%llu %08lx  %08lx  %ps <- %pS\n",