Commit 6cb4b040 authored by Jiri Kosina's avatar Jiri Kosina

HID: hiddev: fix race between hiddev_disconnect and hiddev_release

When hiddev_disconnect() runs with chardev open, it will proceed with
usbhid_close(). When userspace in parallel runs the hiddev_release(),
it sees !hiddev->exists (as it has been already set so by
hiddev_disconnect()) and kfrees hiddev while hiddev_disconnect() hasn't
finished yet.

Serialize the access to hiddev->exists and hiddev->open by existancelock.

Reported-by: mike-@cinci.rr.com
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent 437f3b19
...@@ -242,6 +242,7 @@ static int hiddev_release(struct inode * inode, struct file * file) ...@@ -242,6 +242,7 @@ static int hiddev_release(struct inode * inode, struct file * file)
list_del(&list->node); list_del(&list->node);
spin_unlock_irqrestore(&list->hiddev->list_lock, flags); spin_unlock_irqrestore(&list->hiddev->list_lock, flags);
mutex_lock(&list->hiddev->existancelock);
if (!--list->hiddev->open) { if (!--list->hiddev->open) {
if (list->hiddev->exist) { if (list->hiddev->exist) {
usbhid_close(list->hiddev->hid); usbhid_close(list->hiddev->hid);
...@@ -252,6 +253,7 @@ static int hiddev_release(struct inode * inode, struct file * file) ...@@ -252,6 +253,7 @@ static int hiddev_release(struct inode * inode, struct file * file)
} }
kfree(list); kfree(list);
mutex_unlock(&list->hiddev->existancelock);
return 0; return 0;
} }
...@@ -300,17 +302,21 @@ static int hiddev_open(struct inode *inode, struct file *file) ...@@ -300,17 +302,21 @@ static int hiddev_open(struct inode *inode, struct file *file)
list_add_tail(&list->node, &hiddev->list); list_add_tail(&list->node, &hiddev->list);
spin_unlock_irq(&list->hiddev->list_lock); spin_unlock_irq(&list->hiddev->list_lock);
mutex_lock(&hiddev->existancelock);
if (!list->hiddev->open++) if (!list->hiddev->open++)
if (list->hiddev->exist) { if (list->hiddev->exist) {
struct hid_device *hid = hiddev->hid; struct hid_device *hid = hiddev->hid;
res = usbhid_get_power(hid); res = usbhid_get_power(hid);
if (res < 0) { if (res < 0) {
res = -EIO; res = -EIO;
goto bail; goto bail_unlock;
} }
usbhid_open(hid); usbhid_open(hid);
} }
mutex_unlock(&hiddev->existancelock);
return 0; return 0;
bail_unlock:
mutex_unlock(&hiddev->existancelock);
bail: bail:
file->private_data = NULL; file->private_data = NULL;
kfree(list); kfree(list);
...@@ -911,7 +917,6 @@ void hiddev_disconnect(struct hid_device *hid) ...@@ -911,7 +917,6 @@ void hiddev_disconnect(struct hid_device *hid)
mutex_lock(&hiddev->existancelock); mutex_lock(&hiddev->existancelock);
hiddev->exist = 0; hiddev->exist = 0;
mutex_unlock(&hiddev->existancelock);
usb_deregister_dev(usbhid->intf, &hiddev_class); usb_deregister_dev(usbhid->intf, &hiddev_class);
...@@ -921,4 +926,5 @@ void hiddev_disconnect(struct hid_device *hid) ...@@ -921,4 +926,5 @@ void hiddev_disconnect(struct hid_device *hid)
} else { } else {
kfree(hiddev); kfree(hiddev);
} }
mutex_unlock(&hiddev->existancelock);
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment