2001-10-10 21:34:42

by Mingming Cao

[permalink] [raw]
Subject: [PATCH]Fix bug:rmdir could remove current working directory

Hi Linus, Alan and Al,

I found that rmdir(2) could remove current working directory
successfully. This happens when the given pathname points to current
working directory, not ".", but something else. For example, the current
working directory's absolute pathname. I read the man page of
rmdir(2). It says in this case EBUSY error should be returned. I
suspected this is a bug and added a check in vfs_rmdir(). The following
patch is against 2.4.10 and has been verified. Please comment and
apply.

--
Mingming Cao

--- linux-2.4.10/fs/namei.c Tue Sep 18 11:01:47 2001
+++ /home/ming/linux-tk/fs/namei.c Tue Oct 9 11:58:50 2001
@@ -1362,6 +1362,8 @@
error = -ENOENT;
else if (d_mountpoint(dentry))
error = -EBUSY;
+ else if (dentry == current->fs->pwd)
+ error = -EBUSY;
else {
lock_kernel();
error = dir->i_op->rmdir(dir, dentry);


2001-10-10 21:45:43

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory



On Wed, 10 Oct 2001, Mingming cao wrote:

> Hi Linus, Alan and Al,
>
> I found that rmdir(2) could remove current working directory
> successfully. This happens when the given pathname points to current
> working directory, not ".", but something else. For example, the current
> working directory's absolute pathname. I read the man page of
> rmdir(2). It says in this case EBUSY error should be returned. I
> suspected this is a bug and added a check in vfs_rmdir(). The following
> patch is against 2.4.10 and has been verified. Please comment and
> apply.

It's not a bug. Moreover, test you add is obviously bogus - what about
cwd of other processes?

Actually, rmdir() on a busy directory _is_ OK. Implementation is allowed
to refuse doing that, but it's not required to.

2001-10-10 22:05:35

by Mingming Cao

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory

Alexander Viro wrote:
>
> On Wed, 10 Oct 2001, Mingming cao wrote:
>
> > Hi Linus, Alan and Al,
> >
> > I found that rmdir(2) could remove current working directory
> > successfully. This happens when the given pathname points to current
> > working directory, not ".", but something else. For example, the current
> > working directory's absolute pathname. I read the man page of
> > rmdir(2). It says in this case EBUSY error should be returned. I
> > suspected this is a bug and added a check in vfs_rmdir(). The following
> > patch is against 2.4.10 and has been verified. Please comment and
> > apply.
>
> It's not a bug. Moreover, test you add is obviously bogus - what about
> cwd of other processes?
>
> Actually, rmdir() on a busy directory _is_ OK. Implementation is allowed
> to refuse doing that, but it's not required to.

I thought about the case when rmdir() on the cwd of other processes,
but, as you said, that is implementation dependent. However rmdir() on
"." does returns EBUSY error. Should not we keep the rmdir() behavior
consistent: rmdir() on the current working directory of the current
process is not OK?

--
Mingming Cao

2001-10-10 22:09:05

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory



On Wed, 10 Oct 2001, Mingming cao wrote:

> I thought about the case when rmdir() on the cwd of other processes,
> but, as you said, that is implementation dependent. However rmdir() on
> "." does returns EBUSY error. Should not we keep the rmdir() behavior
> consistent: rmdir() on the current working directory of the current
> process is not OK?

We do. It's not just ".", "foo/bar/." is also prohibited. IOW, the
rule is "the last component of name should not be . or ..". Please,
look through archives - it had been beaten to death many times.

2001-10-10 22:21:18

by Ricky Beam

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory

On Wed, 10 Oct 2001, Mingming cao wrote:
>I read the man page of
>rmdir(2). It says in this case EBUSY error should be returned. I
>suspected this is a bug and added a check in vfs_rmdir(). The following
>patch is against 2.4.10 and has been verified. Please comment and
>apply.

The bug is in the manpage. This was discussed over a year ago (some time
after 2.1.44 introduced dcache (it was broken then, but that's when it
appeared.))

--Ricky


2001-10-10 23:18:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory

In article <[email protected]>,
Mingming cao <[email protected]> wrote:
>
>I thought about the case when rmdir() on the cwd of other processes,
>but, as you said, that is implementation dependent. However rmdir() on
>"." does returns EBUSY error.

That's a completely different thing, though - even though the difference
is rather subtle.

You can remove pretty much any empty directory (if the filesystem
permits it - some don't). HOWEVER, you can not use "." as the final
component of your pathname.

It has nothing to do with home directory: you can try just doing

mkdir /tmp/hello
rmdir /tmp/hello/.

and you'll get the same error (and it _should_ return EINVAL, not EBUSY.
EBUSY is for the "this filesystem doesn't allow you to remove a
directory that is in use" case).

Linus

2001-10-10 23:35:59

by John Levon

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory

On Wed, Oct 10, 2001 at 06:21:01PM -0400, Ricky Beam wrote:

> On Wed, 10 Oct 2001, Mingming cao wrote:
> >I read the man page of
> >rmdir(2). It says in this case EBUSY error should be returned. I
> >suspected this is a bug and added a check in vfs_rmdir(). The following
> >patch is against 2.4.10 and has been verified. Please comment and
> >apply.
>
> The bug is in the manpage. This was discussed over a year ago (some time

Well, the manpage only states what certain error nr. returns may mean, not what
will be returned when. Do you have an improvement on :

EBUSY pathname is the current working directory or root directory of some process.

regards
john

--
"Beware of bugs in the above code; I have only proved it correct, not tried
it."
- Donald Knuth

2001-10-11 00:04:36

by Mingming Cao

[permalink] [raw]
Subject: Re: [PATCH]Fix bug:rmdir could remove current working directory

Linus Torvalds wrote:
>
> In article <[email protected]>,
> Mingming cao <[email protected]> wrote:
> >
> >I thought about the case when rmdir() on the cwd of other processes,
> >but, as you said, that is implementation dependent. However rmdir() on
> >"." does returns EBUSY error.
>
> That's a completely different thing, though - even though the difference
> is rather subtle.
>
> You can remove pretty much any empty directory (if the filesystem
> permits it - some don't). HOWEVER, you can not use "." as the final
> component of your pathname.
>
> It has nothing to do with home directory: you can try just doing
>
> mkdir /tmp/hello
> rmdir /tmp/hello/.
>
> and you'll get the same error (and it _should_ return EINVAL, not EBUSY.
> EBUSY is for the "this filesystem doesn't allow you to remove a
> directory that is in use" case).
>
> Linus

I misunderstanded the rule. Thanks for clarifying!

--
Mingming Cao