Messages in this thread Patch in this message |  | | Date | Fri, 15 Nov 1996 19:18:02 GMT | From | "Andrew M. Bishop" <> | Subject | unlink system call on directories - Bug + fix |
| |
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/
|  |