[patch v3] vfs: fix sys_getcwd for detached mounts

[patch v3] vfs: fix sys_getcwd for detached mounts

[Miklos Szeredi]

I tested this one in various situations, and it appears to do what
it's supposed to.

A somewhat analogous issue is if we have cwd (or fd) outside
current->root.  Should we add a prefix for that case as well?  Perhaps
a double slash?

--
From: Miklos Szeredi <mszeredi@suse.cz>

Currently getcwd(2) on a detached mount will give a garbled result:

   > mkdir /mnt/foo
   > mount --bind /etc /mnt/foo
   > cd /mnt/foo/skel
   > umount -l /mnt/foo
   > /bin/pwd
   etcskel

After the patch the result will be "detached:skel".

Or if the root was mounted:

   > mount --bind / /mnt/foo
   > cd /mnt/foo/etc
   > umount -l /mnt/foo
   > /bin/pwd
   /etc

After the patch the result will be "detached:etc".

Thanks to John Johansen for pointing out this bug.

v2: comment from Al Viro: use "detached:" prefix to differentiate it from
attached paths.

v3: fix case when mounted dentry is root of the filesystem.

Reported-by: John Johansen <jjohansen@suse.de>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/dcache.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c	2008-06-23 19:34:21.000000000 +0200
+++ linux-2.6/fs/dcache.c	2008-06-23 20:15:09.000000000 +0200
@@ -1762,6 +1762,12 @@ static int prepend_name(char **buffer, i
 	return prepend(buffer, buflen, name->name, name->len);
 }
 
+static bool is_pseudo_root(struct dentry *dentry)
+{
+	return IS_ROOT(dentry) &&
+		(dentry->d_name.len != 1 || dentry->d_name.name[0] != '/');
+}
+
 /**
  * __d_path - return the path of a dentry
  * @path: the dentry/vfsmount to report
@@ -1828,8 +1834,16 @@ char *__d_path(const struct path *path, 
 
 global_root:
 	retval += 1;	/* hit the slash */
-	if (prepend_name(&retval, &buflen, &dentry->d_name) != 0)
-		goto Elong;
+
+	if (vfsmnt->mnt_ns != NULL || is_pseudo_root(dentry)) {
+		/* Attached, or pseudo filesystem with "foo:" prefix */
+		if (prepend_name(&retval, &buflen, &dentry->d_name) != 0)
+			goto Elong;
+	} else {
+		/* Detached */
+		if (prepend(&retval, &buflen, "detached:", 9) != 0)
+			goto Elong;
+	}
 	root->mnt = vfsmnt;
 	root->dentry = dentry;
 	return retval;

Re: [patch v3] vfs: fix sys_getcwd for detached mounts

[Matthew Wilcox]

On Mon, Jun 23, 2008 at 08:25:26PM +0200, Miklos Szeredi wrote:
> I tested this one in various situations, and it appears to do what
> it's supposed to.
> 
> A somewhat analogous issue is if we have cwd (or fd) outside
> current->root.  Should we add a prefix for that case as well?  Perhaps
> a double slash?

unreachable: ?

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [patch v3] vfs: fix sys_getcwd for detached mounts

[Al Viro]

On Mon, Jun 23, 2008 at 08:25:26PM +0200, Miklos Szeredi wrote:
> I tested this one in various situations, and it appears to do what
> it's supposed to.
> 
> A somewhat analogous issue is if we have cwd (or fd) outside
> current->root.  Should we add a prefix for that case as well?  Perhaps
> a double slash?

OK, I've applied everything except the last one (all in for-linus in
vfs-2.6.git).  As for the stuff outside current->root (or other namespace,
for that matter)...  I don't like the idea of using // - as it is,
we have a nice sane check (path[0] != '/') and if anything, I'd consider
going for deleted:<path> instead of <path> (deleted) as we have now.
_That_ would eliminate the major annoyance with handling of unlinked/rmdired -
we'd get rid of guessing whether that's a file really called "a (deleted)"
or an unlinked one that used to be called "a"...

Re: [patch v3] vfs: fix sys_getcwd for detached mounts

[Miklos Szeredi]

> On Mon, Jun 23, 2008 at 08:25:26PM +0200, Miklos Szeredi wrote:
> > I tested this one in various situations, and it appears to do what
> > it's supposed to.
> > 
> > A somewhat analogous issue is if we have cwd (or fd) outside
> > current->root.  Should we add a prefix for that case as well?  Perhaps
> > a double slash?
> 
> OK, I've applied everything except the last one (all in for-linus in
> vfs-2.6.git).

Thanks.  Could you please open a branch for this last patch and get
that into linux-next ?  I've also got some other cleanup stuff
pending, and Andrew would prefer all VFS stuff to go through you.

>  As for the stuff outside current->root (or other
> namespace, for that matter)...  I don't like the idea of using // -
> as it is, we have a nice sane check (path[0] != '/') and if
> anything, I'd consider going for deleted:<path> instead of <path>
> (deleted) as we have now.  _That_ would eliminate the major
> annoyance with handling of unlinked/rmdired - we'd get rid of
> guessing whether that's a file really called "a (deleted)" or an
> unlinked one that used to be called "a"...

Yes, it would be nice and consistent.  OTOH, there is a real chance of
breaking some stuff, that already tries to parse "(deleted)".  I don't
think it's worth touching that thing.

Miklos