2000-11-17 07:44:38

by Alexander Viro

[permalink] [raw]
Subject: [PATCH] fs_may_remount_ro()/fput() race fix and ->f_dentry cleanups

* fs_may_remount_ro() used the wrong check for skipping the
files being in the middle of final fput(). NULL ->f_dentry is
possible here (after all references to file are gone, etc.), NULL
->f_dentry->d_inode isn't. I.e. check in
file_list_lock();
for (p = sb->s_files.next; p != &sb->s_files; p = p->next) {
struct file *file = list_entry(p, struct file, f_list);
struct inode *inode = file->f_dentry->d_inode;
if (!inode)
continue;
....
is not only useless, it's an oopsable SMP race.
Fix: check for the right thing.

* when we own the reference on opened file neither ->f_dentry nor
->f_dentry->d_inode may be NULL. Majority of bogus checks had been killed
back in Spring, but some had crawled back. Exterminated.

* if foo_readdir() occurs only in foo_dir_operations and
foo_dir_operations is assigned only to ->i_fops of directories, it's
pointless to check whether the passed file is a directory. It is. Honest.
Again, most of that junk had been killed, but some creatures had resurfaced.
Exterminated.

* ->readdir() never gets a file with negative dentry. Ditto for
->read() and ->write(). Same story.

Patch follows. Please, apply it.
Cheers,
Al

diff -urN rc11-pre6/arch/ia64/ia32/sys_ia32.c rc11-6-f_dentry/arch/ia64/ia32/sys_ia32.c
--- rc11-pre6/arch/ia64/ia32/sys_ia32.c Thu Nov 2 22:38:13 2000
+++ rc11-6-f_dentry/arch/ia64/ia32/sys_ia32.c Fri Nov 17 04:19:20 2000
@@ -3392,7 +3392,7 @@
}

