[PATCH] umount after bad chdir

[PATCH] umount after bad chdir

[Hugh Dickins]

After chdir (or chroot) to non-existent directory on 2.6.5-mm5, you
can no longer unmount filesystem holding working directory (or root).

--- 2.6.5-mm5/fs/open.c	2004-04-13 11:02:25.000000000 +0100
+++ linux/fs/open.c	2004-04-14 12:23:26.633056368 +0100
@@ -517,13 +517,16 @@ asmlinkage long sys_chdir(const char __u
 {
 	struct nameidata nd;
 	int error;
-	struct vfsmount *old_mnt = mntget(current->fs->pwdmnt);
-	struct dentry *old_dentry = dget(current->fs->pwd);
+	struct vfsmount *old_mnt;
+	struct dentry *old_dentry;
 
 	error = __user_walk(filename, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, &nd);
 	if (error)
 		goto out;
 
+	old_mnt = mntget(current->fs->pwdmnt);
+	old_dentry = dget(current->fs->pwd);
+
 	error = permission(nd.dentry->d_inode,MAY_EXEC,&nd);
 	if (error)
 		goto dput_and_out;
@@ -590,13 +593,16 @@ asmlinkage long sys_chroot(const char __
 {
 	struct nameidata nd;
 	int error;
-	struct vfsmount *old_mnt = mntget(current->fs->rootmnt);
-	struct dentry *old_dentry = dget(current->fs->root);
+	struct vfsmount *old_mnt;
+	struct dentry *old_dentry;
 
 	error = __user_walk(filename, LOOKUP_FOLLOW | LOOKUP_DIRECTORY | LOOKUP_NOALT, &nd);
 	if (error)
 		goto out;
 
+	old_mnt = mntget(current->fs->pwdmnt);
+	old_dentry = dget(current->fs->pwd);
+
 	error = permission(nd.dentry->d_inode,MAY_EXEC,&nd);
 	if (error)
 		goto dput_and_out;

Re: [PATCH] umount after bad chdir

[viro]

On Wed, Apr 14, 2004 at 08:12:33PM +0800, raven@themaw.net wrote:
> On Wed, 14 Apr 2004, Hugh Dickins wrote:
> 
> > After chdir (or chroot) to non-existent directory on 2.6.5-mm5, you
> > can no longer unmount filesystem holding working directory (or root).
> > 
> 
> Of course.
> 
> Excellent. Thanks very much.

Mind you, chdir() patch in -mm is broken in a lot of other ways - e.g.
it assumes that another thread sharing ->fs with us won't call chdir()
in the wrong moment...

Re: [PATCH] umount after bad chdir

[raven]

On Wed, 14 Apr 2004, Hugh Dickins wrote:

> After chdir (or chroot) to non-existent directory on 2.6.5-mm5, you
> can no longer unmount filesystem holding working directory (or root).
> 

Of course.

Excellent. Thanks very much.

Ian

Re: [PATCH] umount after bad chdir

[raven]

On Wed, 14 Apr 2004 viro@parcelfarce.linux.theplanet.co.uk wrote:

> On Wed, Apr 14, 2004 at 08:12:33PM +0800, raven@themaw.net wrote:
> > On Wed, 14 Apr 2004, Hugh Dickins wrote:
> > 
> > > After chdir (or chroot) to non-existent directory on 2.6.5-mm5, you
> > > can no longer unmount filesystem holding working directory (or root).
> > > 
> > 
> > Of course.
> > 
> > Excellent. Thanks very much.
> 
> Mind you, chdir() patch in -mm is broken in a lot of other ways - e.g.
> it assumes that another thread sharing ->fs with us won't call chdir()
> in the wrong moment...

Thanks for your interest Al.

I see your point (I think).

If I understand you correctly (please explain if I don't) I need to lock 
the ->fs struct.
But then I can't use the set_fs_xxx calls.

Do you have any suggestions on how I should do this?

Ian

Re: [PATCH] umount after bad chdir

[raven]

On Wed, 14 Apr 2004 raven@themaw.net wrote:

> > Mind you, chdir() patch in -mm is broken in a lot of other ways - e.g.
> > it assumes that another thread sharing ->fs with us won't call chdir()
> > in the wrong moment...
> 
> Thanks for your interest Al.
> 
> I see your point (I think).
> 
> If I understand you correctly (please explain if I don't) I need to lock 
> the ->fs struct.

Mmm ... doesn't look much good in the light of Als comment.

Looks like it's not possible to take the lock for long enough even if I 
could.

Lets have some comments, criticisms or suggestions please.

Ian

Re: [PATCH] umount after bad chdir

[viro]

On Wed, Apr 14, 2004 at 11:13:13PM +0800, raven@themaw.net wrote:
> On Wed, 14 Apr 2004 raven@themaw.net wrote:
> 
> > > Mind you, chdir() patch in -mm is broken in a lot of other ways - e.g.
> > > it assumes that another thread sharing ->fs with us won't call chdir()
> > > in the wrong moment...
> > 
> > Thanks for your interest Al.
> > 
> > I see your point (I think).
> > 
> > If I understand you correctly (please explain if I don't) I need to lock 
> > the ->fs struct.
> 
> Mmm ... doesn't look much good in the light of Als comment.
> 
> Looks like it's not possible to take the lock for long enough even if I 
> could.
> 
> Lets have some comments, criticisms or suggestions please.

Why do you need to assign pwd before revalidation?

Re: [PATCH] umount after bad chdir

[raven]

On Wed, 14 Apr 2004 viro@parcelfarce.linux.theplanet.co.uk wrote:

> On Wed, Apr 14, 2004 at 11:13:13PM +0800, raven@themaw.net wrote:
> > On Wed, 14 Apr 2004 raven@themaw.net wrote:
> > 
> > > > Mind you, chdir() patch in -mm is broken in a lot of other ways - e.g.
> > > > it assumes that another thread sharing ->fs with us won't call chdir()
> > > > in the wrong moment...
> > > 
> > > Thanks for your interest Al.
> > > 
> > > I see your point (I think).
> > > 
> > > If I understand you correctly (please explain if I don't) I need to lock 
> > > the ->fs struct.
> > 
> > Mmm ... doesn't look much good in the light of Als comment.
> > 
> > Looks like it's not possible to take the lock for long enough even if I 
> > could.
> > 
> > Lets have some comments, criticisms or suggestions please.
> 
> Why do you need to assign pwd before revalidation?
> 

Good question.

I'm talking about lazy mounting in autofs version 4 (suprise, suprise).

I think this should be done in the call backs during the path_walk 
but I couldn't work out how. But see below...

The basic problem this is meant to solve is that I can't tell when a 
chdir or chroot is to be done from within the revalidate or lookup. To 
delay mounting until (or correctly trigger a mount at) the proper 
time I must know if the service request is a chdir or chroot, in which 
case an automount needs to be done. The chdir and chroot are the only 
problematic services that I'm aware of atm.

But looking further I see that a LOOKUP_DIRECTORY flag is used only for 
these two routines (excluding pivot_root) and when a trailing slash is 
present in the path. I think that the if this flag is present then the 
request will always want to look into the directory anyway, so if it's 
an autofs4 mount point it should be mounted then. If this is the case I 
can get this stuff into the fs module where it belongs.

I'll think about it some more and look around further before I do 
anything.

Thoughts?

Ian

Re: [PATCH] umount after bad chdir

[raven]

On Thu, 15 Apr 2004 raven@themaw.net wrote:

> 
> But looking further I see that a LOOKUP_DIRECTORY flag is used only for 
> these two routines (excluding pivot_root) and when a trailing slash is 
> present in the path. I think that the if this flag is present then the 
> request will always want to look into the directory anyway, so if it's 
> an autofs4 mount point it should be mounted then. If this is the case I 
> can get this stuff into the fs module where it belongs.
> 

But it's not as simple as that after all ...

Ian