[PATCH] loop device fixes

From: Andries Brouwer (aeb@veritas.com)
Date: Mon May 15 2000 - 16:41:26 EST


Just tried losetup and losetup -d under 2.3.99pre8 and
got a kernel crash. Looking at loop.c I see that the
refcounting is not done correctly. Below an improvement.
(And a few fixed typos.)

The single-line version of this is the mntget() in
        lo->lo_backing_file->f_vfsmnt = mntget(file->f_vfsmnt);

Andries

diff -u --recursive --new-file ../linux-2.3.99p8/linux/Documentation/Configure.help ./linux/Documentation/Configure.help
--- ../linux-2.3.99p8/linux/Documentation/Configure.help Sat May 13 03:19:45 2000
+++ ./linux/Documentation/Configure.help Sat May 13 03:25:16 2000
@@ -10430,7 +10430,7 @@
 Provide NFSv3 server support
 CONFIG_NFSD_V3
   If you would like to include the NFSv3 server as well as the NFSv2
- server, say Y here. In unsure, say Y.
+ server, say Y here. If unsure, say Y.
 
 OS/2 HPFS file system support
 CONFIG_HPFS_FS
diff -u --recursive --new-file ../linux-2.3.99p8/linux/drivers/block/loop.c ./linux/drivers/block/loop.c
--- ../linux-2.3.99p8/linux/drivers/block/loop.c Sat May 13 03:18:23 2000
+++ ./linux/drivers/block/loop.c Mon May 15 23:00:57 2000
@@ -289,7 +289,8 @@
                 if (lo->lo_flags & LO_FLAGS_READ_ONLY)
                         goto error_out;
         } else if (current_request->cmd != READ) {
- printk(KERN_ERR "unknown loop device command (%d)?!?", current_request->cmd);
+ printk(KERN_ERR "unknown loop device command (%d)?!?",
+ current_request->cmd);
                 goto error_out;
         }
 
@@ -423,8 +424,28 @@
                 /* Backed by a block device - don't need to hold onto
                    a file structure */
                 lo->lo_backing_file = NULL;
+
+ if (error)
+ goto out_putf;
         } else if (S_ISREG(inode->i_mode)) {
                 struct address_space_operations *aops;
+
+ aops = inode->i_mapping->a_ops;
+ /*
+ * If we can't read - sorry. If we only can't write - well,
+ * it's going to be read-only.
+ */
+ error = -EINVAL;
+ if (!aops->readpage)
+ goto out_putf;
+
+ if (!aops->prepare_write || !aops->commit_write)
+ lo->lo_flags |= LO_FLAGS_READ_ONLY;
+
+ error = get_write_access(inode);
+ if (error)
+ goto out_putf;
+
                 /* Backed by a regular file - we need to hold onto a file
                    structure for this file. Friggin' NFS can't live without
                    it on write and for reading we use do_generic_file_read(),
@@ -437,35 +458,23 @@
 
                 error = -ENFILE;
                 lo->lo_backing_file = get_empty_filp();
- if (lo->lo_backing_file) {
- lo->lo_backing_file->f_mode = file->f_mode;
- lo->lo_backing_file->f_pos = file->f_pos;
- lo->lo_backing_file->f_flags = file->f_flags;
- lo->lo_backing_file->f_owner = file->f_owner;
- lo->lo_backing_file->f_dentry = file->f_dentry;
- lo->lo_backing_file->f_vfsmnt = file->f_vfsmnt;
- lo->lo_backing_file->f_op = file->f_op;
- lo->lo_backing_file->private_data = file->private_data;
- file_moveto(lo->lo_backing_file, file);
-
- error = get_write_access(inode);
- if (error) {
- put_filp(lo->lo_backing_file);
- lo->lo_backing_file = NULL;
- }
- }
- aops = inode->i_mapping->a_ops;
- /*
- * If we can't read - sorry. If we only can't write - well,
- * it's going to be read-only.
- */
- if (!aops->readpage)
- error = -EINVAL;
- else if (!aops->prepare_write || !aops->commit_write)
- lo->lo_flags |= LO_FLAGS_READ_ONLY;
+ if (lo->lo_backing_file == NULL) {
+ put_write_access(inode);
+ goto out_putf;
+ }
+
+ lo->lo_backing_file->f_mode = file->f_mode;
+ lo->lo_backing_file->f_pos = file->f_pos;
+ lo->lo_backing_file->f_flags = file->f_flags;
+ lo->lo_backing_file->f_owner = file->f_owner;
+ lo->lo_backing_file->f_dentry = file->f_dentry;
+ lo->lo_backing_file->f_vfsmnt = mntget(file->f_vfsmnt);
+ lo->lo_backing_file->f_op = file->f_op;
+ lo->lo_backing_file->private_data = file->private_data;
+ file_moveto(lo->lo_backing_file, file);
+
+ error = 0;
         }
