[1/2] EM28xx - fix race on disconnect

Commit Message

Chris Rankin Aug. 20, 2011, 11:42 a.m. UTC

*Sigh* I overlooked two patches in the original numbering...

This patch closes the race on the device and extension lists at USB disconnect 
time. Previously, the device was removed from the device list during 
em28xx_release_resources(), and then passed to the em28xx_close_extension() 
function so that all extensions could run their fini() operations. However, this 
left a (brief, theoretical, highly unlikely ;-)) window between these two calls 
during which a new module could call em28xx_register_extension(). The result 
would have been that the em28xx_usb_disconnect() function would also have passed 
the device to the new extension's fini() function, despite never having called 
the extension's init() function.

This patch also restores em28xx_close_extension()'s symmetry with 
em28xx_init_extension(), and establishes the property that every device in the 
device list must have been initialised for every extension in the extension list.

Signed-of-by: Chris Rankin <[e-mail]>

Comments

Sylwester Nawrocki Aug. 20, 2011, 12:36 p.m. UTC | #1

Hi Chris,

On 08/20/2011 01:42 PM, Chris Rankin wrote:
> *Sigh* I overlooked two patches in the original numbering...

How about using git format-patch and git send-email ?
http://www.kernel.org/pub/software/scm/git/docs/git-send-email.html

It's easier to review when patches are inlined rather than sent
as an attachment.

git format-patch will make you a nice series of patches. For instance,
when you have 3 of your patches applied on top of some branch and it's
checked out, to generate the patches the simple command:

$ git format-patch -3 --cover-letter

will do. Then you just need to pass the files to 'git send-email'.

Also, the patch description should be wrapped around 75 characters.
So there is no need for text wrapping with 'git log', etc.

Unfortunately git send-email won't work with free e-email accounts
on yahoo.com. The SMTP server throws an error like:
"Access denied : Free users cannot access this server."
Gmail works for me.

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig	2011-08-19 00:23:17.000000000 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c	2011-08-19 00:32:40.000000000 +0100
@@ -2738,9 +2738,9 @@ 
 #endif /* CONFIG_MODULES */
 
 /*
- * em28xx_realease_resources()
+ * em28xx_release_resources()
  * unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
+ * called when the device gets disconnected or at module unload
 */
 void em28xx_release_resources(struct em28xx *dev)
 {
@@ -2754,8 +2754,6 @@ 
 
 	em28xx_release_analog_resources(dev);
 
-	em28xx_remove_from_devlist(dev);
-
 	em28xx_i2c_unregister(dev);
 
 	v4l2_device_unregister(&dev->v4l2_dev);
@@ -3152,7 +3150,7 @@ 
 
 /*
  * em28xx_usb_disconnect()
- * called when the device gets diconencted
+ * called when the device gets disconnected
  * video device will be unregistered on v4l2_close in case it is still open
  */
 static void em28xx_usb_disconnect(struct usb_interface *interface)
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig	2011-08-18 23:07:51.000000000 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c	2011-08-19 00:27:00.000000000 +0100
@@ -1160,18 +1160,6 @@ 
 static DEFINE_MUTEX(em28xx_devlist_mutex);
 
 /*
- * em28xx_realease_resources()
- * unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
-*/
-void em28xx_remove_from_devlist(struct em28xx *dev)
-{
-	mutex_lock(&em28xx_devlist_mutex);
-	list_del(&dev->devlist);
-	mutex_unlock(&em28xx_devlist_mutex);
-};
-
-/*
  * Extension interface
  */
 
@@ -1221,14 +1209,13 @@ 
 
 void em28xx_close_extension(struct em28xx *dev)
 {
-	struct em28xx_ops *ops = NULL;
+	const struct em28xx_ops *ops = NULL;
 
 	mutex_lock(&em28xx_devlist_mutex);
-	if (!list_empty(&em28xx_extension_devlist)) {
-		list_for_each_entry(ops, &em28xx_extension_devlist, next) {
-			if (ops->fini)
-				ops->fini(dev);
-		}
+	list_for_each_entry(ops, &em28xx_extension_devlist, next) {
+		if (ops->fini)
+			ops->fini(dev);
 	}
+	list_del(&dev->devlist);
 	mutex_unlock(&em28xx_devlist_mutex);
 }