Bug 80091
Summary: | NULL pointer dereference in hid_rmi | ||
---|---|---|---|
Product: | Drivers | Reporter: | Dmitry (nrndda) |
Component: | Input Devices | Assignee: | drivers_input-devices |
Status: | RESOLVED CODE_FIX | ||
Severity: | normal | CC: | aduggan, andrey_utkin |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 3.16.0-rc4 | Subsystem: | |
Regression: | No | Bisected commit-id: | |
Attachments: |
dmesg
.config printk()s for Bug 80091 - NULL pointer dereference in hid_rmi dmesg after modprobing hid-rmi with printks Enabled kernel debug options and cmdline. null pinter dereference fix dmesg output after patch lsusb -vvvv from ubuntu syslog from ubuntu current dmesg with working touchpad (until pressing keys) and don't working keyboard this patch works patch to only bind to mouse interfaces |
Description
Dmitry
2014-07-12 11:08:01 UTC
Created attachment 142761 [details]
dmesg
For me it looks close to bugs 56251 and 77341. Created attachment 142771 [details]
.config
Created attachment 142841 [details] printk()s for Bug 80091 - NULL pointer dereference in hid_rmi Still there? Could you please try this? I suppose either hdev->report_enum[HID_INPUT_REPORT].report_id_hash or hdev->report_enum[HID_OUTPUT_REPORT].report_id_hash is null. Created attachment 142851 [details]
dmesg after modprobing hid-rmi with printks
This is everything what I got. Maybe I haven't turned something on?
Created attachment 142861 [details]
Enabled kernel debug options and cmdline.
Well, i think i have added wrong checks, but now we definitely know that the problem is the line data->input_report_size = (hdev->report_enum[HID_INPUT_REPORT] .report_id_hash[RMI_ATTN_REPORT_ID]->size >> 3) + 1 /* report id */; And the NULL expression is likely hdev->report_enum[HID_INPUT_REPORT].report_id_hash[RMI_ATTN_REPORT_ID]. Looking at drivers/hid/hid-magicmouse.c, i see that we should do something like report = hid_register_report(hdev, HID_INPUT_REPORT, RMI_ATTN_REPORT_ID); report->size = XXX; before we use hdev->report_enum[HID_INPUT_REPORT].report_id_hash[RMI_ATTN_REPORT_ID]. I have no idea what should be this "size", but you can try to set to some "reasonably high" number. Another way should be to avoid usage of those expressions in the calculation of allocation sizes. But i have no idea how to calculate them otherwise. OK, I tried with patch below and it fixed null pointer dereference. But module doesn't work. Created attachment 143091 [details]
null pinter dereference fix
Created attachment 143101 [details]
dmesg output after patch
There are some mistakes in printks but values are right. So, it isn't enough for make module work. As i said, try to set report->size after hid_register_report(). Also i _guess_ setting hdev->report_enum[...].report_id_hash[...] with result of hid_register_report() is unnecessary. I have mailed to authors of the driver, also you should mail to linux-usb maillist. This don't help no matter what size I choose: Jul 15 02:15:07 venue11pro kernel: hid-rmi 0003:06CB:2819.0004: unable to set rmi mode to 1 (-32) Jul 15 02:15:07 venue11pro kernel: hid-rmi 0003:06CB:2819.0004: failed to set rmi mode Jul 15 02:15:12 venue11pro kernel: hid-rmi 0003:06CB:2819.0005: failed to write hid report (-110) Jul 15 02:15:12 venue11pro kernel: hid-rmi 0003:06CB:2819.0005: rmi_set_page: set page failed: -110. Jul 15 02:15:12 venue11pro kernel: hid-rmi 0003:06CB:2819.0005: failed to set page select to 0. Tried with (64>>3)+1=9 and (128>>3)+1=17 bits. There does seem to be something wrong here. This touchpad isn't actually supported by hid-rmi. This touchpad complies with the Microsoft Precision Touchpad spec which reports data a little differently, but it is supported by the hid-multitouch driver. The hid-core driver should recognize this when it parses the HID descriptor and bind the hid-multitouch driver to it. This makes me think that hid-core failed to properly parse the HID descriptors. Incidentally, because hid-core isn't recognizing the touchpad correctly hid-rmi is getting loaded by default. Even though hid-rmi doesn't support this type of touchpad it shouldn't be failing like this. There should still be an attention report ID so setting the input report size shouldn't result in a null pointer derefernce. I guess I will have to add some additional error checking here. Also the error codes reported by rmi_set_mode (-32) and rmi_write_report (-110) may be coming from the USB layer so there maybe something wrong with how the device is communicating. Did you try docking and undocking a few times? Did the touchpad work with previous versions of the kernel? Did the touchpad work under Windows? Just to rule out that the hardware isn't completely broken on this unit. I will see if I can find a system and try to reproduce this issue. This dock contains not only touchpad but also a keyboard. Exactly the latter don't work. I've tested with ubuntu with kernel 3.13 and keyboard works. That is xinput from 3.16: ⎡ Virtual core pointer id=2 [master pointer (3)] ⎜ ↳ Virtual core XTEST pointer id=4 [slave pointer (2)] ⎜ ↳ Synaptics T Pad V 01.30 UNKNOWN id=7 [slave pointer (2)] ⎜ ↳ SYNA7500:00 06CB:3AF0 Pen id=8 [slave pointer (2)] ⎜ ↳ SYNA7500:00 06CB:3AF0 id=9 [slave pointer (2)] ⎣ Virtual core keyboard id=3 [master keyboard (2)] ↳ Virtual core XTEST keyboard id=5 [slave keyboard (3)] ↳ Video Bus id=6 [slave keyboard (3)] ↳ gpio-keys id=10 [slave keyboard (3)] ↳ gpio-keys id=11 [slave keyboard (3)] ↳ Dell WMI hotkeys id=12 [slave keyboard (3)] And from ubuntu: ⎡ Virtual core pointer id=2 [master pointer (3)] ⎜ ↳ Virtual core XTEST pointer id=4 [slave pointer (2)] ⎜ ↳ SYNA7500:00 06CB:3AF0 Pen id=10 [slave pointer (2)] ⎜ ↳ SYNA7500:00 06CB:3AF0 id=11 [slave pointer (2)] ⎜ ↳ Synaptics T Pad V 01.30 id=7 [slave pointer (2)] ⎣ Virtual core keyboard id=3 [master keyboard (2)] ↳ Virtual core XTEST keyboard id=5 [slave keyboard (3)] ↳ Video Bus id=6 [slave keyboard (3)] ↳ Synaptics T Pad V 01.30 id=8 [slave keyboard (3)] ↳ Synaptics T Pad V 01.30 id=9 [slave keyboard (3)] ↳ Dell WMI hotkeys id=12 [slave keyboard (3)] In both outputs touchpad does work. hid-rmi blocked in 3.16 and in 3.13 kernel it desn't exist (not yet backported). Suddenly I found that pressing any key on keyboard disables touchpad completely. In UEFI both keyboard and touchpad work. Created attachment 143121 [details]
lsusb -vvvv from ubuntu
Created attachment 143131 [details]
syslog from ubuntu
Created attachment 143141 [details]
current dmesg with working touchpad (until pressing keys) and don't working keyboard
Ok, looks like this is a composite USB device with multiple interfaces for the HID keyboard and the HID touchpad. The strange thing is that they are using our Vendor ID to identify this composite device. Because the USB device has the Synaptics Vendor ID hid-core is trying to bind hid-rmi to the keyboard and whatever the third interface is. But, since those devices don't have the correct report ids hid-rmi ends up dereferencing a null pointer. I can add checks for null pointers, but the main issue is that hid-rmi is incorrectly being bound to a device it should not be. These devices should probably be using hid-generic instead. It looks like we will have to reexamining the following lines since we can no longer solely rely on our Vendor ID distinguishing our devices. hid-core.c:785 if ((hid->vendor == USB_VENDOR_ID_SYNAPTICS) && (hid->group == HID_GROUP_GENERIC)) /* hid-rmi should take care of them, not hid-generic */ hid->group = HID_GROUP_RMI; I'm not sure if typing on the keyboard resulting the in the touchpad being disabled is related or not though. Created attachment 143201 [details]
this patch works
Yep, you were right! This patch works for me. Both device at same time. Of course it's a hack, but now it's clear where the problem is. Thanks for help!
Created attachment 143301 [details]
patch to only bind to mouse interfaces
I came up with a patch should be more generic. If you get a chance can you revert your patch and try it to make sure it works. If so I will submit it upstream and see what they think of it. I wasn't able to get my hands on hardware after all.
I confirm, this patch works too. Of course after reverting my old patch. Thanks for testing! |