- if (error)
- goto out_putf;
 
         if (IS_RDONLY (inode) || is_read_only(lo->lo_device))
                 lo->lo_flags |= LO_FLAGS_READ_ONLY;
@@ -477,9 +486,9 @@
         lo->ioctl = NULL;
         figure_loop_size(lo);
 
-out_putf:
+ out_putf:
         fput(file);
-out:
+ out:
         if (error)
                 MOD_DEC_USE_COUNT;
         return error;
@@ -530,6 +539,7 @@
         lo->lo_dentry = NULL;
 
         if (lo->lo_backing_file != NULL) {
+ put_write_access(lo->lo_backing_file->f_dentry->d_inode);
                 fput(lo->lo_backing_file);
                 lo->lo_backing_file = NULL;
         } else {
diff -u --recursive --new-file ../linux-2.3.99p8/linux/drivers/net/setup.c ./linux/drivers/net/setup.c
--- ../linux-2.3.99p8/linux/drivers/net/setup.c Sun Apr 30 21:49:26 2000
+++ ./linux/drivers/net/setup.c Sun Apr 30 21:50:14 2000
@@ -34,7 +34,7 @@
 extern int madgemc_probe(void);
 extern int tms_pci_probe(void);
 
-/* Pad device name to IFNAMSIZ=16. F.e. __PAD6 is tring of 9 zeros. */
+/* Pad device name to IFNAMSIZ=16. F.e. __PAD6 is string of 9 zeros. */
 #define __PAD6 "\0\0\0\0\0\0\0\0\0"
 #define __PAD5 __PAD6 "\0"
 #define __PAD4 __PAD5 "\0"
diff -u --recursive --new-file ../linux-2.3.99p8/linux/fs/dcache.c ./linux/fs/dcache.c
--- ../linux-2.3.99p8/linux/fs/dcache.c Sat May 13 03:18:41 2000
+++ ./linux/fs/dcache.c Sat May 13 03:21:16 2000
@@ -788,7 +788,7 @@
  * Note that we have to be a lot more careful about getting the hash
  * switched - we have to switch the hash value properly even if it
  * then no longer matches the actual (corrupted) string of the target.
- * The has value has to match the hash queue that the dentry is on..
+ * The hash value has to match the hash queue that the dentry is on..
  */
 static inline void switch_names(struct dentry * dentry, struct dentry * target)
 {
diff -u --recursive --new-file ../linux-2.3.99p8/linux/fs/inode.c ./linux/fs/inode.c
--- ../linux-2.3.99p8/linux/fs/inode.c Sat May 13 03:18:42 2000
+++ ./linux/fs/inode.c Mon May 15 22:59:38 2000
@@ -377,7 +377,7 @@
  *
  * Discard all of the inodes for a given superblock. If the discard
  * fails because there are busy inodes then a non zero value is returned.
- * If the discard is successful all the inodes are dicarded.
+ * If the discard is successful all the inodes have been discarded.
  */
  
 int invalidate_inodes(struct super_block * sb)
@@ -470,7 +470,7 @@
 /*
  * Called with the inode lock held.
  * NOTE: we are not increasing the inode-refcount, you must call __iget()
- * by hand after calling find_inode now! This simplify iunique and won't
+ * by hand after calling find_inode now! This simplifies iunique and won't
  * add any additional branch in the common code.
  */
 static struct inode * find_inode(struct super_block * sb, unsigned long ino, struct list_head *head, find_inode_t find_actor, void *opaque)

-
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/



This archive was generated by hypermail 2b29 : Mon May 15 2000 - 21:00:26 EST