linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. R. Okajima" <hooanon05g@gmail.com>
To: linux-fsdevel@vger.kernel.org, dchinner@redhat.com,
	viro@zeniv.linux.org.uk, Eric Dumazet <edumazet@google.com>,
	Hugh Dickins <hughd@google.com>, Christoph Hellwig <hch@lst.de>,
	Andreas Dilger <adilger@dilger.ca>, Jan Kara <jack@suse.cz>
Subject: [PATCH v2] vfs: get_next_ino(), never inum=0
Date: Wed, 28 May 2014 23:06:32 +0900	[thread overview]
Message-ID: <1401285992-29374-1-git-send-email-hooanon05g@gmail.com> (raw)
In-Reply-To: <'<CANn89i+PBEGp=9QGRioa7CUDZmApT-UNa=OJTdz4eu7AyO3Kbw@mail.gmail.com>'>

It is very rare for get_next_ino() to return zero as a new inode number
since its type is unsigned int, but it can surely happen eventually.

Interestingly, ls(1) and find(1) (actually readdir(3)) don't show a file
whose inum is zero, so people won't be able to find it. This issue may
be harmful especially for tmpfs.

On a very long lived and busy system, users may frequently create files
on tmpfs. And if unluckily he gets inum=0, then he cannot see its
filename. If he remembers its name, he may be able to use or unlink it
by its name since the file surely exists. Otherwise, the file remains on
tmpfs silently. No one can touch it. This behaviour looks like resource
leak.
As a worse case, if a dir gets inum=0 and a user creates several files
under it, then the leaked memory will increase since a user cannot see
the name of all files under the dir whose inum=0, regardless the inum of
the children.

There is another unpleasant effect when get_next_ino() wraps
around. When there is a file whose inum=100 on tmpfs, a new file may get
inum=100, ie. the duplicated inums. I am not sure what will happen when
the duplicated inums exist on tmpfs. If it happens, then some tools
won't work correctly such as backup tools, I am afraid.
Anyway this is not a issue in get_next_ino(). It should be
fixed in mm/shmem.c separatly if it is really necessary.

There are many other get_next_ino() callers other than tmpfs, such as
several drivers, anon_inode, autofs4, freevxfs, procfs, pis, hugetlbfs,
configfs, ramfs, fuse, ocfs2, debugfs, securityfs, cgroup, socket, ipc.
Some of them will not care inum so this issue is harmless for them. But
the others may suffer from inum=0. For example, if procfs gets inum=0
for a task dir (or for one of its children), then several utilities
won't work correctly, including ps(1), lsof(8), etc.

(Essentially the patch is re-written by Eric Dumazet.)

Cc: Eric Dumazet <edumazet@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Andreas Dilger <adilger@dilger.ca>
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: J. R. Okajima <hooanon05g@gmail.com>
---
 fs/inode.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 567296b..58e7c56 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -840,6 +840,8 @@ unsigned int get_next_ino(void)
 	unsigned int *p = &get_cpu_var(last_ino);
 	unsigned int res = *p;
 
+start:
+
 #ifdef CONFIG_SMP
 	if (unlikely((res & (LAST_INO_BATCH-1)) == 0)) {
 		static atomic_t shared_last_ino;
@@ -849,7 +851,9 @@ unsigned int get_next_ino(void)
 	}
 #endif
 
-	*p = ++res;
+	if (unlikely(!++res))
+		goto start;	/* never zero */
+	*p = res;
 	put_cpu_var(last_ino);
 	WARN(!res, "static inum wrapped around");
 	return res;
-- 
1.7.10.4


       reply	other threads:[~2014-05-28 14:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <'<CANn89i+PBEGp=9QGRioa7CUDZmApT-UNa=OJTdz4eu7AyO3Kbw@mail.gmail.com>
2014-05-28 14:06 ` J. R. Okajima [this message]
2014-04-29 15:45 [PATCH] vfs: get_next_ino(), never inum=0 hooanon05g
2014-08-18 18:21 ` [PATCH v2] " Carlos Maiolino
2014-08-19  0:58   ` J. R. Okajima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1401285992-29374-1-git-send-email-hooanon05g@gmail.com \
    --to=hooanon05g@gmail.com \
    --cc=adilger@dilger.ca \
    --cc=dchinner@redhat.com \
    --cc=edumazet@google.com \
    --cc=hch@lst.de \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).