Bug 39882

Summary: Kernel oops when turning bluetooth mouse on
Product: Drivers Reporter: Lamarque V. Souza (lamarque)
Component: BluetoothAssignee: Jiri Kosina (jikos)
Status: RESOLVED PATCH_ALREADY_AVAILABLE    
Severity: high CC: alan, florian, jikos, maciej.rutecki, rjw
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 3.0.0 Subsystem:
Regression: Yes Bisected commit-id:
Bug Depends on:    
Bug Blocks: 36912    
Attachments: dmesg output showing the oops
New dmesg log
Checks blacklist before allocating hid device
Checks blacklist before creating hid session
Checks blacklist before creating hid session (v2)
Checks blacklist before creating hid session (kernel 3.3)
Checks blacklist before creating hid session (kernel 3.7.0-rc8)

Description Lamarque V. Souza 2011-07-24 02:09:33 UTC
Created attachment 66422 [details]
dmesg output showing the oops

Hi, I have this bluetooth mouse (Acer Bluetooth Optical Rechargeable Mouse) which starts to trigger a kernel oops with kernel 3.0.0. I used to use 2.6.38.8 and everything works with it. Today I have finally found the commit that causes the problem: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=0f69dca20f77dc374b67e17e10b30cec37e778c4 (HID: bt: Move hid_add_device() call to after hidp_session() has started). After reverting that commit my mouse works and there is no oops anymore.

Can you revert that commit or at help me found a way to make it work with my mouse? I have attached the dmesg output of the oops.
Comment 1 Alan Ott 2011-07-25 18:22:32 UTC
Your dmesg trace makes it look like the problem is when your mouse disconnects since it's after the main loop in hidp_session(). Is this true? Your comments above make it sound otherwise.

Also, is the call to down_write() where the problem happens (ie: are there other printk() messages after the call to down_write()?)?
Comment 2 Lamarque V. Souza 2011-07-25 18:48:40 UTC
Yes, it happens outside the main loop in hidp_session() but I had just turned the mouse on. For some reason the connection did not work, the session ended and the kthread oopsed.

The down_write printk is located after the down_write call ("depois" means "after" in Brazilian Portuguese). Probably the problem is in the next function call (hidp_del_timer(session)). I think printk is buffered, right? If so then it is not a reliable indication of where the problem really happens.
Comment 3 Alan Ott 2011-07-25 19:01:23 UTC
Interesting. So it sounds like the main loop in hidp_session() never gets going and thus exits before the device actually gets added to the system (by calling hid_add_device()).

The real problem here that the main loop exits on the first pass. See if you can add some logging to see which place session->terminate is getting set.
Comment 4 Lamarque V. Souza 2011-07-25 19:51:34 UTC
Created attachment 66602 [details]
New dmesg log

Here is the new dmesg. I forgot to add a '\n' at the end of the printk (I am used to use kDebug() in KDE which automatically adds the '\n'), but it is easy to see the terminate is set in err_add_device label, which is trigged because hid_add_device has failed.

Now I must say that this mouse is blacklisted for hid session in drivers/hid/hid-core.c, see the line with:

{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_KYE, 0x0058) },

The mouse works with hid but with laggy. Using input session it works much better, that is why I asked to blacklist it two years ago.
Comment 5 Lamarque V. Souza 2011-07-25 21:22:41 UTC
I think I know what is happening:

With your patch applied:

1. hidp_add_connection calls hidp_setup_hid, which returns 0 and set ups a hid session. This is wrong, the mouse is blacklisted although it works with hid.
2. after session->waiting_for_startup hidp_add_connection calls hid_add_device, which fails probably because of the blacklist I mentioned before;
3. because of the failed above hidp_add_connection reaches err_add_device.

Without your patch applied:

1. hidp_add_connection calls hidp_setup_hid, which returns error because hid_add_device is inside hidp_setup_hid;
2. hidp_add_connection tries to setup a input session instead, which works and everything else works.

The problem is that with your patch hidp_setup_hid does not take the blacklist into account anymore.
Comment 6 Lamarque V. Souza 2011-07-26 00:53:19 UTC
Created attachment 66632 [details]
Checks blacklist before allocating hid device

The attached patch fixes the problem for me. I am just not sure if that is the right place to check the blacklist since hid_add_device already checks for it.
Comment 7 Alan Ott 2011-07-29 06:10:23 UTC
Added Jiri Kosina (HID Maintainer) to CC.

