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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1583173259.7365.142.camel@lca.pw>
Date:   Mon, 02 Mar 2020 13:20:59 -0500
From:   Qian Cai <cai@....pw>
To:     linux-kernel@...r.kernel.org, dave@...olabs.net,
        longman@...hat.com, manfred@...orfullife.com, mingo@...hat.com,
        mm-commits@...r.kernel.org, neilb@...e.com, oberpar@...ux.ibm.com,
        rostedt@...dmis.org, viro@...iv.linux.org.uk, vvs@...tuozzo.com
Cc:     akpm@...ux-foundation.org
Subject: Re: + seq_read-info-message-about-buggy-next-functions.patch added
 to -mm tree

On Tue, 2020-02-25 at 19:56 -0800, Andrew Morton wrote:
> ------------------------------------------------------
> From: Vasily Averin <vvs@...tuozzo.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@...tuozzo.com>
> Cc: NeilBrown <neilb@...e.com>
> Cc: Al Viro <viro@...iv.linux.org.uk>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Davidlohr Bueso <dave@...olabs.net>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Manfred Spraul <manfred@...orfullife.com>
> Cc: Peter Oberparleiter <oberpar@...ux.ibm.com>
> Cc: Waiman Long <longman@...hat.com>
> Signed-off-by: Andrew Morton <akpm@...ux-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@...tuozzo.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
>