Re: + seq_read-info-message-about-buggy-next-functions.patch added to -mm tree

From: Qian Cai
Date: Mon Mar 02 2020 - 13:21:06 EST


On Tue, 2020-02-25 at 19:56 -0800, Andrew Morton wrote:
> ------------------------------------------------------
> From: Vasily Averin <vvs@xxxxxxxxxxxxx>
> Subject: fs/seq_file.c: seq_read(): add info message about buggy .next functions
>
> Patch series "seq_file .next functions should increase position index".
>
> 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"
>
> Described problem is still actual. If you make lseek into middle of last
> output line following read will output end of last line and whole last
> line once again.
>
> $ dd if=/proc/swaps bs=1 # usual output
> Filename Type Size Used Priority
> /dev/dm-0 partition 4194812 97536 -2
> 104+0 records in
> 104+0 records out
> 104 bytes copied
>
> $ dd if=/proc/swaps bs=40 skip=1 # last line was generated twice
> dd: /proc/swaps: cannot skip to specified offset
> v/dm-0 partition 4194812 97536 -2
> /dev/dm-0 partition 4194812 97536 -2
> 3+1 records in
> 3+1 records out
> 131 bytes copied
>
> There are lot of other affected files, I've found 30+ including
> /proc/net/ip_tables_matches and /proc/sysvipc/*
>
> I've sent patches into maillists of affected subsystems already, this
> patch-set fixes the problem in files related to pstore, tracing, gcov,
> sysvipc and other subsystems processed via linux-kernel@ mailing list
> directly
>
> https://bugzilla.kernel.org/show_bug.cgi?id=206283
>
>
> This patch (of 4):
>
> Add debug code to seq_read() to detect missed or out-of-tree incorrect
> .next seq_file functions.

This patch spams the console like crazy while reading sysfs,

# dmesg | grep 'buggy seq_file' | wc -l
4204

[ 9505.321981] LTP: starting read_all_proc (read_all -d /proc -q -r 10)
[ 9508.222934] buggy seq_file .next function xt_match_seq_next [x_tables] did
not updated position index
[ 9508.223319] buggy seq_file .next function xt_match_seq_next [x_tables] did
not updated position index
[ 9508.223654] buggy seq_file .next function xt_match_seq_next [x_tables] did
not updated position index
[ 9508.223994] buggy seq_file .next function xt_match_seq_next [x_tables] did
not updated position index
[ 9508.224337] buggy seq_file .next function xt_match_seq_next [x_tables] did
not updated position index
...


>
> https://bugzilla.kernel.org/show_bug.cgi?id=206283
> Link: http://lkml.kernel.org/r/244674e5-760c-86bd-d08a-047042881748@xxxxxxxxxxxxx
> Link: http://lkml.kernel.org/r/7c24087c-e280-e580-5b0c-0cdaeb14cd18@xxxxxxxxxxxxx
> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx>
> Cc: NeilBrown <neilb@xxxxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>
> Cc: Peter Oberparleiter <oberpar@xxxxxxxxxxxxx>
> Cc: Waiman Long <longman@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> fs/seq_file.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> --- a/fs/seq_file.c~seq_read-info-message-about-buggy-next-functions
> +++ a/fs/seq_file.c
> @@ -256,9 +256,12 @@ Fill:
> loff_t pos = m->index;
>
> p = m->op->next(m, p, &m->index);
> - if (pos == m->index)
> - /* Buggy ->next function */
> + if (pos == m->index) {
> + pr_info("buggy seq_file .next function %ps "
> + "did not updated position index\n",
> + m->op->next);
> m->index++;
> + }
> if (!p || IS_ERR(p)) {
> err = PTR_ERR(p);
> break;
> _
>
> Patches currently in -mm which might be from vvs@xxxxxxxxxxxxx are
>
> seq_read-info-message-about-buggy-next-functions.patch
> pstore_ftrace_seq_next-should-increase-position-index.patch
> gcov_seq_next-should-increase-position-index.patch
> sysvipc_find_ipc-should-increase-position-index.patch
>