Commit 65b01bd5 authored by James Hogan's avatar James Hogan Committed by Jiri Kosina

HID: hidraw: protect hidraw_disconnect() better

The function hidraw_disconnect() only acquires the hidraw minors_lock
when clearing the entry in hidraw_table. However the device_destroy()
call can cause a userland read/write to return with an error. It may
cause the program to release the file descripter before the disconnect
is finished. hidraw_disconnect() has already set hidraw->exist to 0,
which makes hidraw_release() kfree the hidraw structure, which
hidraw_disconnect() continues to access and even tries to kfree again.
Similarly if a hidraw_release() occurs after setting hidraw->exist to 0,
the same thing can happen.

This is fixed by expanding the mutex critical section to cover the whole
function from setting hidraw->exist to 0 to freeing the hidraw
structure, preventing a hidraw_release() from interfering.
Signed-off-by: default avatarJames Hogan <james.hogan@imgtec.com>
Tested-by: default avatarDavid Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent e4e436e0
...@@ -510,13 +510,12 @@ void hidraw_disconnect(struct hid_device *hid) ...@@ -510,13 +510,12 @@ void hidraw_disconnect(struct hid_device *hid)
{ {
struct hidraw *hidraw = hid->hidraw; struct hidraw *hidraw = hid->hidraw;
mutex_lock(&minors_lock);
hidraw->exist = 0; hidraw->exist = 0;
device_destroy(hidraw_class, MKDEV(hidraw_major, hidraw->minor)); device_destroy(hidraw_class, MKDEV(hidraw_major, hidraw->minor));
mutex_lock(&minors_lock);
hidraw_table[hidraw->minor] = NULL; hidraw_table[hidraw->minor] = NULL;
mutex_unlock(&minors_lock);
if (hidraw->open) { if (hidraw->open) {
hid_hw_close(hid); hid_hw_close(hid);
...@@ -524,6 +523,7 @@ void hidraw_disconnect(struct hid_device *hid) ...@@ -524,6 +523,7 @@ void hidraw_disconnect(struct hid_device *hid)
} else { } else {
kfree(hidraw); kfree(hidraw);
} }
mutex_unlock(&minors_lock);
} }
EXPORT_SYMBOL_GPL(hidraw_disconnect); EXPORT_SYMBOL_GPL(hidraw_disconnect);
......
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