From: Miklos Szeredi <[email protected]>
Currently getcwd(2) on a detached mount will give a garbled result:
> mkdir /mnt/foo
> mount --bind /etc /mnt/foo
> cd /mnt/foo/skel
> /bin/pwd
/mnt/foo/skel
> umount -l /mnt/foo
> /bin/pwd
etcskel
After the patch it will give a much saner "/skel" result.
Thanks to John Johansen for pointing out this bug.
Reported-by: John Johansen <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
Acked-by: Christoph Hellwig <[email protected]>
---
fs/dcache.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2008-06-13 13:12:57.000000000 +0200
+++ linux-2.6/fs/dcache.c 2008-06-13 13:13:01.000000000 +0200
@@ -1825,10 +1825,20 @@ char *__d_path(const struct path *path,
return retval;
global_root:
- retval += 1; /* hit the slash */
- name = &dentry->d_name;
- if (prepend(&retval, &buflen, name->name, name->len) != 0)
- goto Elong;
+ /*
+ * If this is a root dentry, then overwrite the slash. This
+ * will also DTRT with pseudo filesystems which have root
+ * dentries named "foo:".
+ *
+ * Otherwise this is the root of a detached mount, so don't do
+ * anything.
+ */
+ if (IS_ROOT(dentry)) {
+ retval += 1;
+ name = &dentry->d_name;
+ if (prepend(&retval, &buflen, name->name, name->len) != 0)
+ goto Elong;
+ }
root->mnt = vfsmnt;
root->dentry = dentry;
return retval;
--
On Mon, Jun 16, 2008 at 01:28:06PM +0200, Miklos Szeredi wrote:
> After the patch it will give a much saner "/skel" result.
I'm not sure that /skel is much saner, to be honest. Existing
behaviour is BS, no arguments about that, but I suspect that
we want that to be recognizable. How about doing something
like detached:<rest of path> instead (c.f. "pipe:[6969]" and
its ilk)?
Lack of leading / currently points to pathname *not* resolving
to object; it might be worth doing the same here. Comments?
> > After the patch it will give a much saner "/skel" result.
>
> I'm not sure that /skel is much saner, to be honest.
Just for argument's sake: before the patch getcwd() on detached object
wasn't consistent with any definition of "absolute path". After the
patch it's at least consistent with defining it as the path from the
ultimate reachable ancestor (which does have at least some historical
relevance).
> Existing
> behaviour is BS, no arguments about that, but I suspect that
> we want that to be recognizable. How about doing something
> like detached:<rest of path> instead (c.f. "pipe:[6969]" and
> its ilk)?
OK, obviously current users don't care, otherwise somebody would have
complained about this issue. So if we agree on "detached:...", I'm
fine with that.
Thanks,
Miklos
On Mon, Jun 23, 2008 at 04:34:00PM +0200, Miklos Szeredi wrote:
> > > After the patch it will give a much saner "/skel" result.
> >
> > I'm not sure that /skel is much saner, to be honest.
>
> Just for argument's sake: before the patch getcwd() on detached object
> wasn't consistent with any definition of "absolute path". After the
> patch it's at least consistent with defining it as the path from the
> ultimate reachable ancestor (which does have at least some historical
> relevance).
Before the patch getcwd() on detached object generated junk, period.
As for the path from ultimate reachable ancestor... I'm not sure that
this definition actually matches any real-world problem.
> > Existing
> > behaviour is BS, no arguments about that, but I suspect that
> > we want that to be recognizable. How about doing something
> > like detached:<rest of path> instead (c.f. "pipe:[6969]" and
> > its ilk)?
>
> OK, obviously current users don't care, otherwise somebody would have
> complained about this issue. So if we agree on "detached:...", I'm
> fine with that.
Care to resend? BTW, one more thing - in the 1/3 I'd probably add a
wrapper around prepend() that would take struct qstr * instead of
name/length and used it instead of your locals. As in
prepend_name(&end, &buflen, &dentry->d_name)
etc.