linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Valerie Aurora <vaurora@redhat.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Miklos Szeredi <miklos@szeredi.hu>, Jan Blunck <jblunck@suse.de>,
	Jamie Lokier <jamie@shareable.org>,
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2] d_ino considered harmful
Date: Wed, 16 Jun 2010 15:54:00 -0400	[thread overview]
Message-ID: <20100616195359.GA24382@shell> (raw)
In-Reply-To: <20100616185913.GA15566@shell>

On Wed, Jun 16, 2010 at 02:59:13PM -0400, Valerie Aurora wrote:
> Who needs d_ino anyway?  I am running a kernel with this patch -
> Gnome, a browser, IRC, kernel compile, etc. and everything works.

Gosh, maybe it would help to patch the currently used readdir instead
of just old_readdir() (thanks, Arnd).  And return 1 instead of 0 so ls
doesn't think all files are deleted (thanks, Andreas).

I'm running a kernel with the below patch and everything still works.
Apparently "ls -i" is still using the bogus d_ino performance
improvement mentioned here because it returns all 1's for inode
number.

http://www.mail-archive.com/bug-findutils@gnu.org/msg02531.html

-VAL

commit 5902fd7b7407e059c5cea1bf1ea101a1ff8a6072
Author: Valerie Aurora <vaurora@redhat.com>
Date:   Wed Jun 16 11:05:06 2010 -0700

    VFS: Always return 1 for d_ino
    
    Use of d_ino without the corresponding st_dev is always buggy in the
    presence of submounts, bind mounts, and union mounts.  E.g., the d_ino
    of a mountpoint will be the inode number of the directory under the
    mountpoint, not the mounted directory.  Correct code must call stat(),
    which returns the correct device ID and inode in st_dev and st_ino.
    Since no one should be using d_ino anyway, always return 1 to detect
    bugs.

diff --git a/fs/readdir.c b/fs/readdir.c
index dd3eae1..5ff8f10 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -91,11 +91,8 @@ static int fillonedir(void * __buf, const char * name, int namlen, loff_t offset
 
 	if (buf->result)
 		return -EINVAL;
-	d_ino = ino;
-	if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
-		buf->result = -EOVERFLOW;
-		return -EOVERFLOW;
-	}
+	/* Use of d_ino without st_dev is always buggy. */
+	d_ino = 1;
 	buf->result++;
 	dirent = buf->dirent;
 	if (!access_ok(VERIFY_WRITE, dirent,
@@ -172,11 +169,8 @@ static int filldir(void * __buf, const char * name, int namlen, loff_t offset,
 	buf->error = -EINVAL;	/* only used if we fail.. */
 	if (reclen > buf->count)
 		return -EINVAL;
-	d_ino = ino;
-	if (sizeof(d_ino) < sizeof(ino) && d_ino != ino) {
-		buf->error = -EOVERFLOW;
-		return -EOVERFLOW;
-	}
+	/* Use of d_ino without st_dev is always buggy. */
+	d_ino = 1;
 	dirent = buf->previous;
 	if (dirent) {
 		if (__put_user(offset, &dirent->d_off))

  parent reply	other threads:[~2010-06-16 19:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-16 18:59 [PATCH] d_ino considered harmful Valerie Aurora
2010-06-16 19:10 ` Arnd Bergmann
2010-06-16 19:58   ` Valerie Aurora
2010-06-16 19:15 ` Jeff Layton
2010-06-16 19:58   ` Valerie Aurora
2010-06-16 19:54 ` Valerie Aurora [this message]
2010-06-16 20:44   ` [PATCH v2] " David Dillow
2010-06-17 18:04     ` J. R. Okajima
2010-06-17 18:17       ` David Dillow
2010-06-17 18:58       ` Valerie Aurora
2010-06-18  1:41       ` Andreas Dilger
2010-06-18  2:57         ` J. R. Okajima
2010-06-17 17:54   ` Jamie Lokier
2010-06-17 19:10     ` Valerie Aurora
2010-06-17 23:39   ` Andreas Dilger
2010-06-18 19:41     ` Valerie Aurora

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=20100616195359.GA24382@shell \
    --to=vaurora@redhat.com \
    --cc=hch@infradead.org \
    --cc=jamie@shareable.org \
    --cc=jblunck@suse.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --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).