2010-02-08 17:01:54

by Bruno Prémont

[permalink] [raw]
Subject: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

2.6.33-rc7 (don't know if any previous version resumes properly)
crashes during resume from S2Ram when my USB keyboard is connected but
resumes properly (viafb corruption put apart) when the USB keyboard is
not connected.

Keyboard detection:
[ 3.070054] usb 2-2: new full speed USB device using uhci_hcd and address 2
[ 3.220179] kbd_mode used greatest stack depth: 2228 bytes left
[ 3.276403] usb 2-2: New USB device found, idVendor=058f, idProduct=9462
[ 3.276514] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[ 3.276619] usb 2-2: Product: Multimedia USB Keyboard
[ 3.276711] usb 2-2: Manufacturer: Multimedia USB Keyboard
[ 3.278056] loadkeys used greatest stack depth: 1904 bytes left
[ 3.278791] init-early.sh used greatest stack depth: 1700 bytes left
[ 3.282561] hub 2-2:1.0: USB hub found
[ 3.286387] hub 2-2:1.0: 3 ports detected
[ 3.571454] usb 2-2.1: new full speed USB device using uhci_hcd and address 3
[ 3.761474] usb 2-2.1: New USB device found, idVendor=058f, idProduct=9462
[ 3.761584] usb 2-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 3.761719] usb 2-2.1: Product: Multimedia USB Keyboard
[ 3.762518] usb 2-2.1: Manufacturer: Multimedia USB Keyboard
[ 3.762612] usb 2-2.1: SerialNumber: Multimedia USB Keyboard
[ 3.789221] input: Multimedia USB Keyboard Multimedia USB Keyboard as /devices/pci0000:00/0000:00:10.0/usb2/2
-2/2-2.1/2-2.1:1.0/input/input4
[ 3.789585] generic-usb 0003:058F:9462.0001: input: USB HID v1.10 Keyboard [Multimedia USB Keyboard Multimedi
a USB Keyboard] on usb-0000:00:10.0-2.1/input0
[ 3.818001] generic-usb: probe of 0003:058F:9462.0002 failed with error -22

