From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. R. Okajima" Subject: [PATCH v2] vfs: get_next_ino(), never inum=0 Date: Wed, 28 May 2014 23:06:32 +0900 Message-ID: <1401285992-29374-1-git-send-email-hooanon05g@gmail.com> References: <''> To: linux-fsdevel@vger.kernel.org, dchinner@redhat.com, viro@zeniv.linux.org.uk, Eric Dumazet , Hugh Dickins , Christoph Hellwig , Andreas Dilger , Jan Kara Return-path: Received: from mail03-md.ns.itscom.net ([175.177.155.113]:46961 "EHLO mail03-md.ns.itscom.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753525AbaE1OGg (ORCPT ); Wed, 28 May 2014 10:06:36 -0400 In-Reply-To: <''> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 Cc: Hugh Dickins Cc: Christoph Hellwig Cc: Andreas Dilger Cc: Jan Kara Signed-off-by: J. R. Okajima --- 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