Commit Graph

13 Commits

Author SHA1 Message Date
NeilBrown ebd5a480db seq_file: document how per-entry resources are managed.
commit b3656d8227f4c45812c6b40815d8f4e446ed372a upstream.

Patch series "Fix some seq_file users that were recently broken".

A recent change to seq_file broke some users which were using seq_file
in a non-"standard" way ...  though the "standard" isn't documented, so
they can be excused.  The result is a possible leak - of memory in one
case, of references to a 'transport' in the other.

These three patches:
 1/ document and explain the problem
 2/ fix the problem user in x86
 3/ fix the problem user in net/sctp

This patch (of 3):

Users of seq_file will sometimes find it convenient to take a resource,
such as a lock or memory allocation, in the ->start or ->next operations.
These are per-entry resources, distinct from per-session resources which
are taken in ->start and released in ->stop.

The preferred management of these is release the resource on the
subsequent call to ->next or ->stop.

However prior to Commit 1f4aace60b ("fs/seq_file.c: simplify seq_file
iteration code and interface") it happened that ->show would always be
called after ->start or ->next, and a few users chose to release the
resource in ->show.

This is no longer reliable.  Since the mentioned commit, ->next will
always come after a successful ->show (to ensure m->index is updated
correctly), so the original ordering cannot be maintained.

This patch updates the documentation to clearly state the required
behaviour.  Other patches will fix the few problematic users.

[akpm@linux-foundation.org: fix typo, per Willy]

Link: https://lkml.kernel.org/r/161248518659.21478.2484341937387294998.stgit@noble1
Link: https://lkml.kernel.org/r/161248539020.21478.3147971477400875336.stgit@noble1
Fixes: 1f4aace60b ("fs/seq_file.c: simplify seq_file iteration code and interface")
Signed-off-by: NeilBrown <neilb@suse.de>
Cc: Xin Long <lucien.xin@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-04 10:26:48 +01:00
NeilBrown 1f4aace60b fs/seq_file.c: simplify seq_file iteration code and interface
The documentation for seq_file suggests that it is necessary to be able
to move the iterator to a given offset, however that is not the case.
If the iterator is stored in the private data and is stable from one
read() syscall to the next, it is only necessary to support first/next
interactions.  Implementing this in a client is a little clumsy.

 - if ->start() is given a pos of zero, it should go to start of
   sequence.

 - if ->start() is given the name pos that was given to the most recent
   next() or start(), it should restore the iterator to state just
   before that last call

 - if ->start is given another number, it should set the iterator one
   beyond the start just before the last ->start or ->next call.

Also, the documentation says that the implementation can interpret the
pos however it likes (other than zero meaning start), but seq_file
increments the pos sometimes which does impose on the implementation.

This patch simplifies the interface for first/next iteration and
simplifies the code, while maintaining complete backward compatability.
Now:

 - if ->start() is given a pos of zero, it should return an iterator
   placed at the start of the sequence

 - if ->start() is given a non-zero pos, it should return the iterator
   in the same state it was after the last ->start or ->next.

This is particularly useful for interators which walk the multiple
chains in a hash table, e.g.  using rhashtable_walk*.  See
fs/gfs2/glock.c and drivers/staging/lustre/lustre/llite/vvp_dev.c

A large part of achieving this is to *always* call ->next after ->show
has successfully stored all of an entry in the buffer.  Never just
increment the index instead.  Also:

 - always pass &m->index to ->start() and ->next(), never a temp
   variable

 - don't clear ->from when ->count is zero, as ->from is dead when
   ->count is zero.

Some ->next functions do not increment *pos when they return NULL.  To
maintain compatability with this, we still need to increment m->index in
one place, if ->next didn't increment it.  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.

This patch doesn't work around buggy next() functions for this case.

[neilb@suse.com: ensure ->from is valid]
  Link: http://lkml.kernel.org/r/87601ryb8a.fsf@notabene.neil.brown.name
Signed-off-by: NeilBrown <neilb@suse.com>
Acked-by: Jonathan Corbet <corbet@lwn.net>	[docs]
Tested-by: Jann Horn <jannh@google.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2018-08-17 16:20:28 -07:00
Dmitry V. Levin 380945365d Documentation: update seq_file
Update descriptions of seq_path() and seq_path_root():
starting with commit v3.2-rc4-1-g02125a8, seq_path_root() no longer
changes the value of root;
starting with commit v3.2-rc7-104-g8c9379e, some arguments of seq_path()
and seq_path_root() are const.

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
Acked-by: Rob Landley <rob@landley.net>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
2014-12-29 15:40:18 -07:00
Joe Perches 1f33c41c03 seq_file: Rename seq_overflow() to seq_has_overflowed() and make public
The return values of seq_printf/puts/putc are frequently misused.

Start down a path to remove all the return value uses of these
functions.

Move the seq_overflow() to a global inlined function called
seq_has_overflowed() that can be used by the users of seq_file() calls.

Update the documentation to not show return types for seq_printf
et al.  Add a description of seq_has_overflowed().

Link: http://lkml.kernel.org/p/848ac7e3d1c31cddf638a8526fa3c59fa6fdeb8a.1412031505.git.joe@perches.com

Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Joe Perches <joe@perches.com>
[ Reworked the original patch from Joe ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
2014-10-29 20:26:06 -04:00
Rob Jones 77be4daf4e Documentation: seq_file: Document seq_open_private(), seq_release_private()
Despite the fact that these functions have been around for years, they
are little used (only 15 uses in 13 files at the preseht time) even
though many other files use work-arounds to achieve the same result.

By documenting them, hopefully they will become more widely used.

Signed-off-by: Rob Jones <rob.jones@codethink.co.uk>
Acked-by: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2014-09-07 15:21:13 -07:00
Fabian Frederick 0b07cb8271 Documentation/filesystems/seq_file.txt: create_proc_entry deprecated
Linked article in seq_file.txt still uses create_proc_entry which was
removed in commit 80e928f7eb ("proc: Kill create_proc_entry()").

This patch adds information for kernel 3.10 and above

Signed-off-by: Fabian Frederick <fabf@skynet.be>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2014-06-06 16:08:11 -07:00
Alexey Dobriyan 6be4b78993 seq_file: use proc_create() in documentation
Using create_proc_entry() + ->proc_fops assignment is racy because
->proc_fops will be NULL for some time, use proc_create() to avoid race.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2009-12-16 07:20:07 -08:00
Jesper Dangaard Brouer e8188807b7 Doc: seq_file.txt fix wrong dd command example.
Small error in the "dd" command example, "out=" should be "of=".

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
2009-09-10 14:33:35 -06:00
Jonathan Corbet 9f4def9ae4 Document seq_path_root()
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
2008-04-25 11:56:37 -06:00
Jonathan Corbet 22c36d18c6 Document SEQ_SKIP
2.6.26 adds a SEQ_SKIP return value for the seq_file show() function;
update the documentation to match.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
2008-04-24 15:57:32 -06:00
Dmitri Vorobiev b82d4043b3 Fix typos in Documentation/filesystems/seq_file.txt
A couple of typos crept into the newly added document about the seq_file
interface.  This patch corrects those typos and simultaneously deletes
unnecessary trailing spaces.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2008-04-15 19:35:40 -07:00
Jan Engelhardt f3271f6564 Fixes to the seq_file document
On Friday 2008-03-28 19:20, Jonathan Corbet wrote:
>commit 9756ccfda31b4c4544aa010aacf71b6672d668e8
>Date:   Fri Mar 28 11:19:56 2008 -0600
>
>    Add the seq_file documentation

patch on top:

  - add const qualifiers
  - remove void* casts
  - use proper specifier (%Ld is not valid)

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
2008-03-30 11:05:04 -06:00
Jonathan Corbet ded4926aa2 Add the seq_file documentation
This is an updated version of the document describing the seq_file
interface.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
2008-03-30 11:05:04 -06:00