Jiri, is there any problem moving the quirk blacklist checking to hid_allocate_device() instead of hid_add_device() like Lamarque suggests in his patch?

Lamarque, the checking for blacklist should probably only be done in one place, either hid_allocate_device() or hid_add_device(). I'd suggest removing it from hid_add_device() if you are going to move it to hid_allocate_device().

I'm still not 100% sure that the ordering is correct in this case still. I'll look at it again in the morning.
Comment 8 Florian Mickler 2011-08-09 08:11:53 UTC
Patch: https://bugzilla.kernel.org/attachment.cgi?id=66632

Is this patch gonna be merged?
Comment 9 Lamarque V. Souza 2011-08-18 15:10:14 UTC
The patch http://bugzilla.kernel.org/attachment.cgi?id=66632 does not work because the hdev struct does not contain the vendor and the product ids when hid_allocate_device is called. Checking the blacklist there does not work because of that.

This bug is still an open issue, the only solution I have found so far is reverting 0f69dca20f77dc374b67e17e10b30cec37e778c4. Another solution is exporting the hid_ignore function to other kernel modules and use it in net/bluetooth/hidp/core.c:hidp_setup_hid(). What do you think?
Comment 10 Florian Mickler 2011-08-19 08:46:04 UTC
Ignore-Patch: https://bugzilla.kernel.org/attachment.cgi?id=66632
Comment 11 Lamarque V. Souza 2011-08-26 00:25:24 UTC
Created attachment 70332 [details]
Checks blacklist before creating hid session

The attached patch exports function hid_ignore and adds a check in hidp_setup_hid, which will return -ENODEV if the device is blacklisted. It fixes the problem for me. Is the patch ok to submit to the kernel?
Comment 12 Lamarque V. Souza 2011-08-26 01:03:25 UTC
Created attachment 70342 [details]
Checks blacklist before creating hid session (v2)

Small change in the patch.
Comment 13 Rafael J. Wysocki 2011-08-28 19:31:13 UTC
Patch : https://bugzilla.kernel.org/attachment.cgi?id=70342
Comment 14 Lamarque V. Souza 2012-04-15 00:08:22 UTC
Created attachment 72921 [details]
Checks blacklist before creating hid session (kernel 3.3)

Kernel 3.3 introduced one change that broke the last patch I sent. This one works.
Comment 15 Jiri Kosina 2012-12-03 12:46:02 UTC
Is the issue still present in current vanilla?

If so, could you please refresh the patch against the current vanilla, add proper changelog and signoff line, so that I could review and apply it? Thanks.
Comment 16 Lamarque V. Souza 2012-12-03 13:23:08 UTC
By current vanilla you mean 3.7-rc7, right? If so I will update the patch until tomorrow.

I guess this problem is still reproducible. The problem only happens with bluetooth mice that support both input and hid that are blacklisted in drivers/hid/hid-core.c. That is a rare combination, so most people is not affected by this bug. I myself blacklisted my mouse in drivers/hid/hid-core.c because its hid implementation is buggy (either it causes too much laggy, like in Windows, or the mouse pointer is always stuck at the top left corner).
Comment 17 Lamarque V. Souza 2012-12-05 04:59:03 UTC
Created attachment 88491 [details]
Checks blacklist before creating hid session (kernel 3.7.0-rc8)

Patch updated to apply to 3.7-rc8, which by the way still contains this bug.
Comment 18 Jiri Kosina 2012-12-06 13:58:00 UTC
Thanks for the refresh.

What is still missing is changelog, explaining what is the bug it fixes and how it fixes it, and Signed-off-by line, as documented in Documentation/SubmittingPatches

Ideally, send the patch with all these to me (jkosina@suse.cz) and cc linux-input@ and LKML.

Without this, I can't really review/apply it.

Thanks!
Comment 19 Florian Mickler 2012-12-22 09:23:27 UTC
A patch referencing this bug report has been merged in Linux v3.8-rc1:

commit 4529eefad087f97b33c0f31984d924b1f15d7bae
Author: Lamarque V. Souza <lamarque@gmail.com>
Date:   Thu Dec 6 12:39:55 2012 -0200

    HID: hidp: fallback to input session properly if hid is blacklisted