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

* Re: + seq_read-info-message-about-buggy-next-functions.patch added to -mm tree
       [not found] <20200226035621.4NlNn738T%akpm@linux-foundation.org>
@ 2020-03-02 18:20 ` Qian Cai
  2020-03-02 19:31   ` Qian Cai
  0 siblings, 1 reply; 4+ messages in thread
From: Qian Cai @ 2020-03-02 18:20 UTC (permalink / raw)
  To: linux-kernel, dave, longman, manfred, mingo, mm-commits, neilb,
	oberpar, rostedt, viro, vvs
  Cc: akpm

On Tue, 2020-02-25 at 19:56 -0800, Andrew Morton wrote:
> ------------------------------------------------------
> From: Vasily Averin <vvs@virtuozzo.com>
> 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@virtuozzo.com
> Link: http://lkml.kernel.org/r/7c24087c-e280-e580-5b0c-0cdaeb14cd18@virtuozzo.com
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> Cc: NeilBrown <neilb@suse.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Manfred Spraul <manfred@colorfullife.com>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Cc: Waiman Long <longman@redhat.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  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@virtuozzo.com 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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread