[PATCH] Bug in unlink().

Alexander Viro (viro@math.psu.edu)
Sun, 29 Nov 1998 18:06:50 -0500 (EST)

Folks, we got a bug in unlink(). Description: unlink()ing a directory
should give EPERM. VFS leaves the check to filesystem drivers. Some of them
do it, but some just go and remove the directory in question as if it was a
plain file. Results: lots of lost inodes, unhappy dcache, etc.
Details:
1) affs, ext2, minix, sysv, qnx4, ufs: check done by hands.
2) coda, nfs, smb: it's a server's headache; just pass the request.
3) ncpfs: pass to server. Worse yet, looks like it will work for directories
and may royally screw dcache. I don't have access to NW server, so...
4) autofs: Different check, different return value (EINVAL instead of EPERM).
5) openpromfs (aliases): Looks like it can't contain subdirs. Or maybe not -
I don't have Linux Sparc at hands.
6) procfs (dynamical directories). We are f*cked. unlink("/proc/sys/net/unix")
succeeds, no matter what. No, it isn't even empty ;-<
7) hfs_cap_ndir, hfs_nat_ndir, hfs_dbl_dir: seems to be OK.
8) hfs_nat_hdir: Devil knows. Looks like it can't have subdirs.
9) umsdos (and stub for uvfat in vfat): We are f*cked. Filesystem corruption,
lost inodes, screwed dcache. I suspect that turning it into the local
DoS is _not_ hard.
10) msdos, vfat: OK. They simply require the file to be regular.
11) the rest of filesystems in the kernel: no unlink.
12) filesystems not included in the official kernel: $DEITY only knows.

Fix: include the test into fs/namei.c:do_unlink() and remove it from
filesystem drivers that implement it (group 1 above). Check in autofs
still stands, checks in msdos and vfat probably can be removed (there are
only files and directories on those filesystems), but it's a different
story.

Linus, could you apply the following patch?
TIA,
Al

diff -u --recursive --new-file v2.1.130/linux/fs/affs/namei.c linux/fs/affs/namei.c
--- v2.1.130/linux/fs/affs/namei.c Fri May 8 01:58:04 1998
+++ linux/fs/affs/namei.c Sun Nov 29 07:09:07 1998
@@ -242,8 +242,6 @@

inode = dentry->d_inode;
retval = -EPERM;
- if (S_ISDIR(inode->i_mode))
- goto unlink_done;
if (current->fsuid != inode->i_uid &&
current->fsuid != dir->i_uid && !capable(CAP_FOWNER))
goto unlink_done;
diff -u --recursive --new-file v2.1.130/linux/fs/ext2/namei.c linux/fs/ext2/namei.c
--- v2.1.130/linux/fs/ext2/namei.c Fri Nov 13 18:02:03 1998
+++ linux/fs/ext2/namei.c Sun Nov 29 07:10:41 1998
@@ -733,8 +733,6 @@
DQUOT_INIT(inode);

retval = -EPERM;
- if (S_ISDIR(inode->i_mode))
- goto end_unlink;
if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
goto end_unlink;
if ((dir->i_mode & S_ISVTX) &&
diff -u --recursive --new-file v2.1.130/linux/fs/minix/namei.c linux/fs/minix/namei.c
--- v2.1.130/linux/fs/minix/namei.c Fri Nov 13 18:00:32 1998
+++ linux/fs/minix/namei.c Sun Nov 29 07:13:33 1998
@@ -490,8 +490,6 @@
inode = dentry->d_inode;

retval = -EPERM;
- if (S_ISDIR(inode->i_mode))
- goto end_unlink;
if (de->inode != inode->i_ino) {
brelse(bh);
current->counter = 0;
diff -u --recursive --new-file v2.1.130/linux/fs/namei.c linux/fs/namei.c
--- v2.1.130/linux/fs/namei.c Fri Nov 20 09:38:43 1998
+++ linux/fs/namei.c Sun Nov 29 07:08:28 1998
@@ -943,13 +943,16 @@
goto exit_lock;

/*
+ * A directory can't be unlink'ed.
* A file cannot be removed from an append-only directory.
*/
error = -EPERM;
+ if (S_ISDIR(dentry->d_inode->i_mode))
+ goto exit_lock;
+
if (IS_APPEND(dir->d_inode))
goto exit_lock;

- error = -EPERM;
if (!dir->d_inode->i_op || !dir->d_inode->i_op->unlink)
goto exit_lock;

diff -u --recursive --new-file v2.1.130/linux/fs/qnx4/namei.c linux/fs/qnx4/namei.c
--- v2.1.130/linux/fs/qnx4/namei.c Fri Nov 13 17:59:35 1998
+++ linux/fs/qnx4/namei.c Sun Nov 29 07:14:04 1998
@@ -245,9 +245,6 @@
goto end_unlink;
}
retval = -EPERM;
- if (S_ISDIR(inode->i_mode)) {
- goto end_unlink;
- }
if ((dir->i_mode & S_ISVTX) &&
current->fsuid != inode->i_uid &&
current->fsuid != dir->i_uid && !capable(CAP_FOWNER)) {
diff -u --recursive --new-file v2.1.130/linux/fs/sysv/namei.c linux/fs/sysv/namei.c
--- v2.1.130/linux/fs/sysv/namei.c Thu May 14 21:52:31 1998
+++ linux/fs/sysv/namei.c Sun Nov 29 07:14:29 1998
@@ -476,8 +476,6 @@
inode = dentry->d_inode;

retval = -EPERM;
- if (S_ISDIR(inode->i_mode))
- goto end_unlink;
if (de->inode != inode->i_ino) {
brelse(bh);
current->counter = 0;
diff -u --recursive --new-file v2.1.130/linux/fs/ufs/namei.c linux/fs/ufs/namei.c
--- v2.1.130/linux/fs/ufs/namei.c Fri Nov 13 18:02:04 1998
+++ linux/fs/ufs/namei.c Sun Nov 29 07:14:55 1998
@@ -784,8 +784,6 @@
inode->i_sb->dq_op->initialize (inode, -1);

retval = -EPERM;
- if (S_ISDIR(inode->i_mode))
- goto end_unlink;
if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
goto end_unlink;
if ((dir->i_mode & S_ISVTX) &&

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/