Bugzilla – Attachment 272295 Details for
Bug 431192
kernel oops related to unusual USB device
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Forgot Password
[patch]
Patch fixing similarly looking problem in SLES10
usbfs-private-mutex-for-open-release-and-remove.patch (text/plain), 4.55 KB, created by
Jan Kara
on 2009-02-12 14:09:05 UTC
(
hide
)
Description:
Patch fixing similarly looking problem in SLES10
Filename:
MIME Type:
Creator:
Jan Kara
Created:
2009-02-12 14:09:05 UTC
Size:
4.55 KB
patch
obsolete
>From 4a2a8a2cce86b9d001378cc25acb5c61e6ca7d63 Mon Sep 17 00:00:00 2001 >From: Alan Stern <stern@rowland.harvard.edu> >Date: Sat, 1 Jul 2006 22:05:01 -0400 >Subject: usbfs: private mutex for open, release, and remove >References: bnc#420059 >Patch-mainline: 2.6.19 > > >From: Alan Stern <stern@rowland.harvard.edu> > >commit 4a2a8a2cce86b9d001378cc25acb5c61e6ca7d63 upstream. > >The usbfs code doesn't provide sufficient mutual exclusion among open, >release, and remove. Release vs. remove is okay because they both >acquire the device lock, but open is not exclusive with either one. All >three routines modify the udev->filelist linked list, so they must not >run concurrently. > >Apparently someone gave this a minimum amount of thought in the past by >explicitly acquiring the BKL at the start of the usbdev_open routine. >Oddly enough, there's a comment pointing out that locking is unnecessary >because chrdev_open already has acquired the BKL. > >But this ignores the point that the files in /proc/bus/usb/* are not >char device files; they are regular files and so they don't get any >special locking. Furthermore it's necessary to acquire the same lock in >the release and remove routines, which the code does not do. > >Yet another problem arises because the same file_operations structure is >accessible through both the /proc/bus/usb/* and /dev/usb/usbdev* file >nodes. Even when one of them has been removed, it's still possible for >userspace to open the other. So simple locking around the individual >remove routines is insufficient; we need to lock the entire >usb_notify_remove_device notifier chain. > >Rather than rely on the BKL, this patch (as723) introduces a new private >mutex for the purpose. Holding the BKL while invoking a notifier chain >doesn't seem like a good idea. > > >Signed-off-by: Alan Stern <stern@rowland.harvard.edu> >Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > >--- > drivers/usb/core/devio.c | 26 +++++++++++++++----------- > drivers/usb/core/notify.c | 3 +++ > drivers/usb/core/usb.h | 1 + > 3 files changed, 19 insertions(+), 11 deletions(-) > >--- a/drivers/usb/core/devio.c >+++ b/drivers/usb/core/devio.c >@@ -58,6 +58,9 @@ > #define USB_DEVICE_MAX USB_MAXBUS * 128 > static struct class *usb_device_class; > >+/* Mutual exclusion for removal, open, and release */ >+DEFINE_MUTEX(usbfs_mutex); >+ > struct async { > struct list_head asynclist; > struct dev_state *ps; >@@ -543,15 +546,13 @@ static int usbdev_open(struct inode *ino > struct dev_state *ps; > int ret; > >- /* >- * no locking necessary here, as chrdev_open has the kernel lock >- * (still acquire the kernel lock for safety) >- */ >+ /* Protect against simultaneous removal or release */ >+ mutex_lock(&usbfs_mutex); >+ > ret = -ENOMEM; > if (!(ps = kmalloc(sizeof(struct dev_state), GFP_KERNEL))) >- goto out_nolock; >+ goto out; > >- lock_kernel(); > ret = -ENOENT; > /* check if we are called from a real node or usbfs */ > if (imajor(inode) == USB_DEVICE_MAJOR) >@@ -580,9 +581,8 @@ static int usbdev_open(struct inode *ino > list_add_tail(&ps->list, &dev->filelist); > file->private_data = ps; > out: >- unlock_kernel(); >- out_nolock: >- return ret; >+ mutex_unlock(&usbfs_mutex); >+ return ret; > } > > static int usbdev_release(struct inode *inode, struct file *file) >@@ -592,7 +592,12 @@ static int usbdev_release(struct inode * > unsigned int ifnum; > > usb_lock_device(dev); >+ >+ /* Protect against simultaneous open */ >+ mutex_lock(&usbfs_mutex); > list_del_init(&ps->list); >+ mutex_unlock(&usbfs_mutex); >+ > for (ifnum = 0; ps->ifclaimed && ifnum < 8*sizeof(ps->ifclaimed); > ifnum++) { > if (test_bit(ifnum, &ps->ifclaimed)) >@@ -601,9 +606,8 @@ static int usbdev_release(struct inode * > destroy_all_async(ps); > usb_unlock_device(dev); > usb_put_dev(dev); >- ps->dev = NULL; > kfree(ps); >- return 0; >+ return 0; > } > > static int proc_control(struct dev_state *ps, void __user *arg) >--- a/drivers/usb/core/notify.c >+++ b/drivers/usb/core/notify.c >@@ -100,7 +100,10 @@ void usb_notify_add_device(struct usb_de > > void usb_notify_remove_device(struct usb_device *udev) > { >+ /* Protect against simultaneous usbfs open */ >+ mutex_lock(&usbfs_mutex); > usb_notifier_call_chain(&usb_notifier_list, USB_DEVICE_REMOVE, udev); >+ mutex_unlock(&usbfs_mutex); > } > > void usb_notify_add_bus(struct usb_bus *ubus) >--- a/drivers/usb/core/usb.h >+++ b/drivers/usb/core/usb.h >@@ -56,6 +56,7 @@ static inline int is_active(struct usb_i > extern const char *usbcore_name; > > /* usbfs stuff */ >+extern struct mutex usbfs_mutex; > extern struct usb_driver usbfs_driver; > extern struct file_operations usbfs_devices_fops; > extern struct file_operations usbfs_device_file_operations;
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
Actions:
View
|
Diff
Attachments on
bug 431192
:
242744
|
242756
|
244539
|
245617
| 272295