2004-04-14 11:44:13

by Hugh Dickins

[permalink] [raw]
Subject: [PATCH] umount after bad chdir

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;


2004-04-14 12:07:40

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

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

2004-04-14 12:10:33

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

On Wed, Apr 14, 2004 at 08:12:33PM +0800, [email protected] 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...

2004-04-14 12:27:31

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

On Wed, 14 Apr 2004 [email protected] wrote:

> On Wed, Apr 14, 2004 at 08:12:33PM +0800, [email protected] 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

2004-04-14 15:09:36

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

On Wed, 14 Apr 2004 [email protected] 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

2004-04-14 15:24:23

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

On Wed, Apr 14, 2004 at 11:13:13PM +0800, [email protected] wrote:
> On Wed, 14 Apr 2004 [email protected] 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?

2004-04-14 16:25:18

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

On Wed, 14 Apr 2004 [email protected] wrote:

> On Wed, Apr 14, 2004 at 11:13:13PM +0800, [email protected] wrote:
> > On Wed, 14 Apr 2004 [email protected] 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

2004-04-14 16:44:12

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH] umount after bad chdir

On Thu, 15 Apr 2004 [email protected] 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