linux1394-cvslog

[SVN U. <dm...@li...>]

Author: dmaas
Date: 2002-08-07 04:09:13 -0400 (Wed, 07 Aug 2002)
New Revision: 540

Modified:
   trunk/ieee1394_core.c
Log:
make ieee1394_dispatch_open safer against module unload races
using try_inc_mod_count()


Modified: trunk/ieee1394_core.c
==============================================================================
--- trunk/ieee1394_core.c	(original)
+++ trunk/ieee1394_core.c	2002-08-07 04:09:14.000000000 -0400
@@ -862,19 +862,57 @@
 	write_unlock(&ieee1394_chardevs_lock);
 }
 
+/*
+  ieee1394_get_chardev() - look up and acquire a character device
+  driver that has previously registered using ieee1394_register_chardev()
+  
+  On success, returns 1 and sets module and file_ops to the driver.
+  The module will have an incremented reference count.
+   
+  On failure, returns 0.
+  The module will NOT have an incremented reference count.
+*/
+
+static int ieee1394_get_chardev(int blocknum,
+				struct module **module,
+				struct file_operations **file_ops)
+{
+	int ret = 0;
+       
+	if( (blocknum < 0) || (blocknum > 15) )
+		return ret;
+
+	read_lock(&ieee1394_chardevs_lock);
+
+	*module = ieee1394_chardevs[blocknum].module;
+	*file_ops = ieee1394_chardevs[blocknum].file_ops;
+
+	if(*file_ops == NULL)
+		goto out;
+
+	/* don't need try_inc_mod_count if the driver is non-modular */
+	if(*module && (try_inc_mod_count(*module) == 0))
+		goto out;
+
+	/* success! */
+	ret = 1;
+	
+out:
+	read_unlock(&ieee1394_chardevs_lock);
+	return ret;
+}
+
 /* the point of entry for open() on any ieee1394 character device */
 static int ieee1394_dispatch_open(struct inode *inode, struct file *file)
 {
 	struct file_operations *file_ops;
 	struct module *module;
 	int blocknum;
-	int retval = -ENODEV;
+	int retval;
 
 	/*
 	  Maintaining correct module reference counts is tricky here!
 
-	  For Linux v2.4 and later:
-	  
 	  The key thing to remember is that the VFS increments the
 	  reference count of ieee1394 before it calls
 	  ieee1394_dispatch_open().
@@ -887,16 +925,7 @@
 	  If the open() fails, then the VFS will drop the
 	  reference count of whatever module file->f_op->owner points
 	  to, immediately after this function returns.
-
-	  The comments below refer to the 2.4 case, since the 2.2
-	  case is trivial.
-	  
 	*/
-
-#define INCREF(mod_) do { struct module *mod = (struct module*) mod_; \
-                          if(mod != NULL) __MOD_INC_USE_COUNT(mod); } while(0)
-#define DECREF(mod_) do { struct module *mod = (struct module*) mod_; \
-                          if(mod != NULL) __MOD_DEC_USE_COUNT(mod); } while(0)
 	
         /* shift away lower four bits of the minor
 	   to get the index of the ieee1394_driver
@@ -904,20 +933,10 @@
 	
 	blocknum = (minor(inode->i_rdev) >> 4) & 0xF;
 
-	/* printk("ieee1394_dispatch_open(%d)", blocknum); */
+	/* look up the driver */
 
-	read_lock(&ieee1394_chardevs_lock);
-	module = ieee1394_chardevs[blocknum].module;
-	/* bump the reference count of the driver that
-	   will receive the open() */
-	INCREF(module);
-	file_ops = ieee1394_chardevs[blocknum].file_ops;
-	read_unlock(&ieee1394_chardevs_lock);
-
-	if(file_ops == NULL) {
-		DECREF(module);
-		goto out_fail;
-	}
+	if(ieee1394_get_chardev(blocknum, &module, &file_ops) == 0)
+		return -ENODEV;
 
 	/* redirect all subsequent requests to the driver's
 	   own file_operations */
@@ -929,42 +948,42 @@
 	/* follow through with the open() */
 	retval = file_ops->open(inode, file);
 
-	if(retval) {
+	if(retval == 0) {
 		
-		/* if the open() failed, then we need to drop the
-                   extra reference we gave to the task-specific
-                   driver */
+		/* If the open() succeeded, then ieee1394 will be left
+		   with an extra module reference, so we discard it here.
 
-		DECREF(module);
-		goto out_fail;
+		   The task-specific driver still has the extra
+		   reference given to it by ieee1394_get_chardev().
+		   This extra reference prevents the module from
+		   unloading while the file is open, and will be
+		   dropped by the VFS when the file is released.
+		*/
 		
-	} else {
-
-		/* if the open() succeeded, then ieee1394 will be left
-		   with an extra module reference, so we discard it here.*/
+		if(THIS_MODULE)
+			__MOD_DEC_USE_COUNT((struct module*) THIS_MODULE);
+		
+		/* note that if ieee1394 is compiled into the kernel,
+		   THIS_MODULE will be (void*) NULL, hence the if and
+		   the cast are necessary */
 
-		DECREF(THIS_MODULE);
+	} else {
 
-		/* the task-specific driver still has the extra
-		   reference we gave it. This extra reference prevents
-		   the module from unloading while the file is open,
-		   and will be dropped by the VFS when the file is
-		   released. */
+		/* if the open() failed, then we need to drop the
+		   extra reference we gave to the task-specific
+		   driver */
 		
-		return 0;
-	}
-	       
-out_fail:
-	/* point the file's f_ops back to ieee1394. The VFS will then
-	   decrement ieee1394's reference count immediately after this
-	   function returns. */
+		if(module)
+			__MOD_DEC_USE_COUNT(module);
 	
-	file->f_op = &ieee1394_chardev_ops;
-	return retval;
+		/* point the file's f_ops back to ieee1394. The VFS will then
+		   decrement ieee1394's reference count immediately after this
+		   function returns. */
+		
+		file->f_op = &ieee1394_chardev_ops;
+	}
 
-#undef INCREF
-#undef DECREF
-	     
+	return retval;
 }
 
 struct proc_dir_entry *ieee1394_procfs_entry;