From 65b01bd561dc995aab116aa784f97a37f7c49a65 Mon Sep 17 00:00:00 2001 From: James Hogan Date: Tue, 20 Sep 2011 15:23:46 +0200 Subject: [PATCH 1/6] 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: James Hogan Tested-by: David Herrmann Signed-off-by: Jiri Kosina --- drivers/hid/hidraw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c index c79578b5a788..a8c2b7b6220a 100644 --- a/drivers/hid/hidraw.c +++ b/drivers/hid/hidraw.c @@ -510,13 +510,12 @@ void hidraw_disconnect(struct hid_device *hid) { struct hidraw *hidraw = hid->hidraw; + mutex_lock(&minors_lock); hidraw->exist = 0; device_destroy(hidraw_class, MKDEV(hidraw_major, hidraw->minor)); - mutex_lock(&minors_lock); hidraw_table[hidraw->minor] = NULL; - mutex_unlock(&minors_lock); if (hidraw->open) { hid_hw_close(hid); @@ -524,6 +523,7 @@ void hidraw_disconnect(struct hid_device *hid) } else { kfree(hidraw); } + mutex_unlock(&minors_lock); } EXPORT_SYMBOL_GPL(hidraw_disconnect); From 9561f7faa45cb855b1ba83a4acf3f2ad3665e71f Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 23 Sep 2011 09:21:13 +0300 Subject: [PATCH 2/6] HID: hiddev: potential info leak in hiddev_ioctl() Smatch has a new check for Rosenberg type information leaks where structs are copied to the user with uninitialized stack data in them. In this case, the hiddev_devinfo struct has a two byte hole. struct hiddev_devinfo { __u32 bustype; /* 0 4 */ __u32 busnum; /* 4 4 */ __u32 devnum; /* 8 4 */ __u32 ifnum; /* 12 4 */ __s16 vendor; /* 16 2 */ __s16 product; /* 18 2 */ __s16 version; /* 20 2 */ /* XXX 2 bytes hole, try to pack */ __u32 num_applications; /* 24 4 */ Signed-off-by: Dan Carpenter Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hiddev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 7c1188b53c3e..4ef02b269a71 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -641,6 +641,8 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) struct usb_device *dev = hid_to_usb_dev(hid); struct usbhid_device *usbhid = hid->driver_data; + memset(&dinfo, 0, sizeof(dinfo)); + dinfo.bustype = BUS_USB; dinfo.busnum = dev->bus->busnum; dinfo.devnum = dev->devnum; From f554ff80339b4005856e6a86454d6ea2bb962ee5 Mon Sep 17 00:00:00 2001 From: Amit Nagal Date: Tue, 27 Sep 2011 13:41:58 -0400 Subject: [PATCH 3/6] HID: hidraw: open count should not increase if error In hidraw_open, if hid_hw_power returns with error, hidraw device open count should not increase. Signed-off-by: Amit Nagal Signed-off-by: Jiri Kosina --- drivers/hid/hidraw.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c index a8c2b7b6220a..6d65d4e35120 100644 --- a/drivers/hid/hidraw.c +++ b/drivers/hid/hidraw.c @@ -272,8 +272,10 @@ static int hidraw_open(struct inode *inode, struct file *file) dev = hidraw_table[minor]; if (!dev->open++) { err = hid_hw_power(dev->hid, PM_HINT_FULLON); - if (err < 0) + if (err < 0) { + dev->open--; goto out_unlock; + } err = hid_hw_open(dev->hid); if (err < 0) { From 3797ef6b6bc041755318917855d63879679c6dd9 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sat, 8 Oct 2011 23:20:17 +0200 Subject: [PATCH 4/6] HID: wacom: Set input bits before registration We shouldn't change the event flags of input devices after they get registered. Otherwise, udev will not get notified of these flags and cannot setup the devices properly. This fixes the probing to set the input event flags on the input_mapped callback instead of the probe function. Reported-by: Bastien Nocera Signed-off-by: David Herrmann Tested-by: Bastien Nocera Signed-off-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-wacom.c | 76 ++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c index a597039d0755..519f56c787bb 100644 --- a/drivers/hid/hid-wacom.c +++ b/drivers/hid/hid-wacom.c @@ -304,11 +304,49 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report, return 1; } +static int wacom_input_mapped(struct hid_device *hdev, struct hid_input *hi, + struct hid_field *field, struct hid_usage *usage, unsigned long **bit, + int *max) +{ + struct input_dev *input = hi->input; + + /* Basics */ + input->evbit[0] |= BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_REL); + + __set_bit(REL_WHEEL, input->relbit); + + __set_bit(BTN_TOOL_PEN, input->keybit); + __set_bit(BTN_TOUCH, input->keybit); + __set_bit(BTN_STYLUS, input->keybit); + __set_bit(BTN_STYLUS2, input->keybit); + __set_bit(BTN_LEFT, input->keybit); + __set_bit(BTN_RIGHT, input->keybit); + __set_bit(BTN_MIDDLE, input->keybit); + + /* Pad */ + input->evbit[0] |= BIT(EV_MSC); + + __set_bit(MSC_SERIAL, input->mscbit); + + __set_bit(BTN_0, input->keybit); + __set_bit(BTN_1, input->keybit); + __set_bit(BTN_TOOL_FINGER, input->keybit); + + /* Distance, rubber and mouse */ + __set_bit(BTN_TOOL_RUBBER, input->keybit); + __set_bit(BTN_TOOL_MOUSE, input->keybit); + + input_set_abs_params(input, ABS_X, 0, 16704, 4, 0); + input_set_abs_params(input, ABS_Y, 0, 12064, 4, 0); + input_set_abs_params(input, ABS_PRESSURE, 0, 511, 0, 0); + input_set_abs_params(input, ABS_DISTANCE, 0, 32, 0, 0); + + return 0; +} + static int wacom_probe(struct hid_device *hdev, const struct hid_device_id *id) { - struct hid_input *hidinput; - struct input_dev *input; struct wacom_data *wdata; int ret; @@ -370,39 +408,6 @@ static int wacom_probe(struct hid_device *hdev, goto err_ac; } #endif - hidinput = list_entry(hdev->inputs.next, struct hid_input, list); - input = hidinput->input; - - /* Basics */ - input->evbit[0] |= BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_REL); - - __set_bit(REL_WHEEL, input->relbit); - - __set_bit(BTN_TOOL_PEN, input->keybit); - __set_bit(BTN_TOUCH, input->keybit); - __set_bit(BTN_STYLUS, input->keybit); - __set_bit(BTN_STYLUS2, input->keybit); - __set_bit(BTN_LEFT, input->keybit); - __set_bit(BTN_RIGHT, input->keybit); - __set_bit(BTN_MIDDLE, input->keybit); - - /* Pad */ - input->evbit[0] |= BIT(EV_MSC); - - __set_bit(MSC_SERIAL, input->mscbit); - - __set_bit(BTN_0, input->keybit); - __set_bit(BTN_1, input->keybit); - __set_bit(BTN_TOOL_FINGER, input->keybit); - - /* Distance, rubber and mouse */ - __set_bit(BTN_TOOL_RUBBER, input->keybit); - __set_bit(BTN_TOOL_MOUSE, input->keybit); - - input_set_abs_params(input, ABS_X, 0, 16704, 4, 0); - input_set_abs_params(input, ABS_Y, 0, 12064, 4, 0); - input_set_abs_params(input, ABS_PRESSURE, 0, 511, 0, 0); - input_set_abs_params(input, ABS_DISTANCE, 0, 32, 0, 0); return 0; @@ -446,6 +451,7 @@ static struct hid_driver wacom_driver = { .probe = wacom_probe, .remove = wacom_remove, .raw_event = wacom_raw_event, + .input_mapped = wacom_input_mapped, }; static int __init wacom_init(void) From fad9fbe8651e8abd9794d4b4c4133241aa3093b5 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Thu, 13 Oct 2011 18:21:58 +0200 Subject: [PATCH 5/6] HID: usbhid: cancel timer for retry synchronously This makes sure IO is never restarted while a reset is going on In particular there seems to be no protection from hid_retry_timeout() calling hid_start_in() which would start IO after hid_pre_reset() has already called hid_cease_io() because that uses del_timer(), not del_timer_sync() Signed-off-by: Oliver Neukum Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index ad978f5748d3..77e705c2209c 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1270,7 +1270,7 @@ static void hid_cancel_delayed_stuff(struct usbhid_device *usbhid) static void hid_cease_io(struct usbhid_device *usbhid) { - del_timer(&usbhid->io_retry); + del_timer_sync(&usbhid->io_retry); usb_kill_urb(usbhid->urbin); usb_kill_urb(usbhid->urbctrl); usb_kill_urb(usbhid->urbout); From bca621421c53caf73f36e181d6e5fe41fe0da7a7 Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Fri, 14 Oct 2011 13:39:34 +0800 Subject: [PATCH 6/6] HID: hid-magicmouse: Magic Trackpad has 1 button, not 2 hid-magicmouse was advertising the Apple Magic Trackpad as having 2 buttons (left and right) when it actually only has 1 button. Advertising multiple buttons makes Xorg disable all button 2 and 3 emulation (using multi-finger clicks). So Xorg users don't get working right/middle-click emulation out of the box. This patch makes hid-magicmouse correctly only report one real button for Magic Trackpad, which in turn makes Xorg enable multi-finger click support to emulate right/middle buttons. [http://launchpad.net/bugs/862094] Signed-off-by: Daniel van Vugt Reviewed-by: Chase Douglas Signed-off-by: Jiri Kosina --- drivers/hid/hid-magicmouse.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index f0fbd7bd239e..2ab71758e2e2 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -405,6 +405,13 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h __set_bit(REL_HWHEEL, input->relbit); } } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */ + /* input->keybit is initialized with incorrect button info + * for Magic Trackpad. There really is only one physical + * button (BTN_LEFT == BTN_MOUSE). Make sure we don't + * advertise buttons that don't exist... + */ + __clear_bit(BTN_RIGHT, input->keybit); + __clear_bit(BTN_MIDDLE, input->keybit); __set_bit(BTN_MOUSE, input->keybit); __set_bit(BTN_TOOL_FINGER, input->keybit); __set_bit(BTN_TOOL_DOUBLETAP, input->keybit);