Re: [v3.11][Regression] HID: hyperv: convert alloc+memcpy to memdup
On Mon, 16 Sep 2013, Joseph Salisbury wrote: Can you explain a little further? Mark commit a4a23f6 as bad? An initial bisect already reported that was the first bad commit, so it can't be marked bad. The oops on memcpy() happens after commit a4a23f6 is reverted. The oops on memcpy() did not happen before a4a23f6 was committed, so I assume this new oops was introduced by a change later. Right now I'm bisecting down the oops on memcpy() by updating the bisect with good or bad, depending if the test kernel hit the oops. I then revert a4a23f6, so that revert is the HEAD of the tree each time before building the kernel again(As long as the commit spit out by bisect is after when a4a23f6 was introduced). Yep. Please continue bisecting the memcpy() oops. kmemdup() is just a kzalloc() followed by a memcpy(). When we split it apart by reverting the patch then we would expect the oops to move to the memcpy() part. Somehow desc is a bogus pointer, but I don't immediately see how that is possible. regards, dan carpenter Thanks for the details. We'll continue the bisect and let you know how it goes. Did this please yield any useful result? Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] HID: Delay opening HID device
On Wed, 18 Sep 2013, Srinivas Pandruvada wrote: Don't call hid_open_device till there is actually an user. This saves power by not opening underlying transport for HID. Also close device if there are no active mfd client using HID sensor hub. Signed-off-by: Srinivas Pandruvada srinivas.pandruv...@linux.intel.com Signed-off-by: Jiri Kosina jkos...@suse.cz Jonathan, I'd be good if I pass this to you to apply the whole lot. Thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Few cleanups
On Wed, 11 Sep 2013, Benjamin Tissoires wrote: Well, while debugging hid-lenovo-tpkbd, I also cleaned it up a little (so this goes on top of the CVE series): - use devres API - remove usb references (so that the regressions tests through uhid are working) This has also been tested by https://bugzilla.redhat.com/show_bug.cgi?id=1003998 And I also had a patch for sony that I wanted to send, but I was waiting for a complete drop of usb, which I still did not managed to do. It took me some time, as I wanted to think a little bit more about cleaning up 2/3, but didn't really come up with something that'd be substantially nicer. Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid-elo: some systems cannot stomach work around
On Mon, 9 Sep 2013, oli...@neukum.org wrote: From: Oliver Neukum oneu...@suse.de Some systems although they have firmware class 'M', which usually needs a work around to not crash, must not be subjected to the work around because the work around crashes them. They cannot be told apart by their own device descriptor, but as they are part of compound devices can be identified by looking at their siblings. What a mess ... :/ Signed-off-by: Oliver Neukum oneu...@suse.de --- drivers/hid/hid-elo.c | 35 ++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c index f042a6c..64ac53e 100644 --- a/drivers/hid/hid-elo.c +++ b/drivers/hid/hid-elo.c @@ -181,7 +181,40 @@ fail: */ static bool elo_broken_firmware(struct usb_device *dev) { - return use_fw_quirk le16_to_cpu(dev-descriptor.bcdDevice) == 0x10d; + struct usb_device *hub = dev-parent; + struct usb_device *child = NULL; + u16 fw_lvl = le16_to_cpu(dev-descriptor.bcdDevice); + u16 child_vid, child_pid; + int i; + + if (!use_fw_quirk) + return false; + if (fw_lvl != 0x10d) + return false; + + /*iterate sibling devices of the touch controller*/ Could you please resubmit with comment reformated (spaces before/after asterisk)? + usb_hub_for_each_child(hub, i, child) { + child_vid = le16_to_cpu(child-descriptor.idVendor); + child_pid = le16_to_cpu(child-descriptor.idProduct); + + /* + * If one of the devices below is present attached as a sibling of + * the touch controller then this is a newer IBM 4820 monitor that + * does not need the IBM-requested workaround if fw level is + * 0x010d - aka 'M'. + * No other HW can have this combination. + */ + if (child_vid==0x04b3) { + switch (child_pid) { + case 0x4676: /*4820 21x Video*/ + case 0x4677: /*4820 51x Video*/ + case 0x4678: /*4820 2Lx Video*/ + case 0x4679: /*4820 5Lx Video*/ Here as well. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/10] HID: validate report details
On Wed, 11 Sep 2013, Benjamin Tissoires wrote: here is the v3 of the CVE fixes. Now applied, will be pushing to Linus for 3.12. Thanks everybody, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] update my generaltouch driver for linux by luosong
On Mon, 9 Sep 2013, android wrote: I am a software engineer from GeneralTouch Technology Co., Ltd. I want to add some driver patches to the linux kernel . I do these jobs in hid-ids.h and hid-multitouch.c Adding Henrik and Benjamon to CC for the hid-multitouch driver. The main changes in hid driver are like those: (1)add our new products into kernel driver +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0102 0x0102 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_E100 0xe100 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0101 0x0101 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0106 0x0106 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_010A 0x010a (2) correct previous bug - MT_QUIRK_SLOT_IS_CONTACTNUMBER + MT_QUIRK_SLOT_IS_CONTACTID This needs explanation / clarification in the changelog. the content of patch is shown below: From 5db217392e661695058606c7919be7fa6509f1e4 Mon Sep 17 00:00:00 2001 From: luosong andr...@generaltouch.com This doesn't look like a RFC-compliant from, I think. Date: Mon, 9 Sep 2013 02:30:10 +0800 Subject: [PATCH] update my generaltouch driver for linux by luosong Please insert changelog (description of the changes) and Signed-off-by: line here, as documented in Documentation/SubmittingPatches --- drivers/hid/hid-ids.h|5 + drivers/hid/hid-multitouch.c | 19 +-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index ffe4c7a..ca78f09 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -332,6 +332,11 @@ #define USB_VENDOR_ID_GENERAL_TOUCH 0x0dfc #define USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS 0x0003 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PWT_TENFINGERS 0x0100 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0102 0x0102 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_E100 0xe100 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0101 0x0101 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0106 0x0106 +#define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_010A 0x010a #define USB_VENDOR_ID_GLAB 0x06c2 #define USB_DEVICE_ID_4_PHIDGETSERVO_30 0x0038 diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index cb0e361..9558dde 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -244,12 +244,12 @@ static struct mt_class mt_classes[] = { { .name = MT_CLS_GENERALTOUCH_TWOFINGERS, .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP | MT_QUIRK_VALID_IS_INRANGE | - MT_QUIRK_SLOT_IS_CONTACTNUMBER, + MT_QUIRK_SLOT_IS_CONTACTID, .maxcontacts = 2 }, { .name = MT_CLS_GENERALTOUCH_PWT_TENFINGERS, .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP | - MT_QUIRK_SLOT_IS_CONTACTNUMBER + MT_QUIRK_SLOT_IS_CONTACTID }, { .name = MT_CLS_FLATFROG, @@ -1191,6 +1191,21 @@ static const struct hid_device_id mt_devices[] = { { .driver_data = MT_CLS_GENERALTOUCH_PWT_TENFINGERS, MT_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH, USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PWT_TENFINGERS) }, + { .driver_data = MT_CLS_GENERALTOUCH_TWOFINGERS, + MT_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH, + USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0101) }, + { .driver_data = MT_CLS_GENERALTOUCH_PWT_TENFINGERS, + MT_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH, + USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_E100) }, + { .driver_data = MT_CLS_GENERALTOUCH_PWT_TENFINGERS, + MT_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH, + USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0102) }, + { .driver_data = MT_CLS_GENERALTOUCH_PWT_TENFINGERS, + MT_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH, + USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_0106) }, + { .driver_data = MT_CLS_GENERALTOUCH_PWT_TENFINGERS, + MT_USB_DEVICE(USB_VENDOR_ID_GENERAL_TOUCH, + USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_010A) }, Your mail client seems to be whitespace-corrupting patches (it ate the tabs at least). Could you please fix all the above and resubmit? Thanks a lot, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Input/HID: Guitar/Drums support
On Mon, 26 Aug 2013, David Herrmann wrote: This series adds wiimote extension support for guitars and drums. Applied, thanks! -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: Correct the USB IDs for the new Macbook Air 6
On Tue, 3 Sep 2013, Dmitry Torokhov wrote: A recent patch (9d9a04ee) added support for the new machine, but got the sequence of USB ids wrong. Reports from both Ian and Linus T show that the 0x0291 id is for ISO, not ANSI, which should have the missing number 0x0290. This patchs moves the three numbers accordingly, fixing the problem. Cc: Dmitry Torokhov dmitry.torok...@gmail.com Reported-and-tested-by: Ian Munsie darkstarsw...@gmail.com Tested-by: Linus G Thiel li...@hanssonlarsson.se Signed-off-by: Henrik Rydberg rydb...@euromail.se --- Hi Jiri, Dmitry, it looks like this change has been sufficiently tested now, in addition to making perfect sense. Jiri, would you mind taking it, if Dmitry approves? Absolutely ... waiting for Dmitry's Ack. And here it is: Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com Perfect, thanks. Will push it to Linus this merge window. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: uhid: improve uhid example client
On Mon, 2 Sep 2013, David Herrmann wrote: But thanks for the usb-spec hint, haven't looked there, yet. There is also HID report descriptor tool available at http://www.usb.org/developers/hidpage/ The tool itself isn't that earth-shaking (and you have to run it in wine :) ), but it contains a load of pre-defined example descriptors for various devices. No intention to modify it. Looks all good to me now. Now applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid-wiimote: print small buffers via %*ph
On Tue, 3 Sep 2013, Andy Shevchenko wrote: Instead of passing each byte through stack let's use %*ph specifier to dump buffer as a hex string. Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid-wiimote: print small buffers via %*ph
On Wed, 4 Sep 2013, Jiri Kosina wrote: Instead of passing each byte through stack let's use %*ph specifier to dump buffer as a hex string. Applied, thanks. I acutally applied the phC one, but replied to wrong thread, sorry for the noise. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/14] HID: multitouch: validate feature report details
On Mon, 2 Sep 2013, Benjamin Tissoires wrote: Given the 12 flaws, what do you see as the best way forward? If Jiri thinks we can wait one more week for some of these patches (I know that the most critical one, 1/14 is already on its way to Linus), then I can handle the v2, but not before Friday. Again, I think the current patch series is perfectly fine for distributions as they can add/remove things more easily than we can do in master. I agree. 1/14 will go to Linus shortly. The rest is public, and distributions are taking it depending on their requirements, but I'd like to have the patchset polished before pushing it to Linus. Therefore putting the rest on hold now, and waiting for v2. Thanks a lot to both of you, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/14] HID: pantherlord: validate output report details
On Wed, 28 Aug 2013, Jiri Kosina wrote: From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the pantherlord HID driver to write beyond the output report allocation during initialization, causing a heap overflow: [ 310.939483] usb 1-1: New USB device found, idVendor=0e8f, idProduct=0003 ... [ 315.980774] BUG kmalloc-192 (Tainted: GW ): Redzone overwritten CVE-2013-2892 Applying this one, thanks. Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-pl.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-pl.c b/drivers/hid/hid-pl.c index d29112f..2dcd7d9 100644 --- a/drivers/hid/hid-pl.c +++ b/drivers/hid/hid-pl.c @@ -132,8 +132,14 @@ static int plff_init(struct hid_device *hid) strong = report-field[0]-value[2]; weak = report-field[0]-value[3]; debug(detected single-field device); - } else if (report-maxfield = 4 report-field[0]-maxusage == 1 - report-field[0]-usage[0].hid == (HID_UP_LED | 0x43)) { + } else if (report-field[0]-maxusage == 1 +report-field[0]-usage[0].hid == + (HID_UP_LED | 0x43) +report-maxfield = 4 +report-field[0]-report_count = 1 +report-field[1]-report_count = 1 +report-field[2]-report_count = 1 +report-field[3]-report_count = 1) { report-field[0]-value[0] = 0x00; report-field[1]-value[0] = 0x00; strong = report-field[2]-value[0]; -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/14] HID: ntrig: validate feature report details
On Thu, 29 Aug 2013, Rafi Rubin wrote: Signed-off-by: Rafi Rubin r...@seas.upenn.edu Applied. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/14] HID: picolcd_core: validate output report details
On Sat, 31 Aug 2013, Bruno Prémont wrote: Hi Kees, Jiri, On Wed, 28 August 2013 Jiri Kosina jkos...@suse.cz wrote: From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the picolcd HID driver to trigger a NULL dereference during attr file writing. CVE-2013-2899 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-picolcd_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c index b48092d..72bba1e 100644 --- a/drivers/hid/hid-picolcd_core.c +++ b/drivers/hid/hid-picolcd_core.c @@ -290,7 +290,7 @@ static ssize_t picolcd_operation_mode_store(struct device *dev, buf += 10; cnt -= 10; } - if (!report) + if (!report || report-maxfield 1) Please do + if (!report || report-maxfield != 1) That way we are consistent with whole picolcd driver and a device deciding to change its HID-ABI will be properly detected. With that adjustment, Acked-by/Reviewed-by me Applied with that adjustment. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/14] HID: check for NULL field when setting values
On Wed, 28 Aug 2013, Jiri Kosina wrote: From: Kees Cook keesc...@chromium.org Defensively check that the field to be worked on is not NULL. Applied. Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-core.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 55798b2..192be6b 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1206,7 +1206,12 @@ EXPORT_SYMBOL_GPL(hid_output_report); int hid_set_field(struct hid_field *field, unsigned offset, __s32 value) { - unsigned size = field-report_size; + unsigned size; + + if (!field) + return -1; + + size = field-report_size; hid_dump_input(field-report-device, field-usage + offset, value); -- Jiri Kosina SUSE Labs -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] HID: validate HID report id size
So I have now queued subset of the patches that don't deal with hid_validate_report(); waiting for v2 based on Benjamin's comments with the rest. The patches are public for distributions to apply them if they feel urgent need, but for upstream I'd rather do things properly, and Benjamin had quite some valuable feedback. Thanks again, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] hid-sensor-hub: fix indentation accross the code
On Thu, 29 Aug 2013, Andy Shevchenko wrote: On Mon, 2013-08-19 at 08:28 -0700, Srinivas Pandruvada wrote: On 08/14/2013 01:07 AM, Andy Shevchenko wrote: Patch just rearranges lines to be more compact and/or readable. Additionally it converts double space to one in several places. There is no functional change. Jiri, anything about this patch and the rest of the series? It seems you applied only patch 1/4. Do you have comments? The whole series is now applied. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-hid: explicitly cast parametrs to follow specifiers
On Tue, 25 Jun 2013, Andy Shevchenko wrote: The vendor and product parameters are defined as __u32, but used as short int. Let's do an explicit casting for them. Is this fixing any compiler warning you have seen? Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/hid/i2c-hid/i2c-hid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index 879b0ed..d595f2a 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -1009,7 +1009,7 @@ static int i2c_hid_probe(struct i2c_client *client, hid-product = le16_to_cpu(ihid-hdesc.wProductID); snprintf(hid-name, sizeof(hid-name), %s %04hX:%04hX, - client-name, hid-vendor, hid-product); + client-name, (short int)hid-vendor, (short int)hid-product); ret = hid_add_device(hid); if (ret) { -- 1.8.3.1 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/14] HID: sensor-hub: validate feature report details
On Wed, 4 Sep 2013, Kees Cook wrote: Should this one have been part of the batch you applied? It doesn't use hid_validate_report(). It's there [1], I just somehow forgot to send out information mail, sorry for that. [1] https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-3.12/upstreamid=9e8910257397372633e74b333ef891f20c800ee4 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: Correct the USB IDs for the new Macbook Air 6
On Sun, 1 Sep 2013, Henrik Rydberg wrote: A recent patch (9d9a04ee) added support for the new machine, but got the sequence of USB ids wrong. Reports from both Ian and Linus T show that the 0x0291 id is for ISO, not ANSI, which should have the missing number 0x0290. This patchs moves the three numbers accordingly, fixing the problem. Cc: Dmitry Torokhov dmitry.torok...@gmail.com Reported-and-tested-by: Ian Munsie darkstarsw...@gmail.com Tested-by: Linus G Thiel li...@hanssonlarsson.se Signed-off-by: Henrik Rydberg rydb...@euromail.se --- Hi Jiri, Dmitry, it looks like this change has been sufficiently tested now, in addition to making perfect sense. Jiri, would you mind taking it, if Dmitry approves? Absolutely ... waiting for Dmitry's Ack. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: uhid: add devname module alias
On Sun, 1 Sep 2013, David Herrmann wrote: For simple device node creation, add the devname module alias. Yepp, looks good. uinput already has it, so: Reviewed-by: David Herrmann dh.herrm...@gmail.com Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HID: picolcd: Prevent NULL pointer dereference on _remove()
On Sat, 31 Aug 2013, Bruno Prémont wrote: When picolcd is switched into bootloader mode (for FW flashing) make sure not to try to dereference NULL-pointers of feature-devices during unplug/unbind. This fixes following BUG: BUG: unable to handle kernel NULL pointer dereference at 0298 IP: [f811f56b] picolcd_exit_framebuffer+0x1b/0x80 [hid_picolcd] *pde = Oops: [#1] Modules linked in: hid_picolcd syscopyarea sysfillrect sysimgblt fb_sys_fops CPU: 0 PID: 15 Comm: khubd Not tainted 3.11.0-rc7-2-g50d62d4 #2 EIP: 0060:[f811f56b] EFLAGS: 00010292 CPU: 0 EIP is at picolcd_exit_framebuffer+0x1b/0x80 [hid_picolcd] Call Trace: [f811d1ab] picolcd_remove+0xcb/0x120 [hid_picolcd] [c1469b09] hid_device_remove+0x59/0xc0 [c13464ca] __device_release_driver+0x5a/0xb0 [c134653f] device_release_driver+0x1f/0x30 [c134603d] bus_remove_device+0x9d/0xd0 [c13439a5] device_del+0xd5/0x150 [c14696a4] hid_destroy_device+0x24/0x60 [c1474cbb] usbhid_disconnect+0x1b/0x40 ... Signed-off-by: Bruno Prémont bonb...@linux-vserver.org Cc: sta...@kernel.org --- drivers/hid/hid-picolcd_cir.c | 3 ++- drivers/hid/hid-picolcd_fb.c | 6 +- 2 files changed, 7 insertions(+), 2 deletions(-) Applied, thanks Bruno. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] HID: usbhid: quirk for N-Trig DuoSense Touch Screen
On Fri, 30 Aug 2013, Vasily Titskiy wrote: The DuoSense touchscreen device causes a 10 second timeout. This fix removes the delay. Signed-off-by: Vasily Titskiy qeh...@gmail.com --- drivers/hid/hid-ids.h |1 + drivers/hid/usbhid/hid-quirks.c |1 + 2 files changed, 2 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 2168885..376170e 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -646,6 +646,7 @@ #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_16 0x0012 #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_17 0x0013 #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18 0x0014 +#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500 #define USB_VENDOR_ID_ONTRAK 0x0a07 #define USB_DEVICE_ID_ONTRAK_ADU100 0x0064 diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c index 19b8360..1671b47 100644 --- a/drivers/hid/usbhid/hid-quirks.c +++ b/drivers/hid/usbhid/hid-quirks.c @@ -109,6 +109,7 @@ static const struct hid_blacklist { { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X, HID_QUIRK_MULTI_INPUT }, { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, HID_QUIRK_MULTI_INPUT }, + { USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_DUOSENSE, HID_QUIRK_NO_INIT_REPORTS }, { 0, 0 } }; Vasily, your mail client is line-wrapping text, which causes corrupted patches. I have fixed it up and applied, but please keep that in mind for your future submissions. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: roccat: Added support for KonePureOptical v2
On Fri, 30 Aug 2013, Stefan Achatz wrote: KonePureOptical is a KonePure with different sensor. Signed-off-by: Stefan Achatz erazor...@users.sourceforge.net Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Input: introduce BTN/ABS bits for drums and guitars
On Mon, 26 Aug 2013, David Herrmann wrote: There are a bunch of guitar and drums devices out there that all report similar data. To avoid reporting this as BTN_MISC or ABS_MISC, we allocate some proper namespace for them. Note that most of these devices are toys and we cannot report any sophisticated physics via this API. I did some google-images research and tried to provide definitions that work with all common devices. That's why I went with 4 toms, 4 cymbals, one bass, one hi-hat. I haven't seen other drums and I doubt that we need any additions to that. Anyway, the naming-scheme is intentionally done in an extensible way. For guitars, we support 5 frets (normally aligned vertically, compared to the real horizontal layouts), a single strum-bar with up/down directions, an optional fret-board and a whammy-bar. Most of the devices provide pressure values so I went with ABS_* bits. If we ever support devices which only provide digital input, we have to decide whether to emulate pressure data or add additional BTN_* bits. If someone is not familiar with these devices, here are two pictures which provide almost all introduced interfaces (or try the given keywords with a google-image search): Guitar: (guitar hero world tour guitar) http://images1.wikia.nocookie.net/__cb20120911023442/applezone/es/images/f/f9/Wii_Guitar.jpg Drums: (guitar hero drums) http://oyster.ignimgs.com/franchises/images/03/55/35526_band-hero-drum-set-hands-on-20090929040735768.jpg Signed-off-by: David Herrmann dh.herrm...@gmail.com Hi, I have reviewed and like the implementation of 2/3 and 3/3, but I of course would like to have Ack from Dmitry for this, so that I could take it through my tree together with the rest of the patchset. Dmitry, pretty please? Thanks. --- include/linux/mod_devicetable.h | 2 +- include/uapi/linux/input.h | 25 +++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 45e9214..329aa30 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -277,7 +277,7 @@ struct pcmcia_device_id { #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING 0x71 #define INPUT_DEVICE_ID_KEY_MAX 0x2ff #define INPUT_DEVICE_ID_REL_MAX 0x0f -#define INPUT_DEVICE_ID_ABS_MAX 0x3f +#define INPUT_DEVICE_ID_ABS_MAX 0x4f #define INPUT_DEVICE_ID_MSC_MAX 0x07 #define INPUT_DEVICE_ID_LED_MAX 0x0f #define INPUT_DEVICE_ID_SND_MAX 0x07 diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index d584047..76457ee 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -716,6 +716,14 @@ struct input_keymap_entry { #define BTN_DPAD_LEFT0x222 #define BTN_DPAD_RIGHT 0x223 +#define BTN_FRET_FAR_UP 0x224 +#define BTN_FRET_UP 0x225 +#define BTN_FRET_MID 0x226 +#define BTN_FRET_LOW 0x227 +#define BTN_FRET_FAR_LOW 0x228 +#define BTN_STRUM_BAR_UP 0x229 +#define BTN_STRUM_BAR_DOWN 0x22a + #define BTN_TRIGGER_HAPPY0x2c0 #define BTN_TRIGGER_HAPPY1 0x2c0 #define BTN_TRIGGER_HAPPY2 0x2c1 @@ -829,8 +837,21 @@ struct input_keymap_entry { #define ABS_MT_TOOL_X0x3c/* Center X tool position */ #define ABS_MT_TOOL_Y0x3d/* Center Y tool position */ - -#define ABS_MAX 0x3f +/* Drums and guitars (mostly toys) */ +#define ABS_TOM_FAR_LEFT 0x40 +#define ABS_TOM_LEFT 0x41 +#define ABS_TOM_RIGHT0x42 +#define ABS_TOM_FAR_RIGHT0x43 +#define ABS_CYMBAL_FAR_LEFT 0x44 +#define ABS_CYMBAL_LEFT 0x45 +#define ABS_CYMBAL_RIGHT 0x46 +#define ABS_CYMBAL_FAR_RIGHT 0x47 +#define ABS_BASS 0x48 +#define ABS_HI_HAT 0x49 +#define ABS_FRET_BOARD 0x4a/* Guitar fret board, vertical pos */ +#define ABS_WHAMMY_BAR 0x4b/* Guitar whammy bar (or vibrato) */ + +#define ABS_MAX 0x4f #define ABS_CNT (ABS_MAX+1) /* -- 1.8.4 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] si4713 : HID blacklist Si4713 USB development board
On Fri, 30 Aug 2013, Dinesh Ram wrote: The Si4713 development board contains a Si4713 FM transmitter chip and is handled by the radio-usb-si4713 driver. The board reports itself as (10c4:8244) Cygnal Integrated Products, Inc. and misidentifies itself as a HID device in its USB interface descriptor. This patch ignores this device as an HID device and hence loads the custom driver. Signed-off-by: Dinesh Ram din...@cisco.com Signed-off-by: Jiri Kosina jkos...@suse.cz Please feel free to take it together with my sigoff with the rest of the series. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] HID: validate HID report id size
On Wed, 28 Aug 2013, Jiri Kosina wrote: From: Kees Cook keesc...@chromium.org The Report ID field of a HID report is used to build indexes of reports. The kernel's index of these is limited to 256 entries, so any malicious device that sets a Report ID greater than 255 will trigger memory corruption on the host: [ 1347.156239] BUG: unable to handle kernel paging request at 88094958a878 [ 1347.156261] IP: [813e4da0] hid_register_report+0x2a/0x8b CVE-2013-2888 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org Applied this one to hid.git#for-3.11/CVE-2013-2888 Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] HID: validate HID report id size
On Thu, 29 Aug 2013, Benjamin Tissoires wrote: The Report ID field of a HID report is used to build indexes of reports. The kernel's index of these is limited to 256 entries, so any malicious device that sets a Report ID greater than 255 will trigger memory corruption on the host: [ 1347.156239] BUG: unable to handle kernel paging request at 88094958a878 [ 1347.156261] IP: [813e4da0] hid_register_report+0x2a/0x8b CVE-2013-2888 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org Applied this one to hid.git#for-3.11/CVE-2013-2888 Well, I am a little bit lost here. I know the patch will not break any existing things, but still, I really can not understand how this can happen: - Report IDs are specifically specified to be 1-byte fields. That means that the max they can have is 0xFF, which is 255 :-/ Hmm, with a specially crafted device, you can end up with item-size 1 coming out from fetch_item() for report ID, no? -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/14] HID: provide a helper for validating hid reports
From: Kees Cook keesc...@chromium.org Many drivers need to validate the characteristics of their HID report during initialization to avoid misusing the reports. This adds a common helper to perform validation of the report, its field count, and the value count within the fields. Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-core.c | 50 include/linux/hid.h|4 2 files changed, 54 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 5ea7d51..55798b2 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -759,6 +759,56 @@ int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size) } EXPORT_SYMBOL_GPL(hid_parse_report); +static const char * const hid_report_names[] = { + HID_INPUT_REPORT, + HID_OUTPUT_REPORT, + HID_FEATURE_REPORT, +}; +/** + * hid_validate_report - validate existing device report + * + * @device: hid device + * @type: which report type to examine + * @id: which report ID to examine (0 for first) + * @fields: expected number of fields + * @report_counts: expected number of values per field + * + * Validate the report details after parsing. + */ +struct hid_report *hid_validate_report(struct hid_device *hid, + unsigned int type, unsigned int id, + unsigned int fields, + unsigned int report_counts) +{ + struct hid_report *report; + unsigned int i; + + if (type HID_FEATURE_REPORT) { + hid_err(hid, invalid HID report %u\n, type); + return NULL; + } + + report = hid-report_enum[type].report_id_hash[id]; + if (!report) { + hid_err(hid, missing %s %u\n, hid_report_names[type], id); + return NULL; + } + if (report-maxfield fields) { + hid_err(hid, not enough fields in %s %u\n, + hid_report_names[type], id); + return NULL; + } + for (i = 0; i fields; i++) { + if (report-field[i]-report_count report_counts) { + hid_err(hid, not enough values in %s %u fields\n, + hid_report_names[type], id); + return NULL; + } + } + return report; +} +EXPORT_SYMBOL_GPL(hid_validate_report); + /** * hid_open_report - open a driver-specific device report * diff --git a/include/linux/hid.h b/include/linux/hid.h index ff545cc..76e41d8 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -749,6 +749,10 @@ void hid_output_report(struct hid_report *report, __u8 *data); struct hid_device *hid_allocate_device(void); struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id); int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size); +struct hid_report *hid_validate_report(struct hid_device *hid, + unsigned int type, unsigned int id, + unsigned int fields, + unsigned int report_counts); int hid_open_report(struct hid_device *device); int hid_check_keys_pressed(struct hid_device *hid); int hid_connect(struct hid_device *hid, unsigned int connect_mask); -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/14] HID: sony: validate HID output report details
From: Kees Cook keesc...@chromium.org This driver must validate the availability of the HID output report and its size before it can write LED states via buzz_set_leds(). This stops a heap overflow that is possible if a device provides a malicious HID output report: [ 108.171280] usb 1-1: New USB device found, idVendor=054c, idProduct=0002 ... [ 117.507877] BUG kmalloc-192 (Not tainted): Redzone overwritten CVE-2013-2890 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-sony.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index 87fbe29..b987926 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -537,6 +537,10 @@ static int buzz_init(struct hid_device *hdev) drv_data = hid_get_drvdata(hdev); BUG_ON(!(drv_data-quirks BUZZ_CONTROLLER)); + /* Validate expected report characteristics. */ + if (!hid_validate_report(hdev, HID_OUTPUT_REPORT, 0, 1, 7)) + return -ENODEV; + buzz = kzalloc(sizeof(*buzz), GFP_KERNEL); if (!buzz) { hid_err(hdev, Insufficient memory, cannot allocate driver data\n); -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/14] HID: zeroplus: validate output report details
From: Kees Cook keesc...@chromium.org The zeroplus HID driver was not checking the size of allocated values in fields it used. A HID device could send a malicious output report that would cause the driver to write beyond the output report allocation during initialization, causing a heap overflow: [ 1442.728680] usb 1-1: New USB device found, idVendor=0c12, idProduct=0005 ... [ 1466.243173] BUG kmalloc-192 (Tainted: GW ): Redzone overwritten CVE-2013-2889 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-zpff.c | 14 ++ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/hid/hid-zpff.c b/drivers/hid/hid-zpff.c index 6ec28a3..b124991 100644 --- a/drivers/hid/hid-zpff.c +++ b/drivers/hid/hid-zpff.c @@ -68,22 +68,12 @@ static int zpff_init(struct hid_device *hid) struct hid_report *report; struct hid_input *hidinput = list_entry(hid-inputs.next, struct hid_input, list); - struct list_head *report_list = - hid-report_enum[HID_OUTPUT_REPORT].report_list; struct input_dev *dev = hidinput-input; int error; - if (list_empty(report_list)) { - hid_err(hid, no output report found\n); + report = hid_validate_report(hid, HID_OUTPUT_REPORT, 0, 4, 1); + if (!report) return -ENODEV; - } - - report = list_entry(report_list-next, struct hid_report, list); - - if (report-maxfield 4) { - hid_err(hid, not enough fields in report\n); - return -ENODEV; - } zpff = kzalloc(sizeof(struct zpff_device), GFP_KERNEL); if (!zpff) -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/14] HID: validate HID report id size
From: Kees Cook keesc...@chromium.org The Report ID field of a HID report is used to build indexes of reports. The kernel's index of these is limited to 256 entries, so any malicious device that sets a Report ID greater than 255 will trigger memory corruption on the host: [ 1347.156239] BUG: unable to handle kernel paging request at 88094958a878 [ 1347.156261] IP: [813e4da0] hid_register_report+0x2a/0x8b CVE-2013-2888 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-core.c | 10 +++--- include/linux/hid.h|4 +++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 36668d1..5ea7d51 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -63,6 +63,8 @@ struct hid_report *hid_register_report(struct hid_device *device, unsigned type, struct hid_report_enum *report_enum = device-report_enum + type; struct hid_report *report; + if (id = HID_MAX_IDS) + return NULL; if (report_enum-report_id_hash[id]) return report_enum-report_id_hash[id]; @@ -404,8 +406,10 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item) case HID_GLOBAL_ITEM_TAG_REPORT_ID: parser-global.report_id = item_udata(item); - if (parser-global.report_id == 0) { - hid_err(parser-device, report_id 0 is invalid\n); + if (parser-global.report_id == 0 || + parser-global.report_id = HID_MAX_IDS) { + hid_err(parser-device, report_id %u is invalid\n, + parser-global.report_id); return -1; } return 0; @@ -575,7 +579,7 @@ static void hid_close_report(struct hid_device *device) for (i = 0; i HID_REPORT_TYPES; i++) { struct hid_report_enum *report_enum = device-report_enum + i; - for (j = 0; j 256; j++) { + for (j = 0; j HID_MAX_IDS; j++) { struct hid_report *report = report_enum-report_id_hash[j]; if (report) hid_free_report(report); diff --git a/include/linux/hid.h b/include/linux/hid.h index 0c48991..ff545cc 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -393,10 +393,12 @@ struct hid_report { struct hid_device *device; /* associated device */ }; +#define HID_MAX_IDS 256 + struct hid_report_enum { unsigned numbered; struct list_head report_list; - struct hid_report *report_id_hash[256]; + struct hid_report *report_id_hash[HID_MAX_IDS]; }; #define HID_REPORT_TYPES 3 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/14] HID: steelseries: validate output report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the steelseries HID driver to write beyond the output report allocation during initialization, causing a heap overflow: [ 167.981534] usb 1-1: New USB device found, idVendor=1038, idProduct=1410 ... [ 182.050547] BUG kmalloc-256 (Tainted: GW ): Redzone overwritten CVE-2013-2891 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-steelseries.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index d164911..ef42e86 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -249,6 +249,11 @@ static int steelseries_srws1_probe(struct hid_device *hdev, goto err_free; } + if (!hid_validate_report(hdev, HID_OUTPUT_REPORT, 0, 1, 16)) { + ret = -ENODEV; + goto err_free; + } + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); if (ret) { hid_err(hdev, hw start failed\n); -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/14] HID: ntrig: validate feature report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious feature report that would cause the ntrig HID driver to trigger a NULL dereference during initialization: [57383.031190] usb 3-1: New USB device found, idVendor=1b96, idProduct=0001 ... [57383.315193] BUG: unable to handle kernel NULL pointer dereference at 0030 [57383.315308] IP: [a08102de] ntrig_probe+0x25e/0x420 [hid_ntrig] CVE-2013-2896 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-ntrig.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c index ef95102..5482156 100644 --- a/drivers/hid/hid-ntrig.c +++ b/drivers/hid/hid-ntrig.c @@ -115,7 +115,8 @@ static inline int ntrig_get_mode(struct hid_device *hdev) struct hid_report *report = hdev-report_enum[HID_FEATURE_REPORT]. report_id_hash[0x0d]; - if (!report) + if (!report || report-maxfield 1 || + report-field[0]-report_count 1) return -EINVAL; hid_hw_request(hdev, report, HID_REQ_GET_REPORT); -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/14] HID: pantherlord: validate output report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the pantherlord HID driver to write beyond the output report allocation during initialization, causing a heap overflow: [ 310.939483] usb 1-1: New USB device found, idVendor=0e8f, idProduct=0003 ... [ 315.980774] BUG kmalloc-192 (Tainted: GW ): Redzone overwritten CVE-2013-2892 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-pl.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-pl.c b/drivers/hid/hid-pl.c index d29112f..2dcd7d9 100644 --- a/drivers/hid/hid-pl.c +++ b/drivers/hid/hid-pl.c @@ -132,8 +132,14 @@ static int plff_init(struct hid_device *hid) strong = report-field[0]-value[2]; weak = report-field[0]-value[3]; debug(detected single-field device); - } else if (report-maxfield = 4 report-field[0]-maxusage == 1 - report-field[0]-usage[0].hid == (HID_UP_LED | 0x43)) { + } else if (report-field[0]-maxusage == 1 + report-field[0]-usage[0].hid == + (HID_UP_LED | 0x43) + report-maxfield = 4 + report-field[0]-report_count = 1 + report-field[1]-report_count = 1 + report-field[2]-report_count = 1 + report-field[3]-report_count = 1) { report-field[0]-value[0] = 0x00; report-field[1]-value[0] = 0x00; strong = report-field[2]-value[0]; -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/14] HID: LG: validate HID output report details
, No output report found\n); - return -1; - } - /* Check that the report looks ok */ - report = list_entry(report_list-next, struct hid_report, list); - if (!report) { - hid_err(hid, NULL output report\n); + if (!hid_validate_report(hid, HID_OUTPUT_REPORT, 0, 1, 7)) return -1; - } - - field = report-field[0]; - if (!field) { - hid_err(hid, NULL field\n); - return -1; - } /* Check what wheel has been connected */ for (i = 0; i ARRAY_SIZE(lg4ff_devices); i++) { diff --git a/drivers/hid/hid-lgff.c b/drivers/hid/hid-lgff.c index d7ea8c8..a84fb40 100644 --- a/drivers/hid/hid-lgff.c +++ b/drivers/hid/hid-lgff.c @@ -128,27 +128,14 @@ static void hid_lgff_set_autocenter(struct input_dev *dev, u16 magnitude) int lgff_init(struct hid_device* hid) { struct hid_input *hidinput = list_entry(hid-inputs.next, struct hid_input, list); - struct list_head *report_list = hid-report_enum[HID_OUTPUT_REPORT].report_list; struct input_dev *dev = hidinput-input; - struct hid_report *report; - struct hid_field *field; const signed short *ff_bits = ff_joystick; int error; int i; - /* Find the report to use */ - if (list_empty(report_list)) { - hid_err(hid, No output report found\n); - return -1; - } - /* Check that the report looks ok */ - report = list_entry(report_list-next, struct hid_report, list); - field = report-field[0]; - if (!field) { - hid_err(hid, NULL field\n); - return -1; - } + if (!hid_validate_report(hid, HID_OUTPUT_REPORT, 0, 1, 7)) + return -ENODEV; for (i = 0; i ARRAY_SIZE(devices); i++) { if (dev-id.vendor == devices[i].idVendor -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/14] HID: lenovo-tpkbd: validate output report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the lenovo-tpkbd HID driver to write just beyond the output report allocation during initialization, causing a heap overflow: [ 76.109807] usb 1-1: New USB device found, idVendor=17ef, idProduct=6009 ... [ 80.462540] BUG kmalloc-192 (Tainted: GW ): Redzone overwritten CVE-2013-2894 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-lenovo-tpkbd.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c index 07837f5..b697ada 100644 --- a/drivers/hid/hid-lenovo-tpkbd.c +++ b/drivers/hid/hid-lenovo-tpkbd.c @@ -341,6 +341,11 @@ static int tpkbd_probe_tp(struct hid_device *hdev) char *name_mute, *name_micmute; int ret; + /* Validate required reports. */ + if (!hid_validate_report(hdev, HID_OUTPUT_REPORT, 4, 4, 1) || + !hid_validate_report(hdev, HID_OUTPUT_REPORT, 3, 1, 2)) + return -ENODEV; + if (sysfs_create_group(hdev-dev.kobj, tpkbd_attr_group_pointer)) { hid_warn(hdev, Could not create sysfs group\n); -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/14] HID: logitech-dj: validate output report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the logitech-dj HID driver to leak kernel memory contents to the device, or trigger a NULL dereference during initialization: [ 304.424553] usb 1-1: New USB device found, idVendor=046d, idProduct=c52b ... [ 304.780467] BUG: unable to handle kernel NULL pointer dereference at 0028 [ 304.781409] IP: [815d50aa] logi_dj_recv_send_report.isra.11+0x1a/0x90 CVE-2013-2895 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-logitech-dj.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index cd33084..7b99c2a 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -461,7 +461,7 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev, struct hid_report *report; struct hid_report_enum *output_report_enum; u8 *data = (u8 *)(dj_report-device_index); - int i; + unsigned int i, length; output_report_enum = hdev-report_enum[HID_OUTPUT_REPORT]; report = output_report_enum-report_id_hash[REPORT_ID_DJ_SHORT]; @@ -471,7 +471,9 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev, return -ENODEV; } - for (i = 0; i report-field[0]-report_count; i++) + length = min_t(size_t, sizeof(*dj_report) - 1, + report-field[0]-report_count); + for (i = 0; i length; i++) report-field[0]-value[i] = data[i]; hid_hw_request(hdev, report, HID_REQ_SET_REPORT); @@ -783,6 +785,12 @@ static int logi_dj_probe(struct hid_device *hdev, goto hid_parse_fail; } + if (!hid_validate_report(hdev, HID_OUTPUT_REPORT, REPORT_ID_DJ_SHORT, +1, 3)) { + retval = -ENODEV; + goto hid_parse_fail; + } + /* Starts the usb device and connects to upper interfaces hiddev and * hidraw */ retval = hid_hw_start(hdev, HID_CONNECT_DEFAULT); -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/14] HID: sensor-hub: validate feature report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious feature report that would cause the sensor-hub HID driver to read past the end of heap allocation, leaking kernel memory contents to the caller. CVE-2013-2898 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-sensor-hub.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index ca749810..aa34755 100644 --- a/drivers/hid/hid-sensor-hub.c +++ b/drivers/hid/hid-sensor-hub.c @@ -221,7 +221,8 @@ int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 report_id, mutex_lock(data-mutex); report = sensor_hub_report(report_id, hsdev-hdev, HID_FEATURE_REPORT); - if (!report || (field_index = report-maxfield)) { + if (!report || (field_index = report-maxfield) || + report-field[field_index]-report_count 1) { ret = -EINVAL; goto done_proc; } -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/14] HID: picolcd_core: validate output report details
From: Kees Cook keesc...@chromium.org A HID device could send a malicious output report that would cause the picolcd HID driver to trigger a NULL dereference during attr file writing. CVE-2013-2899 Signed-off-by: Kees Cook keesc...@chromium.org Cc: sta...@kernel.org --- drivers/hid/hid-picolcd_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c index b48092d..72bba1e 100644 --- a/drivers/hid/hid-picolcd_core.c +++ b/drivers/hid/hid-picolcd_core.c @@ -290,7 +290,7 @@ static ssize_t picolcd_operation_mode_store(struct device *dev, buf += 10; cnt -= 10; } - if (!report) + if (!report || report-maxfield 1) return -EINVAL; while (cnt 0 (buf[cnt-1] == '\n' || buf[cnt-1] == '\r')) -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] HID: Win 8 multitouch panels detection in core
On Mon, 26 Aug 2013, Srinivas Pandruvada wrote: Srinivas, I still did not added your tested-by as I made minors modifications of the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and patch 2 will not impact this. Maybe just wait for Henrik to put his Reviewed-by before giving one last test. Here goes: Reviewed-by: Henrik Rydberg rydb...@euromail.se Thanks guys. Not applying this just yet, waiting for Tested-by: from Srinivas. Tested-by: Srinivas Pandruvadasrinivas.pandruv...@linux.intel.com Applied, thanks everybody. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] HID: use module_hid_driver() to simplify the code
On Fri, 23 Aug 2013, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn module_hid_driver() makes the code simpler by eliminating boilerplate code. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- drivers/hid/hid-xinmo.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/hid/hid-xinmo.c b/drivers/hid/hid-xinmo.c index 6153e50..7df5227 100644 --- a/drivers/hid/hid-xinmo.c +++ b/drivers/hid/hid-xinmo.c @@ -57,16 +57,5 @@ static struct hid_driver xinmo_driver = { .event = xinmo_event }; -static int __init xinmo_init(void) -{ - return hid_register_driver(xinmo_driver); -} - -static void __exit xinmo_exit(void) -{ - hid_unregister_driver(xinmo_driver); -} - -module_init(xinmo_init); -module_exit(xinmo_exit); +module_hid_driver(xinmo_driver); MODULE_LICENSE(GPL); Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] HID: Win 8 multitouch panels detection in core
On Thu, 22 Aug 2013, Henrik Rydberg wrote: this is the v3 of this patch series. Srinivas, I still did not added your tested-by as I made minors modifications of the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and patch 2 will not impact this. Maybe just wait for Henrik to put his Reviewed-by before giving one last test. Here goes: Reviewed-by: Henrik Rydberg rydb...@euromail.se Thanks guys. Not applying this just yet, waiting for Tested-by: from Srinivas. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 23/49] hid: usbhid: prepare for enabling irq in complete()
On Sun, 18 Aug 2013, Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Jiri Kosina jkos...@suse.cz Cc: linux-input@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Jiri Kosina jkos...@suse.cz --- drivers/hid/usbhid/hid-core.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index bd38cdf..2445fd6 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -489,8 +489,9 @@ static void hid_ctrl(struct urb *urb) struct hid_device *hid = urb-context; struct usbhid_device *usbhid = hid-driver_data; int unplug = 0, status = urb-status; + unsigned long flags; - spin_lock(usbhid-lock); + spin_lock_irqsave(usbhid-lock, flags); switch (status) { case 0: /* success */ @@ -525,7 +526,7 @@ static void hid_ctrl(struct urb *urb) } clear_bit(HID_CTRL_RUNNING, usbhid-iofl); - spin_unlock(usbhid-lock); + spin_unlock_irqrestore(usbhid-lock, flags); usb_autopm_put_interface_async(usbhid-intf); wake_up(usbhid-wait); } -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] hid-sensor-hub: fix style of comments
On Wed, 14 Aug 2013, Andy Shevchenko wrote: This patch fixes the style of the comments to be like following /* The commentary */ There is no functional change. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com Applied. --- drivers/hid/hid-sensor-hub.c | 2 +- include/linux/hid-sensor-hub.h | 2 +- include/linux/hid-sensor-ids.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index ca749810..ffc80cf 100644 --- a/drivers/hid/hid-sensor-hub.c +++ b/drivers/hid/hid-sensor-hub.c @@ -416,7 +416,7 @@ static int sensor_hub_raw_event(struct hid_device *hdev, return 1; ptr = raw_data; - ptr++; /*Skip report id*/ + ptr++; /* Skip report id */ spin_lock_irqsave(pdata-lock, flags); diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h index ecefb73..32ba451 100644 --- a/include/linux/hid-sensor-hub.h +++ b/include/linux/hid-sensor-hub.h @@ -172,7 +172,7 @@ struct hid_sensor_common { struct hid_sensor_hub_attribute_info sensitivity; }; -/*Convert from hid unit expo to regular exponent*/ +/* Convert from hid unit expo to regular exponent */ static inline int hid_sensor_convert_exponent(int unit_expo) { if (unit_expo 0x08) diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h index 6f24446..4f945d3 100644 --- a/include/linux/hid-sensor-ids.h +++ b/include/linux/hid-sensor-ids.h @@ -37,7 +37,7 @@ #define HID_USAGE_SENSOR_ANGL_VELOCITY_Y_AXIS0x200458 #define HID_USAGE_SENSOR_ANGL_VELOCITY_Z_AXIS0x200459 -/*ORIENTATION: Compass 3D: (200083) */ +/* ORIENTATION: Compass 3D: (200083) */ #define HID_USAGE_SENSOR_COMPASS_3D 0x200083 #define HID_USAGE_SENSOR_ORIENT_MAGN_HEADING 0x200471 #define HID_USAGE_SENSOR_ORIENT_MAGN_HEADING_X 0x200472 -- 1.8.4.rc2 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: Fix Speedlink VAD Cezanne support for some devices
On Sun, 25 Aug 2013, Stefan Kriwanek wrote: Some devices of the Speedlink VAD Cezanne model need more aggressive fixing than already done. Applied, thanks Stefan. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] HID: hidraw: Add spinlock in struct hidraw to protect list
On Thu, 15 Aug 2013, Yonghua Zheng wrote: As hidraw_report_event can be called from interrupt context, it is a mistake to use mutex_lock for protecting the list member in my previous patch, so update the patch which adds a spinlock in struct hidraw to protect the list member from concurrent access: Hi, thanks for the patch. I already have commit 212a871a3934beccf43431608c27ed2e05a476ec Author: Manoj Chourasia mchoura...@nvidia.com Date: Mon Jul 22 15:33:13 2013 +0530 HID: hidraw: correctly deallocate memory on device disconnect in the tree, which collides with your patch. Could you please check what changes are needed on top of it so that it makes sense for my 'for-next' branch, rebase, and resend to me? Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] HID: hidraw: Add spinlock in struct hidraw to protect list
On Mon, 26 Aug 2013, Yonghua Zheng wrote: Hi Jiri, Fix the conflict and update the patch: From 7c06e1f3a5959e73a5b827bda67e7c1eaed7da67 Mon Sep 17 00:00:00 2001 From: Yonghua Zheng younghua.zh...@gmail.com Date: Wed, 14 Aug 2013 17:43:36 +0800 Subject: [PATCH 1/1] HID: hidraw: Add spinlock in struct hidraw to protect member list It is unsafe to call list_for_each_entry in hidraw_report_event to traverse each hidraw_list node without a lock protection, the list could be modified if someone calls hidraw_release and list_del to remove itself from the list, this can cause hidraw_report_event to touch a deleted list struct and panic. To prevent this, introduce a spinlock in struct hidraw to protect list from concurrent access. Signed-off-by: Yonghua Zheng younghua.zh...@gmail.com Applied now, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] input: document gamepad API and add extra keycodes
On Fri, 16 Aug 2013, David Herrmann wrote: Hm, I just noticed that this got merged without the Documentation/input/gamepad.txt description. Was this intentional? https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d09bbfd2a8408a995419dff0d2ba906013cf4cc9 My git-fu horribly failed in this case, sorry for that. Now queued, thanks a lot for noticing. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] HID: hid-logitech-dj, querying_devices was never set
On Tue, 6 Aug 2013, Sune Mølgaard wrote: Being affected by this bug, I can confirm that Linux 3.11-rc4 still exhibits the unwanted behaviour for me, but that commenting out the single line from the second patch makes it work. Thus, for requesting a revert on that line, you are most welcome to put me down as a Tested-By. Okay, this is now queued, and I will be pushing it to Linus. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: List corruption in hidraw_release in 3.11-rc4
On Wed, 7 Aug 2013, Peter Wu wrote: does the patch below fix the problem you are seeing? That one is already in 3.11-rc4 as far as I can see. Also, that code can probably simplified by moving the mutex_unlock after the out label, removing the need to duplicate the mutex_unlock. Remember what I said about no Oopses? Well, it turned out that several memory structures were damaged which causes a general protection fault in sock_alloc_inode and other places. I managed to create a program that can reproduce this bug 100% in a QEMU virtual machine with a Logitech USB receiver passed to it. qemu-system-x86_64 -enable-kvm -m 1G -usb -usbdevice host:046d:c52b (pass -kernel, -initrd, -append as needed) Copy hidraw-test to initrd, boot QEMU and run `hidraw-test`. Result: instant (= +/- 2 seconds) crash. I have applied Manoj's patch[1] on top of 3.11-rc4 which seem to fix the issue. One observation is that the new device is named /dev/hidraw1 instead of /dev/hidraw0. Example: f(){ hidraw-test /dev/hidraw$1 usb1;} # needed for 3.11-rc4 f 1; f 1 # crash # needed for 3.11-rc4 + patch f 1; f 2 # ok Regards, Peter [1]: http://lkml.org/lkml/2013/7/22/248 That one I am still reviewing ... can I add your Tested-by: to it when I'll be applying it and pushing to Linus? Thanks. -- /* cc hidraw-test.c -o hidraw-test * hidraw-test /dev/hidraw0 usb1; hidraw-test /dev/hidraw0 usb1; */ #include unistd.h #include fcntl.h #include stdio.h #include errno.h #include string.h #include stdlib.h int open_and_write(const char *path, const char *data) { int sfd, r; sfd = open(path, O_WRONLY); if (sfd 0) { perror(path); return 1; } r = write(sfd, data, strlen(data)); if (r 0) { fprintf(stderr, write(%s, %s): %s\n, path, data, strerror(errno)); return 1; } close(sfd); return 0; } int dork(const char *hiddev, const char *name) { int fd; char c; fd = open(hiddev, O_RDWR | O_NONBLOCK); if (fd 0) { perror(open); return 1; } if (open_and_write(/sys/bus/usb/drivers/usb/unbind, name)) return 1; // does not make a difference //sleep(1); if (open_and_write(/sys/bus/usb/drivers/usb/bind, name)) return 1; // allow devices to get discovered sleep(1); printf(read() = %zi\n, read(fd, c, 1)); perror(read); close(fd); return 0; } int main(int argc, char **argv) { if (argc 3) { fprintf(stderr, Usage: %s /dev/hidrawN usbN\n, *argv); return 1; } system(modprobe -v usbhid); system(modprobe -v hid-logitech-dj); dork(argv[1], argv[2]); return 0; } -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: List corruption in hidraw_release in 3.11-rc4
On Tue, 6 Aug 2013, Peter Wu wrote: While debugging upowerd (with Logitech Unifying receiver via hidraw), I came across this list corruption warning. Peter, does the patch below fix the problem you are seeing? --- drivers/hid/hidraw.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c index a745163..6f1feb2 100644 --- a/drivers/hid/hidraw.c +++ b/drivers/hid/hidraw.c @@ -518,7 +518,6 @@ int hidraw_connect(struct hid_device *hid) goto out; } - mutex_unlock(minors_lock); init_waitqueue_head(dev-wait); INIT_LIST_HEAD(dev-list); @@ -528,6 +527,7 @@ int hidraw_connect(struct hid_device *hid) dev-exist = 1; hid-hidraw = dev; + mutex_unlock(minors_lock); out: return result; -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-hid: don't push static constants on stack for %*ph
On Fri, 2 Aug 2013, Benjamin Tissoires wrote: andriy.shevche...@linux.intel.com wrote: There is no need to pass constants via stack. The width may be explicitly specified in the format. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com Reviewed-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid-holtekff: don't push static constants on stack for %*ph
On Fri, 2 Aug 2013, Andy Shevchenko wrote: There is no need to pass constants via stack. The width may be explicitly specified in the format. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/hid/hid-holtekff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-holtekff.c b/drivers/hid/hid-holtekff.c index 9a8f051..9325545 100644 --- a/drivers/hid/hid-holtekff.c +++ b/drivers/hid/hid-holtekff.c @@ -98,7 +98,7 @@ static void holtekff_send(struct holtekff_device *holtekff, holtekff-field-value[i] = data[i]; } - dbg_hid(sending %*ph\n, 7, data); + dbg_hid(sending %7ph\n, data); hid_hw_request(hid, holtekff-field-report, HID_REQ_SET_REPORT); Applied, thanks Andy. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: wiimote: work around broken DRM_KAI on GEN10
On Sun, 4 Aug 2013, David Herrmann wrote: GEN10 and earlier devices seem to not support DRM_KAI if we run in basic IR mode. Use DRM_KAIE instead. This might increases overhead slightly as the extension port is read and streamed but we stream accelerometer data constantly, too, so this is negligible. Note that our parsers are hardcoded on IR-formats, so we cannot actually use 96-bit IR DRMs for basic IR data. We would have to adjust the parsers. But as only GEN20 and newer support this, we simply avoid mixed DRMs. This fixes a bug where GEN10 devices didn't provide IR data if accelerometer and IR are enabled simultaneously. As a workaround, you can enable DRM_KAIE without this patch via (disables device power-management): echo 37 /sys/kernel/debug/hid/dev/drm Signed-off-by: David Herrmann dh.herrm...@gmail.com Reported-by: Nicolas Adenis-Lamarre nicolas.adenis.lama...@gmail.com Applied, thanks. --- drivers/hid/hid-wiimote-core.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c index 0c06054..6602098 100644 --- a/drivers/hid/hid-wiimote-core.c +++ b/drivers/hid/hid-wiimote-core.c @@ -212,10 +212,12 @@ static __u8 select_drm(struct wiimote_data *wdata) if (ir == WIIPROTO_FLAG_IR_BASIC) { if (wdata-state.flags WIIPROTO_FLAG_ACCEL) { - if (ext) - return WIIPROTO_REQ_DRM_KAIE; - else - return WIIPROTO_REQ_DRM_KAI; + /* GEN10 and ealier devices bind IR formats to DRMs. + * Hence, we cannot use DRM_KAI here as it might be + * bound to IR_EXT. Use DRM_KAIE unconditionally so we + * work with all devices and our parsers can use the + * fixed formats, too. */ + return WIIPROTO_REQ_DRM_KAIE; } else { return WIIPROTO_REQ_DRM_KIE; } -- 1.8.3.4 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: add driver for IBM/Lenovo ScrollPoint mice
/drivers/hid/hid-scrollpoint.c @@ -0,0 +1,67 @@ +/* + * HID driver for IBM/Lenovo ScrollPoint mice + * + * Copyright (c) 2012 Peter De Wachter pdewa...@gmail.com + */ + +/* + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + */ + +#include linux/device.h +#include linux/hid.h +#include linux/module.h + +#include hid-ids.h + +static int scrollpoint_input_mapping(struct hid_device *hdev, + struct hid_input *hi, struct hid_field *field, + struct hid_usage *usage, unsigned long **bit, int *max) +{ + if (usage-hid == HID_GD_Z) { + hid_map_usage(hi, usage, bit, max, EV_REL, REL_HWHEEL); + return 1; + } + return 0; +} + +static const struct hid_device_id scrollpoint_devices[] = { + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, + USB_DEVICE_ID_IBM_SCROLLPOINT_III) }, + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, + USB_DEVICE_ID_IBM_SCROLLPOINT_PRO) }, + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, + USB_DEVICE_ID_IBM_SCROLLPOINT_OPTICAL) }, + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, + USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL) }, + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, + USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL_PRO) }, + { } +}; +MODULE_DEVICE_TABLE(hid, scrollpoint_devices); + +static struct hid_driver scrollpoint_driver = { + .name = scrollpoint, + .id_table = scrollpoint_devices, + .input_mapping = scrollpoint_input_mapping +}; + +static int __init scrollpoint_init(void) +{ + return hid_register_driver(scrollpoint_driver); +} + +static void __exit scrollpoint_exit(void) +{ + hid_unregister_driver(scrollpoint_driver); +} + +module_init(scrollpoint_init); +module_exit(scrollpoint_exit); + +MODULE_AUTHOR(Peter De Wachter); +MODULE_DESCRIPTION(IBM/Lenovo ScrollPoint mouse driver); +MODULE_LICENSE(GPL); -- 1.8.3.2 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] HID: hid-logitech-dj, querying_devices was never set
On Fri, 2 Aug 2013, Benjamin Tissoires wrote: Could you please elaborate? (and put an elaborate description to revert commit log perhaps?) Sure, so here is the revert commit log: -- Commit HID: hid-logitech-dj, querying_devices was never set activate a flag which guarantees that we do not ask the receiver for too many enumeration. When the flag is set, each following enumeration call is discarded (the usb request is not forwarded to the receiver). The flag is then released when the driver receive a pairing information event, which normally follows the enumeration request. However, the USB3 bug makes the driver think the enumeration request has been forwarded to the receiver. However, it is actually not the case because the USB stack returns -EPIPE. So, when a new unknown device appears, the workaround consisting in asking for a new enumeration is not working anymore: this new enumeration is discarded because of the flag, which is never reset. A solution could be to trigger a timeout before releasing it, but for now, let's just revert the patch. -- Thanks Benjamin. I'd like to have a bit more clarification about the USB3 bug, as this whole issue is not completely clear to me. To be more specific -- when exactly do we receive -EPIPE, why do we receive it and why do we not handle it properly? Thanks again, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/8] HID: usbhid: update LED fields unlocked
On Tue, 16 Jul 2013, Benjamin Tissoires wrote: Report fields can be updated from HID drivers unlocked via hid_set_field(). It is protected by input_lock in HID core so only a single input event is handled at a time. USBHID can thus update the field unlocked and doesn't conflict with any HID vendor/device drivers. Note, many HID drivers make heavy use of hid_set_field() in that way. But usbhid also schedules a work to gather multiple LED changes in a single report. Hence, we used to lock the LED field update so the work can read a consistent state. However, hid_set_field() only writes a single integer field, which is guaranteed to be allocated all the time. So the worst possible race-condition is a garbage read on the LED field. Therefore, there is no need to protect the update. In fact, the only thing that is prevented by locking hid_set_field(), is an LED update while the scheduled work currently writes an older LED update out. However, this means, a new work is scheduled directly when the old one is done writing the new state to the device. So we actually _win_ by not protecting the write and allowing the write to be combined with the current write. A new worker is still scheduled, but will not write any new state. So the LED will not blink unnecessarily on the device. Assume we have the LED set to 0. Two request come in which enable the LED and immediately disable it. The current situation with two CPUs would be: usb_hidinput_input_event() | hid_led() -+-- spin_lock(usbhid-lock); hid_set_field(1); spin_unlock(usbhid-lock); schedule_work(...); spin_lock(usbhid-lock); __usbhid_submit_report(..1..); spin_unlock(usbhid-lock); spin_lock(usbhid-lock); hid_set_field(0); spin_unlock(usbhid-lock); schedule_work(...); spin_lock(usbhid-lock); __usbhid_submit_report(..0..); spin_unlock(usbhid-lock); With the locking removed, we _might_ end up with (look at the changed __usbhid_submit_report() parameters in the first try!): usb_hidinput_input_event() | hid_led() -+-- hid_set_field(1); schedule_work(...); spin_lock(usbhid-lock); hid_set_field(0); schedule_work(...); __usbhid_submit_report(..0..); spin_unlock(usbhid-lock); ... next work ... spin_lock(usbhid-lock); __usbhid_submit_report(..0..); spin_unlock(usbhid-lock); As one can see, we no longer send the LED ON signal as it is disabled immediately afterwards and the following LED OFF request overwrites the pending LED ON. It is important to note that hid_set_field() is not atomic, so we might also end up with any other value. But that doesn't matter either as we _always_ schedule the next work with a correct value and schedule_work() acts as memory barrier, anyways. So in the worst case, we run __usbhid_submit_report(..garbage..) in the first case and the following __usbhid_submit_report() will write the correct value. But LED states are booleans so any garbage will be converted to either 0 or 1 and the remote device will never see invalid requests. Why all this? It avoids any custom locking around hid_set_field() in usbhid and finally allows us to provide a generic hidinput_input_event() handler for all HID transport drivers. Signed-off-by: David Herrmann dh.herrm...@gmail.com --- Hi David, that was a very good commit message! Reviewed-by: Benjamin Tissoires benjamin.tissoi...@redhat.com I love this patch :) Thanks David, thanks Benjamin. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 3/8] HID: input: generic hidinput_input_event handler
On Wed, 17 Jul 2013, David Herrmann wrote: + + /* led_work is spawned by input_dev callbacks, but doesn't access the +* parent input_dev at all. Once all input devices are removed, we +* know that led_work will never get restarted, so we can cancel it +* synchronously and are safe. */ + cancel_work_sync(hid-led_work); You missed the multi-lines comment formatting style on this one :) The ./net/ subsystem uses these comments quite a lot and there was a discussion between davem and linus with the conclusion that these comments are ok. But I actually don't care, so I can change to normal CodingStyle. I once got grilled by Dave for submitting patch to netdev with such comment, but that didn't change my opinion, and I don't care for my subsystem :) -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/8] HID: Transport Driver Cleanup
On Wed, 31 Jul 2013, David Herrmann wrote: This series provides some cleanups for HID transport drivers: Hi David, thanks a lot for your work, again. I have now applied all the patches from the series, except for: - 3/8, waiting for v2 Ugh, patches 4-6 use the generic helper from 3 implicitly. So with the current tree, they will not be able to send any LED events. I think my conclusion in the discussion on #3 was wrong. I cannot rebase it on the end of the series. Sorry, I missed that. For v2, as mentioned in the thread, I cannot write a generic fallback for hid_hw_request() without patch #8. That's because the transport drivers handle the raw reports differently. #8 fixes that. So imho we should apply #3 as it is now. If we ever have a generic hid_hw_request(), we can just move the code from the LED handler to hid_hw_request_generic(). Bah, brainfart on my side as well. You are right. I am now pulling 3/8 into the tree as well and pushing it out. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: i2c-hid: add DT bindings
On Wed, 24 Jul 2013, Benjamin Tissoires wrote: Add device tree based support for HID over I2C devices. Tested on an Odroid-X board with a Synaptics touchpad. Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com --- Hi guys, well, as the commit message says, this is the DT binding for HID over I2C. I honestly don't know if it will be used besides me, but it may help others with a DT based board. As the spec is for ACPI only, I had no specifications regarding the DT names. So these names can be changed if you think they are bad. I also created a new bindings directory in the devicetree doc to reflect the split we have between input and hid. However, if the DT experts prefer having it under input, I'm fine with that. Cheers, Benjamin .../devicetree/bindings/hid/hid-over-i2c.txt | 28 ++ drivers/hid/i2c-hid/i2c-hid.c | 44 +- include/linux/i2c/i2c-hid.h| 3 +- 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/hid/hid-over-i2c.txt diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt DT folks, any objections to this, please? If not, I'd like to apply this for 3.11 into my tree. DT folks, ping??? As there have been no objections for more than one month, I am now queuing this up. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux-next] hid/logitech-dj: Fix non-atomic kmalloc in logi_dj_ll_input_event()
On Wed, 31 Jul 2013, Peter Hurley wrote: The ll_driver's .hidinput_input_event() method is called from atomic context [1]. Use GFP_ATOMIC for allocation of the synthesized hid report. Signed-off-by: Peter Hurley pe...@hurleysoftware.com Applied, thanks Peter. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hidraw: fix unproper mutex release
On Tue, 30 Jul 2013, yonghua zheng wrote: Hi Jiri, We were seeing kernel crash while connect and disconnect a BT remote controller on Android system and kernel was crashing because of null pointer access in hidraw_open() in line 271 as follows: Hi, thanks a lot for the fix. I'd like to have it pushed to Linus for 3.11 still. However you patch is missing Signed-off-by: line, as documented in Documentation/SubmittingPatches. Could you please provide that, so that I can apply your patch? Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: Add new driver for non-compliant Xin-Mo devices.
On Sat, 27 Jul 2013, oscher...@ithink.ch wrote: From: Olivier Scherler oscher...@ithink.ch The driver currently only supports the Dual Arcade controller. It fixes the negative axis event values (the devices sends -2) to match the logical axis minimum of the HID report descriptor (the report announces -1). It is needed because hid-input discards out of bounds values. Applied, thanks Olivier. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3.11-rc2] HID: sony: fix HID mapping for PS3 sixaxis controller
On Wed, 24 Jul 2013, Benjamin Tissoires wrote: Commit f04d51404f51 (HID: driver for PS2/3 Buzz controllers) introduced an input_mapping() callback, but set the return value to -1 to all devices except the Buzz controllers. The result of this is that the Sixaxis input device is not populated, making it useless. Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com --- Hi Jiri, I have since yesterday a Sixaxis controller on my desk, and I noticed that it was broken in v3.11. Good catch, applied, thanks Benjamin. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: replace strict_strtoul() with kstrtoul()
On Fri, 19 Jul 2013, Jingoo Han wrote: The usage of strict_strtoul() is not preferred, because strict_strtoul() is obsolete. Thus, kstrtoul() should be used. Signed-off-by: Jingoo Han jg1@samsung.com Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
On Fri, 19 Jul 2013, Peter Hurley wrote: As far as getting printk output from a custom kernel, I think that may be beyond the reporter's capability. Perhaps one of the Ubuntu devs triaging this bug could provide a test kernel for the OP with those options on. Joseph, would you be willing to do that? Sure thing. I'll build a kernel and request that the bug reporter collect usbmon data. Thanks Joseph for building the test kernel and getting it to the reporter! Sarah, I've attached the dmesg capture supplied by the original reporter on a 3.10 custom kernel w/ the kbuild options you requested. It seems as if your initial suspicion is correct: [ 46.785490] xhci_hcd :00:14.0: Endpoint 0x81 not halted, refusing to reset. [ 46.785493] xhci_hcd :00:14.0: Endpoint 0x82 not halted, refusing to reset. [ 46.785496] xhci_hcd :00:14.0: Endpoint 0x83 not halted, refusing to reset. [ 46.785952] xhci_hcd :00:14.0: Waiting for status stage event At this point, would you recommend proceeding with the workaround or waiting for an xHCI bug fix? Thanks for your efforts. Seems like this might be a part of the picture, but not a complete one. Linus claims to have similar problem, but his receiver is not connected through xHCI (I got this as an off-list report, so can't really provide a pointer to ML archives). Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
On Thu, 18 Jul 2013, Nestor Lopez Casado wrote: This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887. This patch re-adds the workaround introduced by 596264082f10dd4 which was reverted by 8af6c08830b1ae114. The original patch 596264 was needed to overcome a situation where the hid-core would drop incoming reports while probe() was being executed. This issue was solved by c849a6143bec520af which added hid_device_io_start() and hid_device_io_stop() that enable a specific hid driver to opt-in for input reports while its probe() is being executed. Commit a9dd22b730857347 modified hid-logitech-dj so as to use the functionality added to hid-core. Having done that, workaround 596264 was no longer necessary and was reverted by 8af6c08. We now encounter a different problem that ends up 'again' thwarting the Unifying receiver enumeration. The problem is time and usb controller dependent. Ocasionally the reports sent to the usb receiver to start the paired devices enumeration fail with -EPIPE and the receiver never gets to enumerate the paired devices. [ ... snip ... ] Ok, this is now in git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-3.11/logitech-enumeration-fix Linus added to CC. Linus -- could you please by any chance test whether the two patches in that branch make the problem you are observing any better? (and no, this is not a pull request yet). It's still not clear whether we are chasing two different issues here, or not. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: kye: Add report fixup for Genius Gx Imperator Keyboard
On Fri, 12 Jul 2013, Benjamin Tissoires wrote: Genius Gx Imperator Keyboard presents the same problem in its report descriptors than Genius Gila Gaming Mouse. Use the same fixup for both. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=928561 Reported-and-tested-by: Honza Brazdil jbraz...@redhat.com Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com --- Hi Jiri, I have been reported this broken Genius device too. The patch applies on top of the branch for-3.11/upstream, but feel free to schedule it for 3.11 or 3.12. This time, only the multimedia (and macros) keys are broken. The core keyboard part is working fine without the patch. Cheers, Benjamin drivers/hid/hid-ids.h | 1 + drivers/hid/hid-kye.c | 45 - 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index b2b692e..1f28df9 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -475,6 +475,7 @@ #define USB_VENDOR_ID_KYE0x0458 #define USB_DEVICE_ID_KYE_ERGO_525V 0x0087 #define USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE 0x0138 +#define USB_DEVICE_ID_GENIUS_GX_IMPERATOR0x4018 #define USB_DEVICE_ID_KYE_GPEN_560 0x5003 #define USB_DEVICE_ID_KYE_EASYPEN_I405X 0x5010 #define USB_DEVICE_ID_KYE_MOUSEPEN_I608X 0x5011 diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c index 1e2ee2aa..7384512 100644 --- a/drivers/hid/hid-kye.c +++ b/drivers/hid/hid-kye.c @@ -268,6 +268,26 @@ static __u8 easypen_m610x_rdesc_fixed[] = { 0xC0 /* End Collection */ }; +static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc, + unsigned int *rsize, int offset, const char *device_name) { + /* + * the fixup that need to be done: + * - change Usage Maximum in the Comsumer Control + * (report ID 3) to a reasonable value + */ + if (*rsize = offset + 31 + /* Usage Page (Consumer Devices) */ + rdesc[offset] == 0x05 rdesc[offset + 1] == 0x0c + /* Usage (Consumer Control) */ + rdesc[offset + 2] == 0x09 rdesc[offset + 3] == 0x01 + /* Usage Maximum 12287 */ + rdesc[offset + 10] == 0x2a rdesc[offset + 12] 0x2f) { + hid_info(hdev, fixing up %s report descriptor\n, device_name); + rdesc[offset + 12] = 0x2f; + } + return rdesc; +} + static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { @@ -315,23 +335,12 @@ static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc, } break; case USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE: - /* - * the fixup that need to be done: - * - change Usage Maximum in the Comsumer Control - * (report ID 3) to a reasonable value - */ - if (*rsize = 135 - /* Usage Page (Consumer Devices) */ - rdesc[104] == 0x05 rdesc[105] == 0x0c - /* Usage (Consumer Control) */ - rdesc[106] == 0x09 rdesc[107] == 0x01 - /* Usage Maximum 12287 */ - rdesc[114] == 0x2a rdesc[116] 0x2f) { - hid_info(hdev, - fixing up Genius Gila Gaming Mouse - report descriptor\n); - rdesc[116] = 0x2f; - } + rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 104, + Genius Gila Gaming Mouse); + break; + case USB_DEVICE_ID_GENIUS_GX_IMPERATOR: + rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 83, + Genius Gx Imperator Keyboard); break; } return rdesc; @@ -428,6 +437,8 @@ static const struct hid_device_id kye_devices[] = { USB_DEVICE_ID_KYE_EASYPEN_M610X) }, { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE) }, + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, + USB_DEVICE_ID_GENIUS_GX_IMPERATOR) }, { } }; Hi Benjamin, I guess you are missing update of hid_have_special_driver[] list? -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: logitech-dj: use inlined helpers hid_hw_open/close
On Fri, 12 Jul 2013, Benjamin Tissoires wrote: Use the inlined helpers hid_hw_open/close instead of direct calls to -ll_driver-open() and -ll_driver-close(). Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback
On Thu, 11 Jul 2013, Benjamin Tissoires wrote: We can re-enable hidinput_input_event to allow the leds of bluetooth keyboards to be set. Now the callbacks uses hid core to retrieve the right HID report to send, so this version is safer. Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Acked-by: Jiri Kosina jkos...@suse.cz --- net/bluetooth/hidp/core.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index f13a8da..9c8b50d 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -238,6 +238,31 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep return hidp_send_intr_message(session, hdr, buf, rsize); } +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, +unsigned int code, int value) +{ + struct hid_device *hid = input_get_drvdata(dev); + struct hidp_session *session = hid-driver_data; + struct hid_field *field; + int offset; + + BT_DBG(session %p type %d code %d value %d, +session, type, code, value); + + if (type != EV_LED) + return -1; + + offset = hidinput_find_field(hid, type, code, field); + if (offset == -1) { + hid_warn(dev, event field not found\n); + return -1; + } + + hid_set_field(field, offset, value); + + return hidp_send_report(session, field-report); +} + static int hidp_get_raw_report(struct hid_device *hid, unsigned char report_number, unsigned char *data, size_t count, @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { .stop = hidp_stop, .open = hidp_open, .close = hidp_close, + .hidinput_input_event = hidp_hidinput_event, }; /* This function sets up the hid device. It does not add it -- 1.8.3.1 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init
On Thu, 11 Jul 2013, Benjamin Tissoires wrote: The USB hid implementation does retrieve the reports during the start. However, this implementation does not call the HID command GET_REPORT (which would fetch the current status of each report), but use the DATA command, which is an Output Report (so transmitting data from the host to the device). The Wiimote controller is already guarded against this problem in the protocol, but it is not conformant to the specification to set all the reports to 0 on start. Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Acked-by: Jiri Kosina jkos...@suse.cz Gustavo, could you please take it through your tree? Thanks Benjamin, thanks David. --- net/bluetooth/hidp/core.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 9c8b50d..59d132a 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -703,20 +703,6 @@ static int hidp_parse(struct hid_device *hid) static int hidp_start(struct hid_device *hid) { - struct hidp_session *session = hid-driver_data; - struct hid_report *report; - - if (hid-quirks HID_QUIRK_NO_INIT_REPORTS) - return 0; - - list_for_each_entry(report, hid-report_enum[HID_INPUT_REPORT]. - report_list, list) - hidp_send_report(session, report); - - list_for_each_entry(report, hid-report_enum[HID_FEATURE_REPORT]. - report_list, list) - hidp_send_report(session, report); - return 0; } -- 1.8.3.1 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: fix data access in implement()
On Wed, 10 Jul 2013, Benjamin Tissoires wrote: implement() is setting bytes in LE data stream. In case the data is not aligned to 64bits, it reads past the allocated buffer. It doesn't really change any value there (it's properly bitmasked), but in case that this read past the boundary hits a page boundary, pagefault happens when accessing 64bits of 'x' in implement(), and kernel oopses. This happens much more often when numbered reports are in use, as the initial 8bit skip in the buffer makes the whole process work on values which are not aligned to 64bits. This problem dates back to attempts in 2005 and 2006 to make implement() and extract() as generic as possible, and even back then the problem was realized by Adam Kroperlin, but falsely assumed to be impossible to cause any harm: http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html I have made several attempts at fixing it on the spot directly in implement(), but the results were horrible; the special casing for processing last 64bit chunk and switching to different math makes it unreadable mess. I therefore took a path to allocate a few bytes more which will never make it into final report, but are there as a cushion for all the 64bit math operations happening in implement() and extract(). All callers of hid_output_report() are converted at the same time to allocate the buffer by newly introduced hid_alloc_report_buf() helper. Signed-off-by: Jiri Kosina jkos...@suse.cz --- Thanks for that. It should (I hope) fix the bugs we are seeing from time to times under Fedora and that I was not able to answer: https://bugzilla.redhat.com/show_bug.cgi?id=965280 https://bugzilla.redhat.com/show_bug.cgi?id=927488 https://bugzilla.redhat.com/show_bug.cgi?id=881504 965280 and 881504 absolutely look like an incarnation of this very problem. The faulting address is pagesize aligned (i.e. the loop just crossed the boundary), and the faulting code is the put_unaligned_access_le64(). Exactly the same as the report I had. 927488 seems like a different bug to me -- not an address on page boundary, and we're not faulting in implement(). I have a small remark for usbhid: drivers/hid/hid-core.c| 19 ++- drivers/hid/hid-logitech-dj.c | 12 ++-- drivers/hid/hid-picolcd_debugfs.c | 10 +- drivers/hid/usbhid/hid-core.c |4 ++-- include/linux/hid.h |1 + net/bluetooth/hidp/core.c | 14 +- 6 files changed, 49 insertions(+), 11 deletions(-) [snipped] diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 9941828..2b0b96daf 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -546,7 +546,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re return; } - usbhid-out[usbhid-outhead].raw_report = kmalloc(len, GFP_ATOMIC); + usbhid-out[usbhid-outhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC); if (!usbhid-out[usbhid-outhead].raw_report) { hid_warn(hid, output queueing failed\n); return; @@ -595,7 +595,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re } if (dir == USB_DIR_OUT) { - usbhid-ctrl[usbhid-ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC); + usbhid-ctrl[usbhid-ctrlhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC); line 538: int len = ((report-size - 1) 3) + 1 + (report-id 0); is not used anywhere after applying the patch. It can be dropped. Good catch. Besides the picolcd problems spotted by Bruno: Reviewed-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] HID: fix data access in implement()
implement() is setting bytes in LE data stream. In case the data is not aligned to 64bits, it reads past the allocated buffer. It doesn't really change any value there (it's properly bitmasked), but in case that this read past the boundary hits a page boundary, pagefault happens when accessing 64bits of 'x' in implement(), and kernel oopses. This happens much more often when numbered reports are in use, as the initial 8bit skip in the buffer makes the whole process work on values which are not aligned to 64bits. This problem dates back to attempts in 2005 and 2006 to make implement() and extract() as generic as possible, and even back then the problem was realized by Adam Kroperlin, but falsely assumed to be impossible to cause any harm: http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html I have made several attempts at fixing it on the spot directly in implement(), but the results were horrible; the special casing for processing last 64bit chunk and switching to different math makes it unreadable mess. I therefore took a path to allocate a few bytes more which will never make it into final report, but are there as a cushion for all the 64bit math operations happening in implement() and extract(). All callers of hid_output_report() are converted at the same time to allocate the buffer by newly introduced hid_alloc_report_buf() helper. Bruno noticed that the whole raw_size test can be dropped as well, as hid_alloc_report_buf() makes sure that the buffer is always of a proper size. Reviewed-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Acked-by: Gustavo Padovan gustavo.pado...@collabora.co.uk Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/hid/hid-core.c| 19 ++- drivers/hid/hid-logitech-dj.c | 12 ++-- drivers/hid/hid-picolcd_debugfs.c | 23 --- drivers/hid/usbhid/hid-core.c |5 ++--- include/linux/hid.h |1 + net/bluetooth/hidp/core.c | 14 +- 6 files changed, 52 insertions(+), 22 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 264f550..987279b 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1128,7 +1128,8 @@ static void hid_output_field(const struct hid_device *hid, } /* - * Create a report. + * Create a report. 'data' has to be allocated using + * hid_alloc_report_buf() so that it has proper size. */ void hid_output_report(struct hid_report *report, __u8 *data) @@ -1145,6 +1146,22 @@ void hid_output_report(struct hid_report *report, __u8 *data) EXPORT_SYMBOL_GPL(hid_output_report); /* + * Allocator for buffer that is going to be passed to hid_output_report() + */ +u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags) +{ + /* +* 7 extra bytes are necessary to achieve proper functionality +* of implement() working on 8 byte chunks +*/ + + int len = ((report-size - 1) 3) + 1 + (report-id 0) + 7; + + return kmalloc(len, flags); +} +EXPORT_SYMBOL_GPL(hid_alloc_report_buf); + +/* * Set a field value. The report this field belongs to has to be * created and transferred to the device, to set this value in the * device. diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index 5207591a..4d79273 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -574,7 +574,7 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type, struct hid_field *field; struct hid_report *report; - unsigned char data[8]; + unsigned char *data; int offset; dbg_hid(%s: %s, type:%d | code:%d | value:%d\n, @@ -590,6 +590,13 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type, return -1; } hid_set_field(field, offset, value); + + data = hid_alloc_report_buf(field-report, GFP_KERNEL); + if (!data) { + dev_warn(dev-dev, failed to allocate report buf memory\n); + return -1; + } + hid_output_report(field-report, data[0]); output_report_enum = dj_rcv_hiddev-report_enum[HID_OUTPUT_REPORT]; @@ -600,8 +607,9 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type, hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT); - return 0; + kfree(data); + return 0; } static int logi_dj_ll_start(struct hid_device *hid) diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c index 59ab8e1..024cdf3 100644 --- a/drivers/hid/hid-picolcd_debugfs.c +++ b/drivers/hid/hid-picolcd_debugfs.c @@ -394,7 +394,7 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data, void picolcd_debug_out_report(struct picolcd_data *data, struct hid_device *hdev, struct hid_report *report) { - u8 raw_data[70]; + u8
[PATCH] HID: fix data access in implement()
implement() is setting bytes in LE data stream. In case the data is not aligned to 64bits, it reads past the allocated buffer. It doesn't really change any value there (it's properly bitmasked), but in case that this read past the boundary hits a page boundary, pagefault happens when accessing 64bits of 'x' in implement(), and kernel oopses. This happens much more often when numbered reports are in use, as the initial 8bit skip in the buffer makes the whole process work on values which are not aligned to 64bits. This problem dates back to attempts in 2005 and 2006 to make implement() and extract() as generic as possible, and even back then the problem was realized by Adam Kroperlin, but falsely assumed to be impossible to cause any harm: http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html I have made several attempts at fixing it on the spot directly in implement(), but the results were horrible; the special casing for processing last 64bit chunk and switching to different math makes it unreadable mess. I therefore took a path to allocate a few bytes more which will never make it into final report, but are there as a cushion for all the 64bit math operations happening in implement() and extract(). All callers of hid_output_report() are converted at the same time to allocate the buffer by newly introduced hid_alloc_report_buf() helper. Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/hid/hid-core.c| 19 ++- drivers/hid/hid-logitech-dj.c | 12 ++-- drivers/hid/hid-picolcd_debugfs.c | 10 +- drivers/hid/usbhid/hid-core.c |4 ++-- include/linux/hid.h |1 + net/bluetooth/hidp/core.c | 14 +- 6 files changed, 49 insertions(+), 11 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 264f550..987279b 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1128,7 +1128,8 @@ static void hid_output_field(const struct hid_device *hid, } /* - * Create a report. + * Create a report. 'data' has to be allocated using + * hid_alloc_report_buf() so that it has proper size. */ void hid_output_report(struct hid_report *report, __u8 *data) @@ -1145,6 +1146,22 @@ void hid_output_report(struct hid_report *report, __u8 *data) EXPORT_SYMBOL_GPL(hid_output_report); /* + * Allocator for buffer that is going to be passed to hid_output_report() + */ +u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags) +{ + /* +* 7 extra bytes are necessary to achieve proper functionality +* of implement() working on 8 byte chunks +*/ + + int len = ((report-size - 1) 3) + 1 + (report-id 0) + 7; + + return kmalloc(len, flags); +} +EXPORT_SYMBOL_GPL(hid_alloc_report_buf); + +/* * Set a field value. The report this field belongs to has to be * created and transferred to the device, to set this value in the * device. diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index 5207591a..4d79273 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -574,7 +574,7 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type, struct hid_field *field; struct hid_report *report; - unsigned char data[8]; + unsigned char *data; int offset; dbg_hid(%s: %s, type:%d | code:%d | value:%d\n, @@ -590,6 +590,13 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type, return -1; } hid_set_field(field, offset, value); + + data = hid_alloc_report_buf(field-report, GFP_KERNEL); + if (!data) { + dev_warn(dev-dev, failed to allocate report buf memory\n); + return -1; + } + hid_output_report(field-report, data[0]); output_report_enum = dj_rcv_hiddev-report_enum[HID_OUTPUT_REPORT]; @@ -600,8 +607,9 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type, hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT); - return 0; + kfree(data); + return 0; } static int logi_dj_ll_start(struct hid_device *hid) diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c index 59ab8e1..784a17c 100644 --- a/drivers/hid/hid-picolcd_debugfs.c +++ b/drivers/hid/hid-picolcd_debugfs.c @@ -394,7 +394,7 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data, void picolcd_debug_out_report(struct picolcd_data *data, struct hid_device *hdev, struct hid_report *report) { - u8 raw_data[70]; + u8 *raw_data; int raw_size = (report-size 3) + 1; char *buff; #define BUFF_SZ 256 @@ -407,11 +407,18 @@ void picolcd_debug_out_report(struct picolcd_data *data, if (!buff) return; + raw_data = hid_alloc_report_buf(report, GFP_ATOMIC
Re: Logitech M705: Thumb button (was Re: [PATCH v4] HID: Add full support for Logitech Unifying receivers)
On Tue, 9 Jul 2013, Benjamin Tissoires wrote: For those who wants to try, I have set up a project [1] to implement part of the spec Nestor released. I implemented various features: list devices paired to a receiver, pair/unpair a device, and toggle the button 6 on M705 mouse with the faulty firmware. When switching mode for the M705, as Nestor said, we lose the AC panning ability until the correct remapping is done in the kernel. Cheers, Benjamin [1] https://github.com/bentiss/uLogitech Thanks for your work on this, Benjamin. I have a followup question -- why not do this in the kernel completely though? I have actually been asked by Linus directly (so we'd rather do that :) ) to include pairing trigger as a sysfs toggle instead of having to use userspace utilitites, and it actually makes sense to me. Are you aware of any particular reason to not have all that's provided by uLogitech in the kernel? Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: My logitech wireless keyboard and mouse stop working after 3.10 kernel
On Sat, 6 Jul 2013, wujun zhou wrote: Dear maintainers, After I switch to the 3.10 kernel, my logitech wireless keyboard and mouse stop working. The keyboard and mouse are connected to the desktop though logitech unifying receiver. This little problem almost makes my desktop totally unusable. Could someone check and fix the problem? Thanks Keywords: hid, input, logitech unifying receiver Version: Linux version 3.10.0 (gcc version 4.7.3 (Gentoo 4.7.3 p1.0, pie-0.5.5) ) #6 SMP Sat Jul 6 21:29:14 PDT 2013 [ adding some CCs ] Hi, you are not the first one reporting problems getting worse with later kernels with the unifying receiver. Does the keyboard/mouse get properly discovered if you unplug and replug the receiver? Linus reported that sometimes this is the case for him as well. Quoting my response to him two days ago: === we have had such reports before, but commit a9dd22b730 (and subsequent revert of workaround fone by 59626408) was supposed to fix exactly that. Any idea when this started happening to you? What if you revert back to the 59626408 workaround (i.e. revert a9dd22b730 and 8af6c0883)? I have a couple of unifying receivers myself, but was never able to see this problem ... so it's either rather timing sensitive, or device-specific. === In case the model matters: My mouse: logitech M705 Keyboard: logitech K270 The logitech unifying receiver came with one of them. (Sorry, I can't remember exactly which one it belong to) The driver stop working after this commit 4f5a81042909fed6977881f22c024aa3582cfcca. The previous version 83a44ac8bf4a8e6cbbf0c00ff281a482778f708a works. And I think it is caused by the changes to drivers/hid/hid-logitech-dj.c I get the following dmesg and /proc/bus/input/devices using 3.10 kernel [5.523741] logitech-djreceiver 0003:046D:C52B.0005: hiddev0,hidraw2: USB HID v1.11 Device [Logitech USB Receiver] on usb-:00:14.0-9.2/input2 $ cat /proc/bus/input/devices ... I: Bus=0019 Vendor= Product=0006 Version= N: Name=Video Bus P: Phys=LNXVIDEO/video/input0 S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input2 U: Uniq= H: Handlers=kbd event2 B: PROP=0 B: EV=3 B: KEY=3e000b 0 0 0 The kernel find the receiver but not the connected keyboard and mouse. The dmesg and /proc/bus/input/devices when everything works: commit 83a44ac8bf4a8e6cbbf0c00ff281a482778f708a [7.479464] logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID v1.11 Device [Logitech USB Receiver] on usb-:00:14.0-9.2/input2 [ 33.072330] input: Logitech Unifying Device. Wireless PID:101b as /devices/pci:00/:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input3 [ 33.072550] logitech-djdevice 0003:046D:C52B.0004: input,hidraw1: USB HID v1.11 Mouse [Logitech Unifying Device. Wireless PID:101b] on usb-:00:14.0-9.2:1 [ 33.074266] input: Logitech Unifying Device. Wireless PID:4003 as /devices/pci:00/:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input4 [ 33.074380] logitech-djdevice 0003:046D:C52B.0005: input,hidraw2: USB HID v1.11 Keyboard [Logitech Unifying Device. Wireless PID:4003] on usb-:00:14.0-9.2:2 $ cat /proc/bus/input/devices I: Bus=0019 Vendor= Product=0001 Version= N: Name=Power Button P: Phys=PNP0C0C/button/input0 S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0C0C:00/input/input0 U: Uniq= H: Handlers=kbd event0 B: PROP=0 B: EV=3 B: KEY=10 0 I: Bus=0019 Vendor= Product=0001 Version= N: Name=Power Button P: Phys=LNXPWRBN/button/input0 S: Sysfs=/devices/LNXSYSTM:00/LNXPWRBN:00/input/input1 U: Uniq= H: Handlers=kbd event1 B: PROP=0 B: EV=3 B: KEY=10 0 I: Bus=0019 Vendor= Product=0006 Version= N: Name=Video Bus P: Phys=LNXVIDEO/video/input0 S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input2 U: Uniq= H: Handlers=kbd event2 B: PROP=0 B: EV=3 B: KEY=3e000b 0 0 0 I: Bus=0003 Vendor=046d Product=c52b Version=0111 N: Name=Logitech Unifying Device. Wireless PID:101b P: Phys=usb-:00:14.0-9.2:1 S: Sysfs=/devices/pci:00/:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input3 U: Uniq= H: Handlers=mouse0 event3 B: PROP=0 B: EV=17 B: KEY= 0 0 0 0 B: REL=143 B: MSC=10 I: Bus=0003 Vendor=046d Product=c52b Version=0111 N: Name=Logitech Unifying Device. Wireless PID:4003 P: Phys=usb-:00:14.0-9.2:2 S: Sysfs=/devices/pci:00/:00:14.0/usb3/3-9/3-9.2/3-9.2:1.2/0003:046D:C52B.0003/input/input4 U: Uniq= H: Handlers=sysrq kbd event4 B: PROP=0 B: EV=12001f B: KEY=4837fff072ff32d bf56 1 30f908b17c007 7bfad941dfff febeffdfffef fffe B: REL=40 B: ABS=1 B: MSC=10 B: LED=1f Please let me know if you need more debug info. Thanks. Best, Wujun Zhou -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send
Re: [PATCH] HID: kye: Add report fixup for Genius Gila Gaming mouse
On Tue, 2 Jul 2013, Benjamin Tissoires wrote: Genius Gila Gaming Mouse presents an obviously wrong report descriptor. the Consumer control (report ID 3) is the following: 0x05, 0x0c,// Usage Page (Consumer Devices) 105 0x09, 0x01,// Usage (Consumer Control)107 0xa1, 0x01,// Collection (Application)109 0x85, 0x03,// Report ID (3) 111 0x19, 0x00,// Usage Minimum (0) 113 0x2a, 0xff, 0x7f, // Usage Maximum (32767) 115 0x15, 0x00,// Logical Minimum (0) 118 0x26, 0xff, 0x7f, // Logical Maximum (32767) 120 0x75, 0x10,// Report Size (16) 123 0x95, 0x03,// Report Count (3) 125 0x81, 0x00,// Input (Data,Arr,Abs) 127 0x75, 0x08,// Report Size (8) 129 0x95, 0x01,// Report Count (1) 131 0x81, 0x01,// Input (Cnst,Arr,Abs) 133 0xc0, // End Collection 135 So the first input whithin this report has a count of 3 but a usage range of 32768. So this value is obviously wrong as it should not be greater than the report count. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=959721 Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com --- Hi Jiri, I know the window just opened and you don't really like receiving patches at that time. Still, my patch queue is growing, and I would like you to consider this patch for 3.11. This one is really easy enough to go into even post -rc1 :) Now applied for the Linus merge I am going to do shortly. Thanks a lot, Benjamin. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: wacom: Intuos4 battery charging changes
On Sat, 29 Jun 2013, Przemo Firszt wrote: Intuos4 WL is separately reporting power supply and battery charging status - now hid-wacom is using that information. Previously hid-wacom was wrongly treating battery charging bit as power supply connected. Now it should report battery charging, battery discharging, battery full and power supply status. Intuos4 WL sends reports when is in use (obvious) and when unplugging power supply. If means that if the device is being charged, but it's not being used it will never report battery full. The same problem happens after the device has been connected, but it's not in use - the battery/ac status will be incorrect. Currently there is no mechanism to ask the device to send a report containing battery/ac status. Signed-off-by: Przemo Firszt prz...@firszt.eu Applied, thanks Przemo. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] input: document gamepad API and add extra keycodes
On Wed, 26 Jun 2013, Dmitry Torokhov wrote: On Sat, Jun 15, 2013 at 03:32:44PM +0200, David Herrmann wrote: --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -507,10 +507,14 @@ struct input_keymap_entry { #define BTN_GAMEPAD0x130 #define BTN_A 0x130 +#define BTN_SOUTH 0x130 Could we do: #define BTN_SOUTH 0x130 #define BTN_A BTN_SOUTH so that it is clear that BTN_A, BTN_B, etc are legacy definitions and not an accidental typos that need their own key codes. Makes sense, will do that modification. Otherwise: Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com Thanks Dmitry. Will be taking it (with the above modification) through my tree. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Logitech M705: Thumb button (was Re: [PATCH v4] HID: Add full support for Logitech Unifying receivers)
[ Frantisek Fuka added to CC ] On Wed, 14 Sep 2011, Benjamin Tissoires wrote: Nice. Does this mean I will be able to use the thumb-button of my M705 mouse in the near future? (Someone asked the same question 1.5 years ago, but nothing happened till then: http://thread.gmane.org/gmane.linux.kernel.input/11791 ) This is quite weird. I was going to say that it will work, but with the M705 I've got at home,this thumb button doesn't work at all (anything is emitted from the usb, even with this driver). But I'm pretty sure I made it work with the one I got at work. I'll give a try on Monday unless people at Logitech gave us the solution. I got some information for you. Apparently, it won't be possible to enable this thumb button right now. To enable it, the Windows driver put the mouse in a kind of debug mode that need to be handled properly, which is not the case as of today. I was able to make it work at home because there were different series of M705, the latest sending the thumb button by default. Let me resurrect this 2 years old thread, as I have received multiple reports since then, stating that some revisions of M705 don't have working thumb button. Question mostly to the Logitech guys: Nestor, Olivier, could you please provide us with the sequence that needs to be sent to (certain revisisons of) M705 to let enter the debug mode, making the thumb button actually emit a USB message? Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] input: document gamepad API and add extra keycodes
On Wed, 19 Jun 2013, Jiri Kosina wrote: On Sat, 15 Jun 2013, David Herrmann wrote: Until today all gamepad input drivers report their data differently. It is nearly impossible to write applications for more than one device in a generic way. Therefore, this patch introduces a uniform gamepad API which will be used for all new drivers. Instead of mapping buttons by their labels, we now map them by position. This allows applications to work with any gamepad regardless of the labels on the buttons. Furthermore, we standardize the ABS_* codes for analog triggers and sticks. For D-Pads the long overdue BTN_DPAD_* codes are introduced. They should be fairly obvious how to use. To avoid confusion, the action buttons now have BTN_EAST/SOUTH/WEST/NORTH aliases. Reported-by: Todd Showalter t...@electronjump.com Signed-off-by: David Herrmann dh.herrm...@gmail.com I definitely need to have Dmitry's Ack for this. If he's OK with 1/2, I am fine with 2/2 patch for wiimote and will happily take it through my tree. Dmitry, please? I'd like to avoid missing next merge window if you're ok with this. Thanks again, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Posiflex 0d3a:a000 touchscreen press not recognized in kernel 3.9.4
On Wed, 19 Jun 2013, Benjamin Tissoires wrote: From: Benjamin Tissoires benjamin.tissoi...@redhat.com Date: Wed, 19 Jun 2013 17:49:05 +0200 Subject: [PATCH] HID: input: fix false positive out of range values Commit 6da7066906e977d42104a859c490f5f9a300488c introduced in 3.3 HID: ignore absolute values which don't fit between logical min and max prevents some Posiflex touch screen to work because they do not provide logical min and max for their buttons. Thus, logical min and max are at 0, discarding the buttons events, and preventing the device to report appropriate X Y. Adding a check on min max solves the problem. Reported-by: Jan Kandziora j...@gmx.de Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com Thanks. I am applying this with Jan's Tested-by: (and I am also extending the comnent preceeding the condition to better explain this case). -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] input: document gamepad API and add extra keycodes
0x21a +#define BTN_DPAD_UP 0x220 +#define BTN_DPAD_DOWN0x221 +#define BTN_DPAD_LEFT0x222 +#define BTN_DPAD_RIGHT 0x223 + #define BTN_TRIGGER_HAPPY0x2c0 #define BTN_TRIGGER_HAPPY1 0x2c0 #define BTN_TRIGGER_HAPPY2 0x2c1 -- 1.8.3.1 -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: multitouch: prevent memleak with the allocated name
On Wed, 29 May 2013, Benjamin Tissoires wrote: mt_free_input_name() was never called during .remove(): hid_hw_stop() removes the hid_input items in hdev-inputs, and so the list is therefore empty after the call. In the end, we never free the special names that has been allocated during .probe(). Restore the original name before freeing it to avoid acessing already freed pointer. Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com I have now applied this one and will send it to Linus for next -rc. If we are going down the path of using devm API, as proposed by Andy, that will require much more throgough review of interaction with input subsystem, so definitely not a late -rc regression fix material. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/26] HID: wiimote: extend driver description
On Sun, 5 May 2013, David Herrmann wrote: The hid-wiimote driver supports more than the Wii Remote. Nintendo produced many devices based on the Wii Remote, which have extension devices built-in. It is not clear to many users, that these devices have anything in common with the Wii Remote, so fix the driver description. This also updates the copyright information for the coming hotplugging rework. I have now applied the whole patchset, with these two exceptions: 1) [RFC 24/26] HID: wiimote: support Nintendo Wii U Pro Controller - It's not completely clear to me whether you have reached conclusion with Todd on this one or not 2) [RFC 26/26] HID/ALSA: wiimote: add speaker support - It'd be nice to get Ack from sound people on this one. Thanks a lot, David. Good work. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: Re: Fwd: Re: [PATCH] HID: hid-sensor-hub: do not process feature reports in raw_event
On Wed, 29 May 2013, Srinivas Pandruvada wrote: Daniel has tested and confirmed that this patch fixes issue. Thanks for confirmation. I have now applied the patch. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: hyperv: Cocci spatch memdup.spatch
On Sat, 1 Jun 2013, Thomas Meyer wrote: Signed-off-by: Thomas Meyer tho...@m3y3r.de --- diff -u -p a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -199,13 +199,11 @@ static void mousevsc_on_receive_device_i if (desc-bLength == 0) goto cleanup; - input_device-hid_desc = kzalloc(desc-bLength, GFP_ATOMIC); + input_device-hid_desc = kmemdup(desc, desc-bLength, GFP_ATOMIC); if (!input_device-hid_desc) goto cleanup; - memcpy(input_device-hid_desc, desc, desc-bLength); - input_device-report_desc_size = desc-desc[0].wDescriptorLength; if (input_device-report_desc_size == 0) { input_device-dev_info_status = -EINVAL; I guess you are going to get some pushback from some maintainers due to non-descriptive subject and missing changelog :) I have fixed this for the hyperv patch and applied it, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] HID: holtek-mouse: use module_hid_driver() to simplify the code
On Wed, 29 May 2013, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn module_hid_driver() makes the code simpler by eliminating boilerplate code. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- drivers/hid/hid-holtek-mouse.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/hid/hid-holtek-mouse.c b/drivers/hid/hid-holtek-mouse.c index 6a23ee6..7e6db3c 100644 --- a/drivers/hid/hid-holtek-mouse.c +++ b/drivers/hid/hid-holtek-mouse.c @@ -73,16 +73,5 @@ static struct hid_driver holtek_mouse_driver = { .report_fixup = holtek_mouse_report_fixup, }; -static int __init holtek_mouse_init(void) -{ - return hid_register_driver(holtek_mouse_driver); -} - -static void __exit holtek_mouse_exit(void) -{ - hid_unregister_driver(holtek_mouse_driver); -} - -module_exit(holtek_mouse_exit); -module_init(holtek_mouse_init); +module_hid_driver(holtek_mouse_driver); MODULE_LICENSE(GPL); Added Reported-by: Andy Shevchenko andy.shevche...@gmail.com and applied, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Bluetooth: hidp: register HID devices async
On Tue, 28 May 2013, Gustavo Padovan wrote: Signed-off-by: David Herrmann dh.herrm...@gmail.com Acked-by: Jiri Kosina jkos...@suse.cz Gustavo, I think I'd like to take this patch together with the ENODATA change for hid-input, as they, in some sense, stick together. If you are OK with that, could you please provide me with your Acked-by, and I'll take it through my tree? This looks good to me. Acked-by: Gustavo Padovan gustavo.pado...@collabora.co.uk Perfect, I am taking it through my tree. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: input: return ENODATA if reading battery attrs fails
On Fri, 24 May 2013, David Herrmann wrote: So I hope we can apply this for linux-next (probably after gustavo applied the HIDP fix)? Applied to my tree, together with the hidp fix. Thanks David, thanks Daniel! -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: hid-sensor-hub: do not process feature reports in raw_event
On Wed, 10 Apr 2013, Srinivas Pandruvada wrote: I don't know if this should be fixed in client drivers since other drivers may have this issue. Agreed. Srinivas, how about the patch below, could you please test it in your scenario to see whether it actually fixes the issue? Thanks. diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 264f550..65879b9 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1293,7 +1293,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i if (hdrv hdrv-raw_event hid_match_report(hid, report)) { ret = hdrv-raw_event(hid, report, data, size); - if (ret != 0) { + if (ret 0) { ret = ret 0 ? ret : 0; goto unlock; } -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hid: driver for PS2/3 Buzz controllers
On Mon, 27 May 2013, Colin Leitner wrote: This patch adds support for PS2/3 Buzz controllers into hid-sony Thanks, now applied. As a followup, I think I'll merge hid-ps3remote into hid-sony as well. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: add support for Huion 580 tablet
On Tue, 28 May 2013, Martin Rusko wrote: Signed-off-by: Martin Rusko martin.ru...@gmail.com --- drivers/hid/Kconfig |6 ++ drivers/hid/Makefile|1 + drivers/hid/hid-core.c |1 + drivers/hid/hid-huion.c | 177 +++ drivers/hid/hid-ids.h |3 + 5 files changed, 188 insertions(+), 0 deletions(-) create mode 100644 drivers/hid/hid-huion.c Martin, could you please provide changelog for this patch as well? What's mostly interesting is: - a brief description of the device - a brief description of the changes necessary to make it work (the deviations from the HID standard) Thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Bluetooth: hidp: register HID devices async
On Thu, 23 May 2013, David Herrmann wrote: While l2cap_user callbacks are running, the whole hci_dev is locked. Even if we would add more fine-grained locking to HCI core, it would still be called from the non-reentrant rx work-queue and thus block the event processing. However, if we want to perform synchronous I/O during HID device registration (eg., to perform device-detection), we need the HCI core to be able to dispatch incoming data. Therefore, we now move device-registration to a separate worker. The HCI core can continue running and we add devices asynchronously in another kernel thread. Device removal is synchronized and waits for the worker to exit before calling the usual device removal functions. If l2cap_user-remove is called before the thread registered the devices, we set terminate to true and the thread will skip it. If l2cap_user-remove is called after it, we notice this as the device is no longer in HIDP_SESSION_PREPARING state and simply unregister the device as we did before. There is no new deadlock as we now call hidp_session_add_dev() with one lock less held (the HCI lock) and it cannot itself call back into HCI as it was called with the HCI-lock held before. One might wonder whether this can block during device unregistration. But we set terminate to true and wake the HIDP thread up _before_ unregistering the HID/input devices. Therefore, all pending HID I/O operations are canceled. All further I/O attempts will fail with ENODEV or EIO. So all latency we can get are few context-switches, but no timeouts or blocking I/O waits! This change also prepares for a long standing HID bug. All HID devices that register power_supply devices need to be able to handle callbacks during registration (a power_supply oddity that cannot easily be fixed). So with this patch available, we can allow HID I/O during registration by calling the recently introduced hid_device_io_start/stop helpers, which currently are a no-op for bluetooth due to this locking. Note that we cannot do the same for input devices. input-core doesn't allow us to call input_event() asynchronously to input_register_device(), which HID-core kindly allows (for good reasons). Fixing input-core to allow this isn't as easy as it sounds and is, beside simplifying HIDP, not really an improvement. Hence, we still register input devices synchronously as we did before. Only HID devices are registered asynchronously. Signed-off-by: David Herrmann dh.herrm...@gmail.com Acked-by: Jiri Kosina jkos...@suse.cz Gustavo, I think I'd like to take this patch together with the ENODATA change for hid-input, as they, in some sense, stick together. If you are OK with that, could you please provide me with your Acked-by, and I'll take it through my tree? Thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Holtek gaming mouse driver, and the necessity for it instead of increasing HID_MAX_USAGES
On Tue, 21 May 2013, Christian Ohm wrote: Is there any reason why HID_MAX_USAGES shouldn't be more than 12288? Well, the reasoning is a mixture of current implementation, and reasonability. - we currently have statically allocated arrays on a per-parser basis, for parsing usages and collection indices. If the number of max usages is going to grow in an uncontrolled manner, we'll have to change the way our parser works (which is not impossible, of course). - most of the ocurences of huge max usages being presented by the devices have actually turned out to be bogus and could have been fixed by patching the report descriptor in order to reflect the real behavior of the device Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] HID: Add driver for Holtek gaming mouse 04d9:a067
On Tue, 21 May 2013, Christian Ohm wrote: This mouse is sold as Sharkoon Drakonia and Perixx MX-2000 and reports a too high usage maximum and logical maximum. This driver fixes the report descriptor so those values don't exceed HID_MAX_USAGES. Signed-off-by: Christian Ohm chr@gmx.net --- drivers/hid/Kconfig|1 + drivers/hid/Makefile |1 + drivers/hid/hid-holtek-mouse.c | 72 drivers/hid/hid-ids.h |1 + 4 files changed, 75 insertions(+) create mode 100644 drivers/hid/hid-holtek-mouse.c You also need an entry in hid_have_special_driver[] for the devices in order for the device - driver binding work properly. I have applied your patches and fixed that omission. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: fold ps3remote driver into generic Sony driver
On Tue, 28 May 2013, David Dillow wrote: Let's follow the structure we are trying to keep for most of the specific HID drivers, and let the separation follow the producing vendor. Merge functionality provided by ps3remote driver into hid-sony. Signed-off-by: Jiri Kosina jkos...@suse.cz --- I've not had a chance to test yet, but I noticed one issue: @@ -548,6 +722,11 @@ static const struct hid_device_id sony_devices[] = { .driver_data = BUZZ_CONTROLLER }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER), .driver_data = BUZZ_CONTROLLER }, + /* PS3 BD Remote Control */ + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE), + .driver_data = PS3REMOTE }, + /* Logitech Harmony Adapter for PS3 */ + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) }, I think you need .driver_data = PS3REMOTE for the Harmony as well; it uses the same protocol. Right you are, thanks for spotting this. Updated patch below. Also, have you given any thought to making 'make oldconfig' work? Those of us that build are own kernels are likely to have some fun tracking down why our remotes don't work any more... This is not easy ... drivers get renamed time to time, and I don't think there is a good transition mechanism for oldconfig. We can, for compatibility reasons, keep the old Kconfig in there, and have it just alias to hid-sony. I am not sure whether it's worth the hassle and introduced Kconfig mess though. I'd be glad if you could provide your Tested-by: for the patch below. Thanks a lot. From: Jiri Kosina jkos...@suse.cz Subject: [PATCH] HID: fold ps3remote driver into generic Sony driver Let's follow the structure we are trying to keep for most of the specific HID drivers, and let the separation follow the producing vendor. Merge functionality provided by ps3remote driver into hid-sony. Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/hid/Kconfig | 15 +--- drivers/hid/Makefile|1 - drivers/hid/hid-ps3remote.c | 204 --- drivers/hid/hid-sony.c | 184 ++- 4 files changed, 185 insertions(+), 219 deletions(-) delete mode 100644 drivers/hid/hid-ps3remote.c diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 5bad26e..8280eba 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -561,15 +561,6 @@ config HID_PRIMAX Support for Primax devices that are not fully compliant with the HID standard. -config HID_PS3REMOTE - tristate Sony PS3 BD Remote Control - depends on HID - ---help--- - Support for the Sony PS3 Blue-ray Disk Remote Control and Logitech - Harmony Adapter for PS3, which connect over Bluetooth. - - Support for the 6-axis controllers is provided by HID_SONY. - config HID_ROCCAT tristate Roccat device support depends on USB_HID @@ -600,11 +591,11 @@ config HID_SONY depends on LEDS_CLASS ---help--- Support for - + * Sony PS3 6-axis controllers * Buzz controllers - - Support for the Sony PS3 BD Remote is provided by HID_PS3REMOTE. + * Sony PS3 Blue-ray Disk Remote Control (Bluetooth) + * Logitech Harmony adapter for Sony Playstation 3 (Bluetooth) config HID_SPEEDLINK tristate Speedlink VAD Cezanne mouse support diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 2065694..833abcf 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -92,7 +92,6 @@ hid-picolcd-y += hid-picolcd_debugfs.o endif obj-$(CONFIG_HID_PRIMAX) += hid-primax.o -obj-$(CONFIG_HID_PS3REMOTE)+= hid-ps3remote.o obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \ hid-roccat-koneplus.o hid-roccat-konepure.o hid-roccat-kovaplus.o \ diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c deleted file mode 100644 index f1239d3..000 --- a/drivers/hid/hid-ps3remote.c +++ /dev/null @@ -1,204 +0,0 @@ -/* - * HID driver for Sony PS3 BD Remote Control - * - * Copyright (c) 2012 David Dillow d...@thedillows.org - * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann - * and other kernel HID drivers. - */ - -/* - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the Free - * Software Foundation; either version 2 of the License, or (at your option) - * any later version. - */ - -/* NOTE: in order for the Sony PS3 BD Remote Control to be found by - * a Bluetooth host, the key combination Start+Enter has to be kept pressed - * for about 7 seconds with the Bluetooth Host Controller in discovering mode
Re: [PATCH] hid: driver for PS2/3 Buzz controllers
On Sun, 26 May 2013, Colin Leitner wrote: Added a driver for PlayStation 2/3 Buzz controllers, which exposes the LEDs and maps all buttons to BTN_TRIGGER_HAPPY1 to 20. Applies to kernel version 3.10.0-rc2. Tested with Debian 7 and with a minor change on kernel 3.8.5 on Fedora 18. Couldn't test the wireless version, but what can be gathered on information from the net, both should be identical in their report structure. Signed-off-by: Colin Leitner colin.leit...@gmail.com Cc: Jiri Kosina jkos...@suse.cz Cc: linux-input@vger.kernel.org Cc: linux-ker...@vger.kernel.org --- drivers/hid/Kconfig| 10 ++ drivers/hid/Makefile | 1 + drivers/hid/hid-buzz.c | 309 + drivers/hid/hid-core.c | 4 + drivers/hid/hid-ids.h | 2 + 5 files changed, 326 insertions(+) Hi Colin, thanks a lot for the support. I this it'd actually make more sense to add this support to one of the two existing drivers (*) we already have for Sony remote controllers -- either hid-sony or hid-ps3remote both seem appropriate. (*) I think we should even merge hid-ps3remote into hid-sony. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html