unlink system call on directories - Bug + fix

Andrew M. Bishop (amb@gedanken.demon.co.uk)
Fri, 15 Nov 1996 19:18:02 GMT


The manual page for unlink() and the actual behaviour do not match
when the file to be unlinked is a directory.

The manual page says that unlink() returns EISDIR when the pathname
refers to a directory. But the kernel returns EPERM for minix and
derivatives ext,ext2 etc., see the file namei.c in those directories.

The fix for these is trivial (if I understand correctly how the kernel
handles the unlink command) and left as an exercise to the reader.

Even worse for NFS, the unlink() call will succeed on directories.
The inode will disapear, only to be found in lost+found on the
file server next time it is fsck'd.

This patch (for kernel 2.0.2[45]) fixes it, but maybe a guru could
look it over for me before including it in the kernel.

I have merged part of the nfs_sillyrename() function into nfs_unlink()
so that nfs_lookup(dir, name, len, &inode) is not called twice (once
in each).

diff -u linux/fs/nfs/dir.c.orig linux/fs/nfs/dir.c
@@ -513,23 +513,14 @@
return error;
}

-static int nfs_sillyrename(struct inode *dir, const char *name, int len)
+static int nfs_sillyrename(struct inode *dir, const char *name, int len, struct inode *inode)
{
- struct inode *inode;
- char silly[16];
- int slen, ret;
+ char silly[14];
+ int slen, ret;

- dir->i_count++;
- if (nfs_lookup(dir, name, len, &inode) < 0)
- return -EIO; /* arbitrary */
- if (inode->i_count == 1 || NFS_RENAMED_DIR(inode)) {
- iput(inode);
- return -EIO;
- }
slen = sprintf(silly, ".nfs%ld", inode->i_ino);

if (len == slen && !strncmp(name, silly, len)) {
- iput(inode);
return -EIO; /* DWIM */
}
ret = nfs_proc_rename(NFS_SERVER(dir), NFS_FH(dir), name,
@@ -540,7 +531,6 @@
NFS_RENAMED_DIR(inode) = dir;
dir->i_count++;
}
- iput(inode);
return ret;
}

@@ -560,7 +550,8 @@

static int nfs_unlink(struct inode *dir, const char *name, int len)
{
- int error;
+ struct inode *inode;
+ int error;

if (!dir || !S_ISDIR(dir->i_mode)) {
printk("nfs_unlink: inode is NULL or not a directory\n");
@@ -571,7 +562,28 @@
iput(dir);
return -ENAMETOOLONG;
}
- if ((error = nfs_sillyrename(dir, name, len)) < 0) {
+
+ dir->i_count++;
+ if (nfs_lookup(dir, name, len, &inode) < 0) {
+ error = -EIO;
+ goto end_unlink;
+ }
+ if (S_ISDIR(inode->i_mode)){
+ iput(inode);
+ iput(dir);
+ return -EPERM;
+ }
+ if (inode->i_count == 1 || NFS_RENAMED_DIR(inode)) {
+ iput(inode);
+ error = -EIO;
+ goto end_unlink;
+ }
+
+ error = nfs_sillyrename(dir, name, len, inode);
+ iput(inode);
+
+end_unlink:
+ if (error) {
error = nfs_proc_remove(NFS_SERVER(dir), NFS_FH(dir), name);
nfs_lookup_cache_remove(dir, NULL, name);
}

-- 
Andrew.
----------------------------------------------------------------------
Andrew M. Bishop                             amb@gedanken.demon.co.uk
                                      http://www.gedanken.demon.co.uk/