From e90a6df80dc45ab53d2f4f4db297434e48c0208e Mon Sep 17 00:00:00 2001 From: Henrik Rydberg Date: Mon, 25 Feb 2013 11:31:43 +0100 Subject: HID: Extend the interface with report requests Some drivers send reports directly to underlying device, creating an unwanted dependency on the underlying transport layer. This patch adds hid_hw_request() to the interface, thereby removing usbhid from the lion share of the drivers. Signed-off-by: Henrik Rydberg Signed-off-by: Benjamin Tissoires Reviewed-by: Mika Westerberg Signed-off-by: Jiri Kosina --- include/linux/hid.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'include') diff --git a/include/linux/hid.h b/include/linux/hid.h index e14b465b1146..261c713d4842 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -662,6 +662,7 @@ struct hid_driver { * @hidinput_input_event: event input event (e.g. ff or leds) * @parse: this method is called only once to parse the device data, * shouldn't allocate anything to not leak memory + * @request: send report request to device (e.g. feature report) */ struct hid_ll_driver { int (*start)(struct hid_device *hdev); @@ -676,6 +677,10 @@ struct hid_ll_driver { unsigned int code, int value); int (*parse)(struct hid_device *hdev); + + void (*request)(struct hid_device *hdev, + struct hid_report *report, int reqtype); + }; #define PM_HINT_FULLON 1<<5 @@ -883,6 +888,21 @@ static inline int hid_hw_power(struct hid_device *hdev, int level) return hdev->ll_driver->power ? hdev->ll_driver->power(hdev, level) : 0; } + +/** + * hid_hw_request - send report request to device + * + * @hdev: hid device + * @report: report to send + * @reqtype: hid request type + */ +static inline void hid_hw_request(struct hid_device *hdev, + struct hid_report *report, int reqtype) +{ + if (hdev->ll_driver->request) + hdev->ll_driver->request(hdev, report, reqtype); +} + int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, int interrupt); -- cgit v1.2.3 From 3373443befa73ee60e4275e7699b26058b01455a Mon Sep 17 00:00:00 2001 From: Henrik Rydberg Date: Mon, 25 Feb 2013 11:31:44 +0100 Subject: HID: Extend the interface with wait io request Some drivers need to wait for an io from the underlying device, creating an unwanted dependency on the underlying transport layer. This patch adds wait() to the interface, thereby removing usbhid from the lion share of the drivers. Signed-off-by: Henrik Rydberg Signed-off-by: Benjamin Tissoires Reviewed-by: Mika Westerberg Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 1 + include/linux/hid.h | 14 ++++++++++++++ 2 files changed, 15 insertions(+) (limited to 'include') diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 366fd09d257d..99d95d3368b5 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1264,6 +1264,7 @@ static struct hid_ll_driver usb_hid_driver = { .power = usbhid_power, .hidinput_input_event = usb_hidinput_input_event, .request = usbhid_request, + .wait = usbhid_wait_io, }; static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) diff --git a/include/linux/hid.h b/include/linux/hid.h index 261c713d4842..7071eb3d36c7 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -663,6 +663,7 @@ struct hid_driver { * @parse: this method is called only once to parse the device data, * shouldn't allocate anything to not leak memory * @request: send report request to device (e.g. feature report) + * @wait: wait for buffered io to complete (send/recv reports) */ struct hid_ll_driver { int (*start)(struct hid_device *hdev); @@ -681,6 +682,8 @@ struct hid_ll_driver { void (*request)(struct hid_device *hdev, struct hid_report *report, int reqtype); + int (*wait)(struct hid_device *hdev); + }; #define PM_HINT_FULLON 1<<5 @@ -903,6 +906,17 @@ static inline void hid_hw_request(struct hid_device *hdev, hdev->ll_driver->request(hdev, report, reqtype); } +/** + * hid_hw_wait - wait for buffered io to complete + * + * @hdev: hid device + */ +static inline void hid_hw_wait(struct hid_device *hdev) +{ + if (hdev->ll_driver->wait) + hdev->ll_driver->wait(hdev); +} + int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, int interrupt); -- cgit v1.2.3 From c849a6143bec520aff2a6646518b0d041402428b Mon Sep 17 00:00:00 2001 From: Andrew de los Reyes Date: Mon, 18 Feb 2013 09:20:21 -0800 Subject: HID: Separate struct hid_device's driver_lock into two locks. This patch separates struct hid_device's driver_lock into two. The goal is to allow hid device drivers to receive input during their probe() or remove() function calls. This is necessary because some drivers need to communicate with the device to determine parameters needed during probe (e.g., size of a multi-touch surface), and if possible, may perfer to communicate with a device on host-initiated disconnect (e.g., to put it into a low-power state). Historically, three functions used driver_lock: - hid_device_probe: blocks to acquire lock - hid_device_remove: blocks to acquire lock - hid_input_report: if locked returns -EBUSY, else acquires lock This patch adds another lock (driver_input_lock) which is used to block input from occurring. The lock behavior is now: - hid_device_probe: blocks to acq. driver_lock, then driver_input_lock - hid_device_remove: blocks to acq. driver_lock, then driver_input_lock - hid_input_report: if driver_input_lock locked returns -EBUSY, else acquires driver_input_lock This patch also adds two helper functions to be called during probe() or remove(): hid_device_io_start() and hid_device_io_stop(). These functions lock and unlock, respectively, driver_input_lock; they also make a note of whether they did so that hid-core knows if a driver has changed the lock state. This patch results in no behavior change for existing devices and drivers. However, during a probe() or remove() function call in a driver, that driver may now selectively call hid_device_io_start() to let input events come through, then optionally call hid_device_io_stop() to stop them. Signed-off-by: Andrew de los Reyes Signed-off-by: Jiri Kosina --- drivers/hid/hid-core.c | 24 +++++++++++++++++++++--- include/linux/hid.h | 46 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index ff75cabf7393..680068c0c46a 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1267,7 +1267,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i if (!hid) return -ENODEV; - if (down_trylock(&hid->driver_lock)) + if (down_trylock(&hid->driver_input_lock)) return -EBUSY; if (!hid->driver) { @@ -1324,7 +1324,7 @@ nomem: ret = hid_report_raw_event(hid, type, data, size, interrupt); unlock: - up(&hid->driver_lock); + up(&hid->driver_input_lock); return ret; } EXPORT_SYMBOL_GPL(hid_input_report); @@ -1845,6 +1845,11 @@ static int hid_device_probe(struct device *dev) if (down_interruptible(&hdev->driver_lock)) return -EINTR; + if (down_interruptible(&hdev->driver_input_lock)) { + ret = -EINTR; + goto unlock_driver_lock; + } + hdev->io_started = false; if (!hdev->driver) { id = hid_match_device(hdev, hdrv); @@ -1867,6 +1872,9 @@ static int hid_device_probe(struct device *dev) } } unlock: + if (!hdev->io_started) + up(&hdev->driver_input_lock); +unlock_driver_lock: up(&hdev->driver_lock); return ret; } @@ -1875,9 +1883,15 @@ static int hid_device_remove(struct device *dev) { struct hid_device *hdev = container_of(dev, struct hid_device, dev); struct hid_driver *hdrv; + int ret = 0; if (down_interruptible(&hdev->driver_lock)) return -EINTR; + if (down_interruptible(&hdev->driver_input_lock)) { + ret = -EINTR; + goto unlock_driver_lock; + } + hdev->io_started = false; hdrv = hdev->driver; if (hdrv) { @@ -1889,8 +1903,11 @@ static int hid_device_remove(struct device *dev) hdev->driver = NULL; } + if (!hdev->io_started) + up(&hdev->driver_input_lock); +unlock_driver_lock: up(&hdev->driver_lock); - return 0; + return ret; } static ssize_t modalias_show(struct device *dev, struct device_attribute *a, @@ -2329,6 +2346,7 @@ struct hid_device *hid_allocate_device(void) init_waitqueue_head(&hdev->debug_wait); INIT_LIST_HEAD(&hdev->debug_list); sema_init(&hdev->driver_lock, 1); + sema_init(&hdev->driver_input_lock, 1); return hdev; } diff --git a/include/linux/hid.h b/include/linux/hid.h index e14b465b1146..895b85639dec 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -456,7 +456,8 @@ struct hid_device { /* device report descriptor */ unsigned country; /* HID country */ struct hid_report_enum report_enum[HID_REPORT_TYPES]; - struct semaphore driver_lock; /* protects the current driver */ + struct semaphore driver_lock; /* protects the current driver, except during input */ + struct semaphore driver_input_lock; /* protects the current driver */ struct device dev; /* device */ struct hid_driver *driver; struct hid_ll_driver *ll_driver; @@ -477,6 +478,7 @@ struct hid_device { /* device report descriptor */ unsigned int status; /* see STAT flags above */ unsigned claimed; /* Claimed by hidinput, hiddev? */ unsigned quirks; /* Various quirks the device can pull on us */ + bool io_started; /* Protected by driver_lock. If IO has started */ struct list_head inputs; /* The list of inputs */ void *hiddev; /* The hiddev structure */ @@ -599,6 +601,10 @@ struct hid_usage_id { * @resume: invoked on resume if device was not reset (NULL means nop) * @reset_resume: invoked on resume if device was reset (NULL means nop) * + * probe should return -errno on error, or 0 on success. During probe, + * input will not be passed to raw_event unless hid_device_io_start is + * called. + * * raw_event and event should return 0 on no action performed, 1 when no * further processing should be done and negative on error * @@ -737,6 +743,44 @@ const struct hid_device_id *hid_match_id(struct hid_device *hdev, const struct hid_device_id *id); s32 hid_snto32(__u32 value, unsigned n); +/** + * hid_device_io_start - enable HID input during probe, remove + * + * @hid - the device + * + * This should only be called during probe or remove and only be + * called by the thread calling probe or remove. It will allow + * incoming packets to be delivered to the driver. + */ +static inline void hid_device_io_start(struct hid_device *hid) { + if (hid->io_started) { + dev_warn(&hid->dev, "io already started"); + return; + } + hid->io_started = true; + up(&hid->driver_input_lock); +} + +/** + * hid_device_io_stop - disable HID input during probe, remove + * + * @hid - the device + * + * Should only be called after hid_device_io_start. It will prevent + * incoming packets from going to the driver for the duration of + * probe, remove. If called during probe, packets will still go to the + * driver after probe is complete. This function should only be called + * by the thread calling probe or remove. + */ +static inline void hid_device_io_stop(struct hid_device *hid) { + if (!hid->io_started) { + dev_warn(&hid->dev, "io already stopped"); + return; + } + hid->io_started = false; + down(&hid->driver_input_lock); +} + /** * hid_map_usage - map usage input bits * -- cgit v1.2.3 From 9684819b5a29e62acd8265a92d8f3454de9bb71e Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 27 Feb 2013 16:38:17 +0100 Subject: HID: ll_driver: Extend the interface with idle requests Some drivers send the idle command directly to underlying device, creating an unwanted dependency on the underlying transport layer. This patch adds hid_hw_idle() to the interface, thereby removing usbhid from the lion share of the drivers. Signed-off-by: Benjamin Tissoires Reviewed-by: David Herrmann Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 15 +++++++++++++++ include/linux/hid.h | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+) (limited to 'include') diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 420466bc481a..effcd3d6f5cf 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1253,6 +1253,20 @@ static void usbhid_request(struct hid_device *hid, struct hid_report *rep, int r } } +static int usbhid_idle(struct hid_device *hid, int report, int idle, + int reqtype) +{ + struct usb_device *dev = hid_to_usb_dev(hid); + struct usb_interface *intf = to_usb_interface(hid->dev.parent); + struct usb_host_interface *interface = intf->cur_altsetting; + int ifnum = interface->desc.bInterfaceNumber; + + if (reqtype != HID_REQ_SET_IDLE) + return -EINVAL; + + return hid_set_idle(dev, ifnum, report, idle); +} + static struct hid_ll_driver usb_hid_driver = { .parse = usbhid_parse, .start = usbhid_start, @@ -1263,6 +1277,7 @@ static struct hid_ll_driver usb_hid_driver = { .hidinput_input_event = usb_hidinput_input_event, .request = usbhid_request, .wait = usbhid_wait_io, + .idle = usbhid_idle, }; static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) diff --git a/include/linux/hid.h b/include/linux/hid.h index 7071eb3d36c7..863744c38ddc 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -664,6 +664,7 @@ struct hid_driver { * shouldn't allocate anything to not leak memory * @request: send report request to device (e.g. feature report) * @wait: wait for buffered io to complete (send/recv reports) + * @idle: send idle request to device */ struct hid_ll_driver { int (*start)(struct hid_device *hdev); @@ -683,6 +684,7 @@ struct hid_ll_driver { struct hid_report *report, int reqtype); int (*wait)(struct hid_device *hdev); + int (*idle)(struct hid_device *hdev, int report, int idle, int reqtype); }; @@ -906,6 +908,23 @@ static inline void hid_hw_request(struct hid_device *hdev, hdev->ll_driver->request(hdev, report, reqtype); } +/** + * hid_hw_idle - send idle request to device + * + * @hdev: hid device + * @report: report to control + * @idle: idle state + * @reqtype: hid request type + */ +static inline int hid_hw_idle(struct hid_device *hdev, int report, int idle, + int reqtype) +{ + if (hdev->ll_driver->idle) + return hdev->ll_driver->idle(hdev, report, idle, reqtype); + + return 0; +} + /** * hid_hw_wait - wait for buffered io to complete * -- cgit v1.2.3 From 4f22decf9b6329acfe59091c5cba6b378b9b31db Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Fri, 22 Mar 2013 18:38:28 +0100 Subject: HID: input: don't register unmapped input devices There is no need to register an input device containing no events. This allows drivers using the quirk MULTI_INPUT to register one input per report effectively used. For backward compatibility, we need to add a quirk to request this behavior. Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/hid.h | 1 + 2 files changed, 78 insertions(+) (limited to 'include') diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 21b196c394b1..945b8158ec4c 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1198,6 +1198,67 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid) return hidinput; } +static bool hidinput_has_been_populated(struct hid_input *hidinput) +{ + int i; + unsigned long r = 0; + + for (i = 0; i < BITS_TO_LONGS(EV_CNT); i++) + r |= hidinput->input->evbit[i]; + + for (i = 0; i < BITS_TO_LONGS(KEY_CNT); i++) + r |= hidinput->input->keybit[i]; + + for (i = 0; i < BITS_TO_LONGS(REL_CNT); i++) + r |= hidinput->input->relbit[i]; + + for (i = 0; i < BITS_TO_LONGS(ABS_CNT); i++) + r |= hidinput->input->absbit[i]; + + for (i = 0; i < BITS_TO_LONGS(MSC_CNT); i++) + r |= hidinput->input->mscbit[i]; + + for (i = 0; i < BITS_TO_LONGS(LED_CNT); i++) + r |= hidinput->input->ledbit[i]; + + for (i = 0; i < BITS_TO_LONGS(SND_CNT); i++) + r |= hidinput->input->sndbit[i]; + + for (i = 0; i < BITS_TO_LONGS(FF_CNT); i++) + r |= hidinput->input->ffbit[i]; + + for (i = 0; i < BITS_TO_LONGS(SW_CNT); i++) + r |= hidinput->input->swbit[i]; + + return !!r; +} + +static void hidinput_cleanup_hidinput(struct hid_device *hid, + struct hid_input *hidinput) +{ + struct hid_report *report; + int i, k; + + list_del(&hidinput->list); + input_free_device(hidinput->input); + + for (k = HID_INPUT_REPORT; k <= HID_OUTPUT_REPORT; k++) { + if (k == HID_OUTPUT_REPORT && + hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORTS) + continue; + + list_for_each_entry(report, &hid->report_enum[k].report_list, + list) { + + for (i = 0; i < report->maxfield; i++) + if (report->field[i]->hidinput == hidinput) + report->field[i]->hidinput = NULL; + } + } + + kfree(hidinput); +} + /* * Register the input device; print a message. * Configure the input layer interface @@ -1249,6 +1310,10 @@ int hidinput_connect(struct hid_device *hid, unsigned int force) hidinput_configure_usage(hidinput, report->field[i], report->field[i]->usage + j); + if ((hid->quirks & HID_QUIRK_NO_EMPTY_INPUT) && + !hidinput_has_been_populated(hidinput)) + continue; + if (hid->quirks & HID_QUIRK_MULTI_INPUT) { /* This will leave hidinput NULL, so that it * allocates another one if we have more inputs on @@ -1265,6 +1330,18 @@ int hidinput_connect(struct hid_device *hid, unsigned int force) } } + if (hidinput && (hid->quirks & HID_QUIRK_NO_EMPTY_INPUT) && + !hidinput_has_been_populated(hidinput)) { + /* no need to register an input device not populated */ + hidinput_cleanup_hidinput(hid, hidinput); + hidinput = NULL; + } + + if (list_empty(&hid->inputs)) { + hid_err(hid, "No inputs registered, leaving\n"); + goto out_unwind; + } + if (hidinput) { if (drv->input_configured) drv->input_configured(hid, hidinput); diff --git a/include/linux/hid.h b/include/linux/hid.h index 863744c38ddc..fffa06bc4880 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -282,6 +282,7 @@ struct hid_item { #define HID_QUIRK_BADPAD 0x00000020 #define HID_QUIRK_MULTI_INPUT 0x00000040 #define HID_QUIRK_HIDINPUT_FORCE 0x00000080 +#define HID_QUIRK_NO_EMPTY_INPUT 0x00000100 #define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x00010000 #define HID_QUIRK_FULLSPEED_INTERVAL 0x10000000 #define HID_QUIRK_NO_INIT_REPORTS 0x20000000 -- cgit v1.2.3 From a5f04b9df1113e0c16271afe5e43028f0d763f13 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 17 Apr 2013 19:38:13 +0200 Subject: HID: debug: break out hid_dump_report() into hid-debug No semantic changes, but hid_dump_report should be in hid-debug.c, not in hid-core.c Signed-off-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/hid-core.c | 25 ++----------------------- drivers/hid/hid-debug.c | 30 ++++++++++++++++++++++++++++++ include/linux/hid-debug.h | 6 ++++-- 3 files changed, 36 insertions(+), 25 deletions(-) (limited to 'include') diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index aa341d135867..f86dd9708ca5 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1260,8 +1260,6 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i struct hid_report_enum *report_enum; struct hid_driver *hdrv; struct hid_report *report; - char *buf; - unsigned int i; int ret = 0; if (!hid) @@ -1284,28 +1282,9 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i } /* Avoid unnecessary overhead if debugfs is disabled */ - if (list_empty(&hid->debug_list)) - goto nomem; - - buf = kmalloc(sizeof(char) * HID_DEBUG_BUFSIZE, GFP_ATOMIC); - - if (!buf) - goto nomem; - - /* dump the report */ - snprintf(buf, HID_DEBUG_BUFSIZE - 1, - "\nreport (size %u) (%snumbered) = ", size, report_enum->numbered ? "" : "un"); - hid_debug_event(hid, buf); - - for (i = 0; i < size; i++) { - snprintf(buf, HID_DEBUG_BUFSIZE - 1, - " %02x", data[i]); - hid_debug_event(hid, buf); - } - hid_debug_event(hid, "\n"); - kfree(buf); + if (!list_empty(&hid->debug_list)) + hid_dump_report(hid, type, data, size); -nomem: report = hid_get_report(report_enum, data); if (!report) { diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index 933fff0fff1f..094cbcfe1e1a 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -591,6 +591,36 @@ void hid_debug_event(struct hid_device *hdev, char *buf) } EXPORT_SYMBOL_GPL(hid_debug_event); +void hid_dump_report(struct hid_device *hid, int type, u8 *data, + int size) +{ + struct hid_report_enum *report_enum; + char *buf; + unsigned int i; + + buf = kmalloc(sizeof(char) * HID_DEBUG_BUFSIZE, GFP_ATOMIC); + + if (!buf) + return; + + report_enum = hid->report_enum + type; + + /* dump the report */ + snprintf(buf, HID_DEBUG_BUFSIZE - 1, + "\nreport (size %u) (%snumbered) = ", size, + report_enum->numbered ? "" : "un"); + hid_debug_event(hid, buf); + + for (i = 0; i < size; i++) { + snprintf(buf, HID_DEBUG_BUFSIZE - 1, + " %02x", data[i]); + hid_debug_event(hid, buf); + } + hid_debug_event(hid, "\n"); + kfree(buf); +} +EXPORT_SYMBOL_GPL(hid_dump_report); + void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, __s32 value) { char *buf; diff --git a/include/linux/hid-debug.h b/include/linux/hid-debug.h index 53744fa1c8b7..8663f216c563 100644 --- a/include/linux/hid-debug.h +++ b/include/linux/hid-debug.h @@ -22,11 +22,12 @@ * */ -#define HID_DEBUG_BUFSIZE 512 - #ifdef CONFIG_DEBUG_FS +#define HID_DEBUG_BUFSIZE 512 + void hid_dump_input(struct hid_device *, struct hid_usage *, __s32); +void hid_dump_report(struct hid_device *, int , u8 *, int); void hid_dump_device(struct hid_device *, struct seq_file *); void hid_dump_field(struct hid_field *, int, struct seq_file *); char *hid_resolv_usage(unsigned, struct seq_file *); @@ -50,6 +51,7 @@ struct hid_debug_list { #else #define hid_dump_input(a,b,c) do { } while (0) +#define hid_dump_report(a,b,c,d) do { } while (0) #define hid_dump_device(a,b) do { } while (0) #define hid_dump_field(a,b,c) do { } while (0) #define hid_resolv_usage(a,b) do { } while (0) -- cgit v1.2.3 From 2353f2bea307390e015493118e425152b8a5a431 Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Tue, 16 Apr 2013 15:40:09 -0700 Subject: HID: protect hid_debug_list Accesses to hid_device->hid_debug_list are not serialized properly, which could result in SMP concurrency issues when HID debugfs events are accessesed by multiple userspace processess. Serialize all the list operations by a mutex. Spotted by Al Viro. Signed-off-by: Jiri Kosina --- drivers/hid/hid-core.c | 1 + drivers/hid/hid-debug.c | 6 ++++++ include/linux/hid.h | 1 + 3 files changed, 8 insertions(+) (limited to 'include') diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index f86dd9708ca5..e7765ede339e 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2319,6 +2319,7 @@ struct hid_device *hid_allocate_device(void) init_waitqueue_head(&hdev->debug_wait); INIT_LIST_HEAD(&hdev->debug_list); + mutex_init(&hdev->debug_list_lock); sema_init(&hdev->driver_lock, 1); return hdev; diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index 094cbcfe1e1a..7e56cb3855e3 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -580,12 +580,14 @@ void hid_debug_event(struct hid_device *hdev, char *buf) int i; struct hid_debug_list *list; + mutex_lock(&hdev->debug_list_lock); list_for_each_entry(list, &hdev->debug_list, node) { for (i = 0; i < strlen(buf); i++) list->hid_debug_buf[(list->tail + i) % HID_DEBUG_BUFSIZE] = buf[i]; list->tail = (list->tail + i) % HID_DEBUG_BUFSIZE; } + mutex_unlock(&hdev->debug_list_lock); wake_up_interruptible(&hdev->debug_wait); } @@ -990,7 +992,9 @@ static int hid_debug_events_open(struct inode *inode, struct file *file) file->private_data = list; mutex_init(&list->read_mutex); + mutex_lock(&list->hdev->debug_list_lock); list_add_tail(&list->node, &list->hdev->debug_list); + mutex_unlock(&list->hdev->debug_list_lock); out: return err; @@ -1085,7 +1089,9 @@ static int hid_debug_events_release(struct inode *inode, struct file *file) { struct hid_debug_list *list = file->private_data; + mutex_lock(&list->hdev->debug_list_lock); list_del(&list->node); + mutex_unlock(&list->hdev->debug_list_lock); kfree(list->hid_debug_buf); kfree(list); diff --git a/include/linux/hid.h b/include/linux/hid.h index e14b465b1146..06579c72d195 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -512,6 +512,7 @@ struct hid_device { /* device report descriptor */ struct dentry *debug_rdesc; struct dentry *debug_events; struct list_head debug_list; + struct mutex debug_list_lock; wait_queue_head_t debug_wait; }; -- cgit v1.2.3