inode = file->f_dentry->d_inode;
- if (!inode || !inode->i_sock || !socki_lookup(inode))
+ if (!inode->i_sock || !socki_lookup(inode))
{
*err = -ENOTSOCK;
fput(file);
diff -urN rc11-pre6/arch/sparc64/kernel/sys_sunos32.c rc11-6-f_dentry/arch/sparc64/kernel/sys_sunos32.c
--- rc11-pre6/arch/sparc64/kernel/sys_sunos32.c Mon Aug 28 21:14:42 2000
+++ rc11-6-f_dentry/arch/sparc64/kernel/sys_sunos32.c Fri Nov 17 04:22:08 2000
@@ -601,7 +601,6 @@
int try_port;
int ret;
struct socket *socket;
- struct dentry *dentry;
struct inode *inode;
struct file *file;

@@ -609,8 +608,7 @@
if(!file)
return 0;

- dentry = file->f_dentry;
- inode = dentry->d_inode;
+ inode = file->f_dentry->d_inode;

socket = &inode->u.socket_i;
local.sin_family = AF_INET;
diff -urN rc11-pre6/arch/sparc64/solaris/ioctl.c rc11-6-f_dentry/arch/sparc64/solaris/ioctl.c
--- rc11-pre6/arch/sparc64/solaris/ioctl.c Sat Aug 5 05:50:26 2000
+++ rc11-6-f_dentry/arch/sparc64/solaris/ioctl.c Fri Nov 17 04:23:05 2000
@@ -464,8 +464,8 @@
struct sol_socket_struct *sock;
struct module_info *mi;

- if (! (ino = filp->f_dentry->d_inode) ||
- ! ino->i_sock)
+ ino = filp->f_dentry->d_inode;
+ if (! ino->i_sock)
return -EBADF;
sock = filp->private_data;
if (! sock) {
diff -urN rc11-pre6/arch/sparc64/solaris/socket.c rc11-6-f_dentry/arch/sparc64/solaris/socket.c
--- rc11-pre6/arch/sparc64/solaris/socket.c Mon Aug 28 21:14:42 2000
+++ rc11-6-f_dentry/arch/sparc64/solaris/socket.c Fri Nov 17 04:23:17 2000
@@ -265,7 +265,7 @@
}

inode = file->f_dentry->d_inode;
- if (!inode || !inode->i_sock || !socki_lookup(inode)) {
+ if (!inode->i_sock || !socki_lookup(inode)) {
*err = -ENOTSOCK;
fput(file);
return NULL;
diff -urN rc11-pre6/drivers/block/loop.c rc11-6-f_dentry/drivers/block/loop.c
--- rc11-pre6/drivers/block/loop.c Fri Nov 17 02:22:37 2000
+++ rc11-6-f_dentry/drivers/block/loop.c Fri Nov 17 04:24:30 2000
@@ -411,10 +411,6 @@

error = -EINVAL;
inode = file->f_dentry->d_inode;
- if (!inode) {
- printk(KERN_ERR "loop_set_fd: NULL inode?!?\n");
- goto out_putf;
- }

if (S_ISBLK(inode->i_mode)) {
/* dentry will be wired, so... */
diff -urN rc11-pre6/fs/devfs/base.c rc11-6-f_dentry/fs/devfs/base.c
--- rc11-pre6/fs/devfs/base.c Fri Nov 17 02:23:19 2000
+++ rc11-6-f_dentry/fs/devfs/base.c Fri Nov 17 05:32:45 2000
@@ -2416,11 +2416,6 @@
struct devfs_entry *parent, *de;
struct inode *inode = file->f_dentry->d_inode;

- if (inode == NULL)
- {
- printk ("%s: readdir(): NULL inode\n", DEVFS_NAME);
- return -EBADF;
- }
if ( !S_ISDIR (inode->i_mode) )
{
printk ("%s: readdir(): inode is not a directory\n", DEVFS_NAME);
diff -urN rc11-pre6/fs/dquot.c rc11-6-f_dentry/fs/dquot.c
--- rc11-pre6/fs/dquot.c Wed Oct 4 03:44:53 2000
+++ rc11-6-f_dentry/fs/dquot.c Fri Nov 17 04:33:23 2000
@@ -679,8 +679,6 @@
if (!filp->f_dentry)
continue;
inode = filp->f_dentry->d_inode;
- if (!inode)
- continue;
if (filp->f_mode & FMODE_WRITE && dqinit_needed(inode, type)) {
file_list_unlock();
sb->dq_op->initialize(inode, type);
diff -urN rc11-pre6/fs/efs/dir.c rc11-6-f_dentry/fs/efs/dir.c
--- rc11-pre6/fs/efs/dir.c Mon Aug 28 21:15:15 2000
+++ rc11-6-f_dentry/fs/efs/dir.c Fri Nov 17 04:42:38 2000
@@ -28,9 +28,6 @@
int slot, namelen;
char *nameptr;

- if (!inode || !S_ISDIR(inode->i_mode))
- return -EBADF;
-
if (inode->i_size & (EFS_DIRBSIZE-1))
printk(KERN_WARNING "EFS: WARNING: readdir(): directory size not a multiple of EFS_DIRBSIZE\n");

diff -urN rc11-pre6/fs/file_table.c rc11-6-f_dentry/fs/file_table.c
--- rc11-pre6/fs/file_table.c Wed Oct 4 03:44:54 2000
+++ rc11-6-f_dentry/fs/file_table.c Fri Nov 17 04:38:17 2000
@@ -176,9 +176,12 @@
file_list_lock();
for (p = sb->s_files.next; p != &sb->s_files; p = p->next) {
struct file *file = list_entry(p, struct file, f_list);
- struct inode *inode = file->f_dentry->d_inode;
- if (!inode)
+ struct inode *inode;
+
+ if (!file->f_dentry)
continue;
+
+ inode = file->f_dentry->d_inode;

/* File with pending delete? */
if (inode->i_nlink == 0)
diff -urN rc11-pre6/fs/ncpfs/file.c rc11-6-f_dentry/fs/ncpfs/file.c
--- rc11-pre6/fs/ncpfs/file.c Wed Oct 4 03:44:55 2000
+++ rc11-6-f_dentry/fs/ncpfs/file.c Fri Nov 17 04:45:39 2000
@@ -120,11 +120,6 @@
DPRINTK("ncp_file_read: enter %s/%s\n",
dentry->d_parent->d_name.name, dentry->d_name.name);

- error = -EINVAL;
- if (inode == NULL) {
- DPRINTK("ncp_file_read: inode = NULL\n");
- goto out;
- }
error = -EIO;
if (!ncp_conn_valid(NCP_SERVER(inode)))
goto out;
@@ -210,10 +205,6 @@

DPRINTK("ncp_file_write: enter %s/%s\n",
dentry->d_parent->d_name.name, dentry->d_name.name);
- if (inode == NULL) {
- DPRINTK("ncp_file_write: inode = NULL\n");
- return -EINVAL;
- }
errno = -EIO;
if (!ncp_conn_valid(NCP_SERVER(inode)))
goto out;
diff -urN rc11-pre6/fs/proc/generic.c rc11-6-f_dentry/fs/proc/generic.c
--- rc11-pre6/fs/proc/generic.c Wed Oct 4 03:44:57 2000
+++ rc11-6-f_dentry/fs/proc/generic.c Fri Nov 17 04:55:55 2000
@@ -396,8 +396,6 @@
if (dentry->d_op != &proc_dentry_operations)
continue;
inode = dentry->d_inode;
- if (!inode)
- continue;
if (inode->u.generic_ip != de)
continue;
fops_put(filp->f_op);
diff -urN rc11-pre6/fs/udf/dir.c rc11-6-f_dentry/fs/udf/dir.c
--- rc11-pre6/fs/udf/dir.c Mon Aug 28 21:15:19 2000
+++ rc11-6-f_dentry/fs/udf/dir.c Fri Nov 17 04:54:52 2000
@@ -86,12 +86,6 @@
struct inode *dir = filp->f_dentry->d_inode;
int result;

- if (!dir)
- return -EBADF;
-
- if (!S_ISDIR(dir->i_mode))
- return -ENOTDIR;
-
if ( filp->f_pos == 0 )
{
if (filldir(dirent, ".", 1, filp->f_pos, dir->i_ino, DT_DIR))
diff -urN rc11-pre6/mm/filemap.c rc11-6-f_dentry/mm/filemap.c
--- rc11-pre6/mm/filemap.c Fri Nov 17 02:23:30 2000
+++ rc11-6-f_dentry/mm/filemap.c Fri Nov 17 04:53:20 2000
@@ -1208,8 +1208,6 @@
if (!out_file->f_op || !out_file->f_op->write)
goto fput_out;
out_inode = out_file->f_dentry->d_inode;
- if (!out_inode)
- goto fput_out;
retval = locks_verify_area(FLOCK_VERIFY_WRITE, out_inode, out_file, out_file->f_pos, count);
if (retval)
goto fput_out;
diff -urN rc11-pre6/net/khttpd/datasending.c rc11-6-f_dentry/net/khttpd/datasending.c
--- rc11-pre6/net/khttpd/datasending.c Fri Nov 17 02:23:36 2000
+++ rc11-6-f_dentry/net/khttpd/datasending.c Fri Nov 17 04:51:53 2000
@@ -114,7 +114,7 @@

inode = CurrentRequest->filp->f_dentry->d_inode;

- if (inode && inode->i_mapping->a_ops->readpage) {
+ if (inode->i_mapping->a_ops->readpage) {
/* This does the actual transfer using sendfile */
read_descriptor_t desc;
loff_t *ppos;
diff -urN rc11-pre6/net/socket.c rc11-6-f_dentry/net/socket.c
--- rc11-pre6/net/socket.c Fri Nov 17 02:23:36 2000
+++ rc11-6-f_dentry/net/socket.c Fri Nov 17 04:51:17 2000
@@ -405,7 +405,7 @@
}

inode = file->f_dentry->d_inode;
- if (!inode || !inode->i_sock || !(sock = socki_lookup(inode)))
+ if (!inode->i_sock || !(sock = socki_lookup(inode)))
{
*err = -ENOTSOCK;
fput(file);
diff -urN rc11-pre6/net/unix/garbage.c rc11-6-f_dentry/net/unix/garbage.c
--- rc11-pre6/net/unix/garbage.c Mon Jul 31 16:30:21 2000
+++ rc11-6-f_dentry/net/unix/garbage.c Fri Nov 17 04:52:37 2000
@@ -100,7 +100,7 @@
/*
* Socket ?
*/
- if (inode && inode->i_sock) {
+ if (inode->i_sock) {
struct socket * sock = &inode->u.socket_i;
struct sock * s = sock->sk;