[1/1] media: dvb-usb/af9015, fix disconnection crashes

  When both remote controller and receiver intfs are handled by
af9015, .probe do nothing for remote intf, but when .disconnect
is called for both of them it touches intfdata every time. For
remote it crashes obviously (as intfdata are unset).

Altough there is test against data being NULL, it is not enough.
It is because someone before us does not set intf drvdata to
NULL. (In this case the hid layer.) But we cannot rely on intf
being NULL anyway.

Fix that by checking bInterfaceNumber in af9015_usb_device_exit
and do actually nothing if it is not 0.

The crash in question:
BUG: unable to handle kernel paging request at 00000000000700c5
IP: [<ffffffffa005f4f9>] dvb_usb_device_exit+0x39/0x60 [dvb_usb]
PGD 192344067 PUD 1921ba067 PMD 0
Oops: 0000 [#1] SMP
last sysfs file: /sys/devices/virtual/net/tun0/statistics/collisions
CPU 1
Pid: 10515, comm: rmmod Not tainted 2.6.33-rc4-mm1_64 #930 To be filled by O.E.M./To Be Filled By O.E.M.
RIP: 0010:[<ffffffffa005f4f9>]  [<ffffffffa005f4f9>] dvb_usb_device_exit+0x39/0x60 [dvb_usb]
RSP: 0018:ffff88018066bd48  EFLAGS: 00010206
RAX: 00000000000700c5 RBX: ffffffffa0061c8a RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8801cab599e0
RBP: ffff88018066bd58 R08: 0000000000000001 R09: 000000000046dd74
R10: 0000000000000005 R11: 0000000000000064 R12: ffff8801caa14090
R13: ffff8801ca886360 R14: ffffffffa00a8668 R15: 0000000000000000
FS:  00007fe2ec1566f0(0000) GS:ffff880028280000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00000000000700c5 CR3: 00000001b126e000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process rmmod (pid: 10515, threadinfo ffff88018066a000, task ffff88019f1eb880)
Stack:
 ffff8801cab599b0 ffff8801cab599b0 ffff88018066bd78 ffffffffa00a604c
<0> ffff8801ca886360 ffff8801cab599e0 ffff88018066bdc8 ffffffff812fd844
<0> ffffffffa00a8668 ffffffffa00a8600 ffffffffa00a8668 ffff8801cab599e0
Call Trace:
 [<ffffffffa00a604c>] af9015_usb_device_exit+0x3c/0x60 [dvb_usb_af9015]
 [<ffffffff812fd844>] usb_unbind_interface+0x124/0x170
...
Code: e8 8d 4b 22 e1 31 f6 49 89 c4 48 89 df 48 c7 c3 8a 1c 06 a0 e8 a9 4b 22 e1 4d 85 e4 74 18 49 8b 84 24 c0 0c 00 00 48 85 c0 74 0b <48> 8b 18 4c 89 e7 e8 ec fe ff ff 48 89 de 48 c7 c7 40 14 06 a0
RIP  [<ffffffffa005f4f9>] dvb_usb_device_exit+0x39/0x60 [dvb_usb]
 RSP <ffff88018066bd48>
CR2: 00000000000700c5
---[ end trace f25ee66d2135f162 ]---

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/dvb/dvb-usb/af9015.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
  

On 01/20/2010 07:19 PM, Jiri Slaby wrote:
> When both remote controller and receiver intfs are handled by
> af9015, .probe do nothing for remote intf, but when .disconnect
> is called for both of them it touches intfdata every time. For
> remote it crashes obviously (as intfdata are unset).
>
> Altough there is test against data being NULL, it is not enough.
> It is because someone before us does not set intf drvdata to
> NULL. (In this case the hid layer.) But we cannot rely on intf
> being NULL anyway.
>
> Fix that by checking bInterfaceNumber in af9015_usb_device_exit
> and do actually nothing if it is not 0.

I was a little bit surprised when saw this error, why it haven't 
detected earlier. When I initially added interface check for .probe it 
was surely needed, it was creating two instances without that check in 
that time. When I now test this patch with debugs enabled I don't see 
.probe and .disconnect be called for this HID interface (interface 1) at 
all and thus checks not needed. I have Fedora Kernel 2.6.31.12 running 
with latest v4l-dvb. Is there now some kind of check added recently 
which blocks .probe and disconnect from HID interface?

regards
Antti
  

On 01/25/2010 12:44 AM, Antti Palosaari wrote:
> When I now test this patch with debugs enabled I don't see
> .probe and .disconnect be called for this HID interface (interface 1) at
> all and thus checks not needed.

What happens if you disable the HID layer? Or at least if you add an
ignore quirk for the device in usbhid?

I forbid usbhid to attach to the device, as the remote kills X with HID
driver. With dvb-usb-remote it works just fine (with remote=2 for af9015
or the 4 patches I've sent).
  

On 01/25/2010 11:12 AM, Jiri Slaby wrote:
> On 01/25/2010 12:44 AM, Antti Palosaari wrote:
>> When I now test this patch with debugs enabled I don't see
>> .probe and .disconnect be called for this HID interface (interface 1) at
>> all and thus checks not needed.
>
> What happens if you disable the HID layer? Or at least if you add an
> ignore quirk for the device in usbhid?

Looks like Fedora doesn't have usbhid compiled as module. I looked 
hid-quirks.c file and there was only one af9015 device blacklisted 
15a4:9016. I have 15a4:9015, 15a4:9016 and 13d3:3237 devices and no 
difference.

How can I disable HID layer?

> I forbid usbhid to attach to the device, as the remote kills X with HID
> driver. With dvb-usb-remote it works just fine (with remote=2 for af9015
> or the 4 patches I've sent).

In my understanding the cause of the remote problem is chipset bug which 
sets USB2.0 polling interval to 4096ms. Therefore HID remote does not 
work at all or starts repeating. It is possible to implement remote as 
polling from the driver which works very well. But HID problem still 
remains. I have some hacks in my mind to test to kill HID. One is to 
configure HID wrongly to see if it stops outputting characters. Other 
way is try to read remote codes directly from the chip memory.

But all in all, your patch does not break anything, it is safe to add. 
It could be still nice to know if there is better alternatives. And 
there is surely few other devices having HID remote - are those also 
affected.

regards
Antti
  

On Mon, 25 Jan 2010, Antti Palosaari wrote:

> > What happens if you disable the HID layer? Or at least if you add an
> > ignore quirk for the device in usbhid?
> 
> Looks like Fedora doesn't have usbhid compiled as module. I looked
> hid-quirks.c file and there was only one af9015 device blacklisted 15a4:9016.
> I have 15a4:9015, 15a4:9016 and 13d3:3237 devices and no difference.
> 
> How can I disable HID layer?

In case usbhid is compiled in, you should still be able to force the 
ignore quirk by passing

	usbhid.quirks=0x15a4:0x9015:0x04

to kernel boot commandline.

> > I forbid usbhid to attach to the device, as the remote kills X with HID
> > driver. With dvb-usb-remote it works just fine (with remote=2 for af9015
> > or the 4 patches I've sent).
> 
> In my understanding the cause of the remote problem is chipset bug which sets
> USB2.0 polling interval to 4096ms. Therefore HID remote does not work at all
> or starts repeating. It is possible to implement remote as polling from the
> driver which works very well. But HID problem still remains. I have some hacks
> in my mind to test to kill HID. One is to configure HID wrongly to see if it
> stops outputting characters. Other way is try to read remote codes directly
> from the chip memory.

Yes, Pekka Sarnila has added this workaround to the HID driver, as the 
device is apparently broken.

I want to better understand why others are not hitting this with the 
DVB remote driver before removing the quirk from HID code completely.

> But all in all, your patch does not break anything, it is safe to add. It
> could be still nice to know if there is better alternatives. And there is
> surely few other devices having HID remote - are those also affected.