Resume crash:
[ 48.130061] usb 2-2: reset full speed USB device using uhci_hcd and address 2
[ 48.591086] usb 2-2.1: reset full speed USB device using uhci_hcd and address 3
[ 48.741106] BUG: unable to handle kernel NULL pointer dereference at 00000020
[ 48.741558] IP: [<c11eabb0>] dev_get_drvdata+0x10/0x20
[ 48.741902] *pde = 00000000
[ 48.742083] Oops: 0000 [#1]
[ 48.742269] last sysfs file: /sys/power/state
[ 48.742528] Modules linked in: e_powersaver via_cputemp snd_hda_codec_via snd_hda_intel snd_hda_codec snd_pcm snd_timer via_agp snd soundcore viafb snd_page_alloc i2c_algo_bit cfbcopyarea cfbimgblt cfbfillrect sg agpgart
[ 48.743939]
[ 48.744040] Pid: 1512, comm: bash Not tainted 2.6.33-rc7-venus #1 CX700+W697HG/CX700+W697HG
[ 48.744528] EIP: 0060:[<c11eabb0>] EFLAGS: 00010202 CPU: 0
[ 48.744859] EIP is at dev_get_drvdata+0x10/0x20
[ 48.745121] EAX: 0000001c EBX: f6814000 ECX: 00000000 EDX: 00000000
[ 48.745493] ESI: f68bd064 EDI: f6814000 EBP: f6539dbc ESP: f6539dbc
[ 48.745861] DS: 0068 ES: 007b FS: 0000 GS: 0033 SS: 0068
[ 48.746178] Process bash (pid: 1512, ti=f6539000 task=f73686e0 task.ti=f6539000)
[ 48.746595] Stack:
[ 48.746716] f6539dcc c12823fe f68bd064 f682fe00 f6539e00 c1282558 00000021 00000000
[ 48.747391] <0> 00000001 00000000 00000000 00001388 f68bd000 f692a000 f682fe00 00000001
[ 48.747975] <0> f682fe00 f6539e0c c12825a8 c149d690 f6539e34 c122e65a c1498654 f68bd000
[ 48.751052] Call Trace:
[ 48.751052] [<c12823fe>] ? usbhid_restart_queues+0x3e/0x100
[ 48.751052] [<c1282558>] ? hid_post_reset+0x98/0xc0
[ 48.751052] [<c12825a8>] ? hid_reset_resume+0x28/0x30
[ 48.751052] [<c122e65a>] ? usb_resume_interface+0x9a/0x190
[ 48.751052] [<c123641f>] ? generic_resume+0xf/0x30
[ 48.751052] [<c122eb0f>] ? usb_resume_both+0x8f/0x130
[ 48.751052] [<c122f7fb>] ? usb_external_resume_device+0x2b/0x70
[ 48.751052] [<c122f85d>] ? usb_resume+0x1d/0x30
[ 48.751052] [<c12240ed>] ? usb_dev_resume+0xd/0x10
[ 48.751052] [<c11ee1b4>] ? pm_op+0x94/0xb0
[ 48.751052] [<c11711d0>] ? kobject_put+0x20/0x50
[ 48.751052] [<c11ee9f9>] ? dpm_resume_end+0xe9/0x330
[ 48.751052] [<c105131a>] ? resume_device_irqs+0x2a/0x60
[ 48.751052] [<c104d0ff>] ? suspend_devices_and_enter+0x7f/0x1a0
[ 48.751052] [<c132e0dd>] ? printk+0x18/0x1b
[ 48.751052] [<c104d320>] ? enter_state+0x100/0x120
[ 48.751052] [<c104cae0>] ? state_store+0x80/0xb0
[ 48.751052] [<c104ca60>] ? state_store+0x0/0xb0
[ 48.751052] [<c11710a4>] ? kobj_attr_store+0x24/0x30
[ 48.751052] [<c10c27bd>] ? sysfs_write_file+0x9d/0xf0
[ 48.751052] [<c10810cc>] ? vfs_write+0x9c/0x160
[ 48.751052] [<c10c2720>] ? sysfs_write_file+0x0/0xf0
[ 48.751052] [<c108124d>] ? sys_write+0x3d/0x70
[ 48.751052] [<c1002bd0>] ? sysenter_do_call+0x12/0x26
[ 48.751052] Code: 55 89 e5 5d 83 f8 01 19 c0 f7 d0 83 e0 f0 c3 8d b6 00 00 00 00 8d bf 00 00 00 00 55 85 c0 89 e5 75 09 31 c0 5d c3 90 8d 74 26 00 <8b> 40 04 85 c0 74 f0 8b 40 40 5d c3 8d 74 26 00 55 89 e5 83 ec
[ 48.751052] EIP: [<c11eabb0>] dev_get_drvdata+0x10/0x20 SS:ESP 0068:f6539dbc
[ 48.751052] CR2: 0000000000000020
[ 49.281981] ---[ end trace 1f0c734925556462 ]---

Could the crash be related to the generic-usb probe error?

lsusb -v for the keyboard:
Bus 002 Device 003: ID 058f:9462 Alcor Micro Corp.
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x058f Alcor Micro Corp.
idProduct 0x9462
bcdDevice 4.10
iManufacturer 1 Multimedia USB Keyboard
iProduct 2 Multimedia USB Keyboard
iSerial 3 Multimedia USB Keyboard
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 59
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 50mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 1 Keyboard
iInterface 0
HID Device Descriptor:
bLength 9
bDescriptorType 33
bcdHID 1.10
bCountryCode 0 Not supported
bNumDescriptors 1
bDescriptorType 34 Report
wDescriptorLength 65
Report Descriptors:
** UNAVAILABLE **
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0008 1x 8 bytes
bInterval 10
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 0 No Subclass
bInterfaceProtocol 0 None
iInterface 0
HID Device Descriptor:
bLength 9
bDescriptorType 33
bcdHID 1.10
bCountryCode 0 Not supported
bNumDescriptors 1
bDescriptorType 34 Report
wDescriptorLength 106
Report Descriptors:
** UNAVAILABLE **
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0004 1x 4 bytes
bInterval 255
Device Status: 0x0000
(Bus Powered)

Bus 002 Device 002: ID 058f:9462 Alcor Micro Corp.
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 9 Hub
bDeviceSubClass 0 Unused
bDeviceProtocol 0 Full speed (or root) hub
bMaxPacketSize0 8
idVendor 0x058f Alcor Micro Corp.
idProduct 0x9462
bcdDevice 1.58
iManufacturer 1 Multimedia USB Keyboard
iProduct 2 Multimedia USB Keyboard
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 25
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 50mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 9 Hub
bInterfaceSubClass 0 Unused
bInterfaceProtocol 0 Full speed (or root) hub
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0001 1x 1 bytes
bInterval 255
Hub Descriptor:
bLength 9
bDescriptorType 41
nNbrPorts 3
wHubCharacteristic 0x000d
Per-port power switching
Compound device
Per-port overcurrent protection
bPwrOn2PwrGood 22 * 2 milli seconds
bHubContrCurrent 50 milli Ampere
DeviceRemovable 0x02
PortPwrCtrlMask 0xff
Hub Port Status:
Port 1: 0000.0103 power enable connect
Port 2: 0000.0100 power
Port 3: 0000.0100 power
Device Status: 0x0000
(Bus Powered)


2010-02-08 18:07:21

by Bruno Prémont

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Mon, 08 February 2010 Bruno Prémont <[email protected]> wrote:
> 2.6.33-rc7 (don't know if any previous version resumes properly)
> crashes during resume from S2Ram when my USB keyboard is connected but
> resumes properly (viafb corruption put apart) when the USB keyboard is
> not connected.

The patch below works around the crash though the WARN_ON() in
usbhid_restart_out_queue() triggers in place.

Bruno

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index e2997a8..d2f8eef 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -196,7 +196,7 @@ static void usbhid_mark_busy(struct usbhid_device *usbhid)

static int usbhid_restart_out_queue(struct usbhid_device *usbhid)
{
- struct hid_device *hid = usb_get_intfdata(usbhid->intf);
+ struct hid_device *hid = usbhid->intf ? usb_get_intfdata(usbhid->intf) : NULL;
int kicked;

if (!hid)
@@ -214,7 +214,7 @@ static int usbhid_restart_out_queue(struct usbhid_device *usbhid)

static int usbhid_restart_ctrl_queue(struct usbhid_device *usbhid)
{
- struct hid_device *hid = usb_get_intfdata(usbhid->intf);
+ struct hid_device *hid = usbhid->intf ? usb_get_intfdata(usbhid->intf) : NULL;
int kicked;

WARN_ON(hid == NULL);

2010-02-08 18:16:07

by Alan Stern

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Mon, 8 Feb 2010, Bruno [UTF-8] Prémont wrote:

> 2.6.33-rc7 (don't know if any previous version resumes properly)
> crashes during resume from S2Ram when my USB keyboard is connected but
> resumes properly (viafb corruption put apart) when the USB keyboard is
> not connected.
>
> Keyboard detection:
> [ 3.070054] usb 2-2: new full speed USB device using uhci_hcd and address 2
> [ 3.220179] kbd_mode used greatest stack depth: 2228 bytes left
> [ 3.276403] usb 2-2: New USB device found, idVendor=058f, idProduct=9462
> [ 3.276514] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> [ 3.276619] usb 2-2: Product: Multimedia USB Keyboard
> [ 3.276711] usb 2-2: Manufacturer: Multimedia USB Keyboard
> [ 3.278056] loadkeys used greatest stack depth: 1904 bytes left
> [ 3.278791] init-early.sh used greatest stack depth: 1700 bytes left
> [ 3.282561] hub 2-2:1.0: USB hub found
> [ 3.286387] hub 2-2:1.0: 3 ports detected
> [ 3.571454] usb 2-2.1: new full speed USB device using uhci_hcd and address 3
> [ 3.761474] usb 2-2.1: New USB device found, idVendor=058f, idProduct=9462
> [ 3.761584] usb 2-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 3.761719] usb 2-2.1: Product: Multimedia USB Keyboard
> [ 3.762518] usb 2-2.1: Manufacturer: Multimedia USB Keyboard
> [ 3.762612] usb 2-2.1: SerialNumber: Multimedia USB Keyboard
> [ 3.789221] input: Multimedia USB Keyboard Multimedia USB Keyboard as /devices/pci0000:00/0000:00:10.0/usb2/2
> -2/2-2.1/2-2.1:1.0/input/input4
> [ 3.789585] generic-usb 0003:058F:9462.0001: input: USB HID v1.10 Keyboard [Multimedia USB Keyboard Multimedi
> a USB Keyboard] on usb-0000:00:10.0-2.1/input0
> [ 3.818001] generic-usb: probe of 0003:058F:9462.0002 failed with error -22

> Could the crash be related to the generic-usb probe error?

Yes, it could. Can you post the contents of
/sys/kernel/debug/usb/devices (after mounting a debugfs filesystem on
/sys/kernel/debug), with the keyboard plugged in?

Alan Stern

2010-02-08 18:48:17

by Bruno Prémont

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Mon, 8 Feb 2010, Bruno [UTF-8] Prémont wrote:
> 2.6.33-rc7 (don't know if any previous version resumes properly)
> crashes during resume from S2Ram when my USB keyboard is connected
> but resumes properly (viafb corruption put apart) when the USB
> keyboard is not connected.
>
> Keyboard detection:
> [ 3.070054] usb 2-2: new full speed USB device using uhci_hcd and address 2
> [ 3.220179] kbd_mode used greatest stack depth: 2228 bytes left
> [ 3.276403] usb 2-2: New USB device found, idVendor=058f, idProduct=9462
> [ 3.276514] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> [ 3.276619] usb 2-2: Product: Multimedia USB Keyboard
> [ 3.276711] usb 2-2: Manufacturer: Multimedia USB Keyboard
> [ 3.278056] loadkeys used greatest stack depth: 1904 bytes left
> [ 3.278791] init-early.sh used greatest stack depth: 1700 bytes left
> [ 3.282561] hub 2-2:1.0: USB hub found
> [ 3.286387] hub 2-2:1.0: 3 ports detected
> [ 3.571454] usb 2-2.1: new full speed USB device using uhci_hcd and address 3
> [ 3.761474] usb 2-2.1: New USB device found, idVendor=058f, idProduct=9462
> [ 3.761584] usb 2-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 3.761719] usb 2-2.1: Product: Multimedia USB Keyboard
> [ 3.762518] usb 2-2.1: Manufacturer: Multimedia USB Keyboard
> [ 3.762612] usb 2-2.1: SerialNumber: Multimedia USB Keyboard
> [ 3.789221] input: Multimedia USB Keyboard Multimedia USB Keyboard as /devices/pci0000:00/0000:00:10.0/usb2/2-2/2-2.1/2-2.1:1.0/input/input4
> [ 3.789585] generic-usb 0003:058F:9462.0001: input: USB HID v1.10 Keyboard [Multimedia USB Keyboard Multimedia USB Keyboard] on usb-0000:00:10.0-2.1/input0
> [ 3.818001] generic-usb: probe of 0003:058F:9462.0002 failed with error -22


On Mon, 08 February 2010 Alan Stern <[email protected]> wrote:
> > Could the crash be related to the generic-usb probe error?
>
> Yes, it could. Can you post the contents of
> /sys/kernel/debug/usb/devices (after mounting a debugfs filesystem on
> /sys/kernel/debug), with the keyboard plugged in?

Here it is:

T: Bus=04 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=12 MxCh= 2
B: Alloc= 0/900 us ( 0%), #Int= 0, #Iso= 0
D: Ver= 1.10 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0001 Rev= 2.06
S: Manufacturer=Linux 2.6.33-rc7-venus uhci_hcd
S: Product=UHCI Host Controller
S: SerialNumber=0000:00:10.2
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 2 Ivl=255ms

T: Bus=03 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=12 MxCh= 2
B: Alloc= 0/900 us ( 0%), #Int= 0, #Iso= 0
D: Ver= 1.10 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0001 Rev= 2.06
S: Manufacturer=Linux 2.6.33-rc7-venus uhci_hcd
S: Product=UHCI Host Controller
S: SerialNumber=0000:00:10.1
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 2 Ivl=255ms

T: Bus=02 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=12 MxCh= 2
B: Alloc= 2/900 us ( 0%), #Int= 2, #Iso= 0
D: Ver= 1.10 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0001 Rev= 2.06
S: Manufacturer=Linux 2.6.33-rc7-venus uhci_hcd
S: Product=UHCI Host Controller
S: SerialNumber=0000:00:10.0
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 2 Ivl=255ms

T: Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 2 Spd=12 MxCh= 3
D: Ver= 1.10 Cls=09(hub ) Sub=00 Prot=00 MxPS= 8 #Cfgs= 1
P: Vendor=058f ProdID=9462 Rev= 1.58
S: Manufacturer=Multimedia USB Keyboard
S: Product=Multimedia USB Keyboard
C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr= 50mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 1 Ivl=255ms

T: Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#= 3 Spd=12 MxCh= 0
D: Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs= 1
P: Vendor=058f ProdID=9462 Rev= 4.10
S: Manufacturer=Multimedia USB Keyboard
S: Product=Multimedia USB Keyboard
S: SerialNumber=Multimedia USB Keyboard
C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr= 50mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=01 Driver=usbhid
E: Ad=81(I) Atr=03(Int.) MxPS= 8 Ivl=10ms
I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=00 Prot=00 Driver=usbhid
E: Ad=82(I) Atr=03(Int.) MxPS= 4 Ivl=255ms

T: Bus=01 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 6
B: Alloc= 0/800 us ( 0%), #Int= 0, #Iso= 0
D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0002 Rev= 2.06
S: Manufacturer=Linux 2.6.33-rc7-venus ehci_hcd
S: Product=EHCI Host Controller
S: SerialNumber=0000:00:10.4
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=256ms


Thanks,
Bruno

2010-02-08 20:25:11

by Alan Stern

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Mon, 8 Feb 2010, Bruno [UTF-8] Prémont wrote:

> On Mon, 08 February 2010 Bruno Prémont <[email protected]> wrote:
> > 2.6.33-rc7 (don't know if any previous version resumes properly)
> > crashes during resume from S2Ram when my USB keyboard is connected but
> > resumes properly (viafb corruption put apart) when the USB keyboard is
> > not connected.
>
> The patch below works around the crash though the WARN_ON() in
> usbhid_restart_out_queue() triggers in place.
>
> Bruno
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index e2997a8..d2f8eef 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -196,7 +196,7 @@ static void usbhid_mark_busy(struct usbhid_device *usbhid)
>
> static int usbhid_restart_out_queue(struct usbhid_device *usbhid)
> {
> - struct hid_device *hid = usb_get_intfdata(usbhid->intf);
> + struct hid_device *hid = usbhid->intf ? usb_get_intfdata(usbhid->intf) : NULL;
> int kicked;
>
> if (!hid)
> @@ -214,7 +214,7 @@ static int usbhid_restart_out_queue(struct usbhid_device *usbhid)
>
> static int usbhid_restart_ctrl_queue(struct usbhid_device *usbhid)
> {
> - struct hid_device *hid = usb_get_intfdata(usbhid->intf);
> + struct hid_device *hid = usbhid->intf ? usb_get_intfdata(usbhid->intf) : NULL;
> int kicked;
>
> WARN_ON(hid == NULL);

Clearly something is setting usbhid->intf to NULL. But I don't see any
code that would do it. You may have to resort to putting printk()
statements at various strategic places to find out where it happens.
You could start with the beginnings and ends of hid_suspend,
hid_resume, and hid_reset_resume. Maybe also usbhid_disconnect(), just
in case.

Alan Stern

2010-02-13 13:04:17

by Bruno Prémont

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Mon, 08 February 2010 Alan Stern <[email protected]> wrote:
> Clearly something is setting usbhid->intf to NULL. But I don't see
> any code that would do it. You may have to resort to putting
> printk() statements at various strategic places to find out where it
> happens. You could start with the beginnings and ends of hid_suspend,
> hid_resume, and hid_reset_resume. Maybe also usbhid_disconnect(),
> just in case.

I did add a few printk()s and WARN_ON()s to get a better idea of
why/when usbhid->intf is NULL and it is already since probe time of the
second interface anounced by the USB keyboard (hid.debug=1):

[ 3.822393] usb 2-2.1: new full speed USB device using uhci_hcd and address 3
[ 4.011388] usb 2-2.1: New USB device found, idVendor=058f, idProduct=9462
[ 4.011502] usb 2-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 4.011639] usb 2-2.1: Product: Multimedia USB Keyboard
[ 4.011733] usb 2-2.1: Manufacturer: Multimedia USB Keyboard
[ 4.011826] usb 2-2.1: SerialNumber: Multimedia USB Keyboard
[ 4.014514] /usr/src/linux-2.6-git/drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 0
[ 4.037712] /usr/src/linux-2.6-git/drivers/hid/usbhid/hid-core.c: submitting ctrl urb: Set_Report wValue=0x0200 wIndex=0x0000 wLength=1
[ 4.038160] input: Multimedia USB Keyboard Multimedia USB Keyboard as /devices/pci0000:00/0000:00:10.0/usb2/2-2/2-2.1/2-2.1:1.0/input/input4
[ 4.038523] generic-usb 0003:058F:9462.0001: input: USB HID v1.10 Keyboard [Multimedia USB Keyboard Multimedia USB Keyboard] on usb-0000:00:10.0-2.1/input0
[ 4.038901] /usr/src/linux-2.6-git/drivers/hid/usbhid/hid-core.c: HID probe called for ifnum 1
[ 4.066881] /usr/src/linux-2.6-git/drivers/hid/hid-core.c: usage index exceeded
[ 4.066894] /usr/src/linux-2.6-git/drivers/hid/hid-core.c: hid_add_usage failed
[ 4.066905] /usr/src/linux-2.6-git/drivers/hid/hid-core.c: item 0 2 2 2 parsing failed
[ 4.066931] /usr/src/linux-2.6-git/drivers/hid/usbhid/hid-core.c: parsing report descriptor failed
>>>> following WARNING comes from WARN_ON() I added to usbhid_parse
>>>> to know what the call stack is up to the failing report parsing
[ 4.066941] ------------[ cut here ]------------
[ 4.067065] WARNING: at /usr/src/linux-2.6-git/drivers/hid/usbhid/hid-core.c:891 usbhid_parse+0x1db/0x340()
[ 4.067226] Hardware name: CX700+W697HG
[ 4.067316] Modules linked in:
[ 4.067463] Pid: 228, comm: khubd Not tainted 2.6.33-rc7-venus #6
[ 4.067560] Call Trace:
[ 4.067662] [<c1330cdd>] ? printk+0x18/0x1b
[ 4.067763] [<c12855db>] ? usbhid_parse+0x1db/0x340
[ 4.067873] [<c10251ec>] warn_slowpath_common+0x6c/0xc0
[ 4.067976] [<c12855db>] ? usbhid_parse+0x1db/0x340
[ 4.068080] [<c1025255>] warn_slowpath_null+0x15/0x20
[ 4.068183] [<c12855db>] usbhid_parse+0x1db/0x340
[ 4.068293] [<c127ac85>] hid_device_probe+0x155/0x170
[ 4.068396] [<c11ec838>] driver_probe_device+0x68/0x160
[ 4.068500] [<c127a2a8>] ? hid_bus_match+0x88/0x160
[ 4.068605] [<c11ec9f1>] __device_attach+0x41/0x50
[ 4.068707] [<c11ebe53>] bus_for_each_drv+0x53/0x80
[ 4.068810] [<c11eca9b>] device_attach+0x6b/0x70
[ 4.068911] [<c11ec9b0>] ? __device_attach+0x0/0x50
[ 4.069014] [<c11ebc4f>] bus_probe_device+0x1f/0x40
[ 4.069117] [<c11ea557>] device_add+0x357/0x570
[ 4.069224] [<c117a693>] ? kvasprintf+0x43/0x60
[ 4.069326] [<c1172c52>] ? kobject_set_name_vargs+0x62/0x70
[ 4.069432] [<c127a76e>] hid_add_device+0x14e/0x1d0
[ 4.069579] [<c1286012>] usbhid_probe+0x202/0x360
[ 4.069685] [<c1230e8f>] usb_probe_interface+0xaf/0x1c0
[ 4.069791] [<c11ec742>] ? driver_sysfs_add+0x52/0x70
[ 4.069895] [<c11ec838>] driver_probe_device+0x68/0x160
[ 4.070000] [<c122fd90>] ? usb_device_match+0x50/0xb0
[ 4.070135] [<c11ec9f1>] __device_attach+0x41/0x50
[ 4.070234] [<c11ebe53>] bus_for_each_drv+0x53/0x80
[ 4.070338] [<c11eca9b>] device_attach+0x6b/0x70
[ 4.070434] [<c11ec9b0>] ? __device_attach+0x0/0x50
[ 4.070530] [<c11ebc4f>] bus_probe_device+0x1f/0x40
[ 4.070626] [<c11ea557>] device_add+0x357/0x570
[ 4.070722] [<c1234ccc>] ? usb_create_ep_devs+0x7c/0xb0
[ 4.070821] [<c122db03>] ? create_intf_ep_devs+0x43/0x70
[ 4.070919] [<c122f7e7>] usb_set_configuration+0x4a7/0x640
[ 4.071019] [<c1237ff9>] generic_probe+0x39/0xb0
[ 4.071120] [<c10c4352>] ? sysfs_create_link+0x12/0x20
[ 4.071218] [<c122fb5f>] usb_probe_device+0x1f/0x30
[ 4.071314] [<c11ec838>] driver_probe_device+0x68/0x160
[ 4.071412] [<c11ec9f1>] __device_attach+0x41/0x50
[ 4.071508] [<c11ebe53>] bus_for_each_drv+0x53/0x80
[ 4.071605] [<c11eca9b>] device_attach+0x6b/0x70
[ 4.071700] [<c11ec9b0>] ? __device_attach+0x0/0x50
[ 4.071797] [<c11ebc4f>] bus_probe_device+0x1f/0x40
[ 4.071892] [<c11ea557>] device_add+0x357/0x570
[ 4.071987] [<c1330cdd>] ? printk+0x18/0x1b
[ 4.072081] [<c12266ab>] ? show_string+0x4b/0x50
[ 4.072177] [<c12292d6>] usb_new_device+0x116/0x180
[ 4.072274] [<c122aadf>] hub_thread+0xdbf/0x11d0
[ 4.072372] [<c1021377>] ? dequeue_task_fair+0x27/0x1d0
[ 4.072470] [<c102106e>] ? set_next_entity+0x2e/0x70
[ 4.072567] [<c1021ef1>] ? finish_task_switch+0x31/0x80
[ 4.072669] [<c10374b0>] ? autoremove_wake_function+0x0/0x50
[ 4.072767] [<c1229d20>] ? hub_thread+0x0/0x11d0
[ 4.072863] [<c10370ec>] kthread+0x6c/0x80
[ 4.072958] [<c1037080>] ? kthread+0x0/0x80
[ 4.073053] [<c10030f6>] kernel_thread_helper+0x6/0x10
[ 4.073146] ---[ end trace 74d7f471f706deb5 ]---
[ 4.073256] generic-usb: probe of 0003:058F:9462.0002 failed with error -22
>>>> This is the WARN_ON(usbhid->intf ==NULL) I added just before
>>>> return 0 to usbhid_probe() to confirm that intf is already NULL
>>>> since the very beginning for this HID device
[ 4.073378] ------------[ cut here ]------------
[ 4.073470] WARNING: at /usr/src/linux-2.6-git/drivers/hid/usbhid/hid-core.c:1166 usbhid_probe+0x2d9/0x360()
[ 4.073606] Hardware name: CX700+W697HG
[ 4.073694] Modules linked in:
[ 4.073828] Pid: 228, comm: khubd Tainted: G W 2.6.33-rc7-venus #6
[ 4.073923] Call Trace:
[ 4.074013] [<c1330cdd>] ? printk+0x18/0x1b
[ 4.074106] [<c12860e9>] ? usbhid_probe+0x2d9/0x360
[ 4.074203] [<c10251ec>] warn_slowpath_common+0x6c/0xc0
[ 4.074299] [<c12860e9>] ? usbhid_probe+0x2d9/0x360
[ 4.074397] [<c1025255>] warn_slowpath_null+0x15/0x20
[ 4.074493] [<c12860e9>] usbhid_probe+0x2d9/0x360
[ 4.074590] [<c1230e8f>] usb_probe_interface+0xaf/0x1c0
[ 4.074688] [<c11ec742>] ? driver_sysfs_add+0x52/0x70
[ 4.074785] [<c11ec838>] driver_probe_device+0x68/0x160
[ 4.074881] [<c122fd90>] ? usb_device_match+0x50/0xb0
[ 4.074979] [<c11ec9f1>] __device_attach+0x41/0x50
[ 4.075074] [<c11ebe53>] bus_for_each_drv+0x53/0x80
[ 4.075171] [<c11eca9b>] device_attach+0x6b/0x70
[ 4.075266] [<c11ec9b0>] ? __device_attach+0x0/0x50
[ 4.075363] [<c11ebc4f>] bus_probe_device+0x1f/0x40
[ 4.075458] [<c11ea557>] device_add+0x357/0x570
[ 4.075553] [<c1234ccc>] ? usb_create_ep_devs+0x7c/0xb0
[ 4.075650] [<c122db03>] ? create_intf_ep_devs+0x43/0x70
[ 4.075749] [<c122f7e7>] usb_set_configuration+0x4a7/0x640
[ 4.075847] [<c1237ff9>] generic_probe+0x39/0xb0
[ 4.075944] [<c10c4352>] ? sysfs_create_link+0x12/0x20
[ 4.076041] [<c122fb5f>] usb_probe_device+0x1f/0x30
[ 4.076138] [<c11ec838>] driver_probe_device+0x68/0x160
[ 4.076235] [<c11ec9f1>] __device_attach+0x41/0x50
[ 4.076331] [<c11ebe53>] bus_for_each_drv+0x53/0x80
[ 4.076428] [<c11eca9b>] device_attach+0x6b/0x70
[ 4.076523] [<c11ec9b0>] ? __device_attach+0x0/0x50
[ 4.076619] [<c11ebc4f>] bus_probe_device+0x1f/0x40
[ 4.076714] [<c11ea557>] device_add+0x357/0x570
[ 4.076809] [<c1330cdd>] ? printk+0x18/0x1b
[ 4.076902] [<c12266ab>] ? show_string+0x4b/0x50
[ 4.076997] [<c12292d6>] usb_new_device+0x116/0x180
[ 4.077094] [<c122aadf>] hub_thread+0xdbf/0x11d0
[ 4.077191] [<c1021377>] ? dequeue_task_fair+0x27/0x1d0
[ 4.077288] [<c102106e>] ? set_next_entity+0x2e/0x70
[ 4.077384] [<c1021ef1>] ? finish_task_switch+0x31/0x80
[ 4.077482] [<c10374b0>] ? autoremove_wake_function+0x0/0x50
[ 4.077579] [<c1229d20>] ? hub_thread+0x0/0x11d0
[ 4.077674] [<c10370ec>] kthread+0x6c/0x80
[ 4.077768] [<c1037080>] ? kthread+0x0/0x80
[ 4.077861] [<c10030f6>] kernel_thread_helper+0x6/0x10
[ 4.077954] ---[ end trace 74d7f471f706deb6 ]---
[ 5.401011] udev: starting version 146
[ 5.672965] Linux agpgart interface v0.103
[ 5.776141] sd 0:0:0:0: Attached scsi generic sg0 type 0
[ 5.776327] sd 1:0:1:0: Attached scsi generic sg1 type 0
[ 5.968845] VIA Graphics Intergration Chipset framebuffer 2.4 initializing
[ 6.145620] agpgart: Detected VIA CX700 chipset
[ 6.155095] agpgart-via 0000:00:00.0: AGP aperture is 128M @ 0xd0000000

This lets me guess that hid_add_device() is doing something wrong
here when report parsing fails... (as that one is the only one which
could be doing the initialization of usbhid which does work for the
first interface announced by my keyboard)

Bruno

2010-02-13 17:46:14

by Alan Stern

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Sat, 13 Feb 2010, Bruno [UTF-8] Prémont wrote:

> On Mon, 08 February 2010 Alan Stern <[email protected]> wrote:
> > Clearly something is setting usbhid->intf to NULL. But I don't see
> > any code that would do it. You may have to resort to putting
> > printk() statements at various strategic places to find out where it
> > happens. You could start with the beginnings and ends of hid_suspend,
> > hid_resume, and hid_reset_resume. Maybe also usbhid_disconnect(),
> > just in case.
>
> I did add a few printk()s and WARN_ON()s to get a better idea of
> why/when usbhid->intf is NULL and it is already since probe time of the
> second interface anounced by the USB keyboard (hid.debug=1):

...

> This lets me guess that hid_add_device() is doing something wrong
> here when report parsing fails... (as that one is the only one which
> could be doing the initialization of usbhid which does work for the
> first interface announced by my keyboard)

I don't know about doing anything wrong... However it does appear that
in this case the interface is registered on the HID bus but doesn't get
bound to a driver. Jiri will know whether or not that's the desired
outcome.

On the other hand, I don't think there would be anything wrong with
moving the

usbhid->intf = intf;
usbhid->ifnum = interface->desc.bInterfaceNumber;

lines from usbhid_start() to usbhid_probe(), just before the call to
hid_add_device(). It should fix the bug, and those lines do belong in
the probe routine. Jiri, any problems with doing this?

Alan Stern

2010-02-13 18:36:42

by Bruno Prémont

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Sat, 13 February 2010 Alan Stern <[email protected]> wrote:
>
> On Sat, 13 Feb 2010, Bruno [UTF-8] Prémont wrote:
>
> > On Mon, 08 February 2010 Alan Stern <[email protected]>
> > wrote:
> > > Clearly something is setting usbhid->intf to NULL. But I don't
> > > see any code that would do it. You may have to resort to putting
> > > printk() statements at various strategic places to find out where
> > > it happens. You could start with the beginnings and ends of
> > > hid_suspend, hid_resume, and hid_reset_resume. Maybe also
> > > usbhid_disconnect(), just in case.
> >
> > I did add a few printk()s and WARN_ON()s to get a better idea of
> > why/when usbhid->intf is NULL and it is already since probe time of
> > the second interface anounced by the USB keyboard (hid.debug=1):
>
> ...
>
> > This lets me guess that hid_add_device() is doing something wrong
> > here when report parsing fails... (as that one is the only one which
> > could be doing the initialization of usbhid which does work for the
> > first interface announced by my keyboard)
>
> I don't know about doing anything wrong... However it does appear
> that in this case the interface is registered on the HID bus but
> doesn't get bound to a driver. Jiri will know whether or not that's
> the desired outcome.
>
> On the other hand, I don't think there would be anything wrong with
> moving the
>
> usbhid->intf = intf;
> usbhid->ifnum = interface->desc.bInterfaceNumber;
>
> lines from usbhid_start() to usbhid_probe(), just before the call to
> hid_add_device(). It should fix the bug, and those lines do belong in
> the probe routine. Jiri, any problems with doing this?

With the below patch (which is only half of the move work) I don't
get crashes anymore. Though I wonder if other initialization steps
(like the spin_lock_init() right before setting usbhid->intf) would
need to be moved as well.

Bruno

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index e2997a8..690ca75 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1154,6 +1154,8 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *

hid->driver_data = usbhid;
usbhid->hid = hid;
+ usbhid->intf = intf;
+ usbhid->ifnum = interface->desc.bInterfaceNumber;

ret = hid_add_device(hid);
if (ret) {

2010-02-17 13:24:16

by Jiri Kosina

[permalink] [raw]
Subject: Re: S2R resume crash in 2.6.33-rc7 - NULL pointer dereference in dev_get_drvdata() for usbhid

On Sat, 13 Feb 2010, Bruno Prémont wrote:

> > > This lets me guess that hid_add_device() is doing something wrong
> > > here when report parsing fails... (as that one is the only one which
> > > could be doing the initialization of usbhid which does work for the
> > > first interface announced by my keyboard)
> >
> > I don't know about doing anything wrong... However it does appear
> > that in this case the interface is registered on the HID bus but
> > doesn't get bound to a driver. Jiri will know whether or not that's
> > the desired outcome.
> >
> > On the other hand, I don't think there would be anything wrong with
> > moving the
> >
> > usbhid->intf = intf;
> > usbhid->ifnum = interface->desc.bInterfaceNumber;
> >
> > lines from usbhid_start() to usbhid_probe(), just before the call to
> > hid_add_device(). It should fix the bug, and those lines do belong in
> > the probe routine. Jiri, any problems with doing this?
>
> With the below patch (which is only half of the move work) I don't
> get crashes anymore. Though I wonder if other initialization steps
> (like the spin_lock_init() right before setting usbhid->intf) would
> need to be moved as well.
>
> Bruno
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index e2997a8..690ca75 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1154,6 +1154,8 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
>
> hid->driver_data = usbhid;
> usbhid->hid = hid;
> + usbhid->intf = intf;
> + usbhid->ifnum = interface->desc.bInterfaceNumber;
>
> ret = hid_add_device(hid);
> if (ret) {
>

Thanks for beating me with looking into this guys.

I have pushed fix for this into for-next branch, so it should appear in
tomorrow linux-next pile. If you still experience the problem, please let
me know.

Thanks,

--
Jiri Kosina
SUSE Labs, Novell Inc.