2.6.28-rc2 / hso driver oops

Hi.

Tried out 2.6.28-rc2 today on my EEE 901 and my Option Icon 225 and got the following oops:

hso: drivers/net/usb/hso.c: 1.2 Option Wireless
usbcore: registered new interface driver hso
usb 2-2: new full speed USB device using uhci_hcd and address 3 usb 2-2: configuration #1 chosen from 1 choice
hso0: Disabled Privacy Extensions

BUG: unable to handle kernel NULL pointer dereference at 000000d0
IP: [<c03589b9>] dev_driver_string+0x1/0x2a
*pde = 00000000 
Oops: 0000 [#1] PREEMPT SMP 
last sysfs file: /sys/devices/pci0000:00/0000:00:1d.0/usb2/idVendor
Modules linked in: hso usb_storage rfcomm l2cap rt2860sta evdev

Pid: 230, comm: khubd Not tainted (2.6.28-rc2 #2) 901
EIP: 0060:[<c03589b9>] EFLAGS: 00010246 CPU: 0
EIP is at dev_driver_string+0x1/0x2a
EAX: 00000000 EBX: f2c88480 ECX: f7159cb0 EDX: 00000001
ESI: f2fdee40 EDI: f68bd200 EBP: f7159cf8 ESP: f7159cd0
DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process khubd (pid: 230, ti=f7158000 task=f70b02b0 task.ti=f7158000)

Stack:
f7159cf8 f81b8265 f2c88480 f6ab8440 00000003 00000003 00000000 00000112
00000000 f81b90c8 f7159d48 f81b840d c029f782 f68bd200 fffffffe f6b143a8
f7159d20 f7159d2c 00000000 f6b143a8 f81b90c8 f7159d38 f7159d40 c038886b
Call Trace:
[<f81b8265>] ? hso_create_net_device+0x305/0x32d [hso]
[<f81b840d>] ? hso_probe+0x180/0x56c [hso]
[<c029f782>] ? __sysfs_add_one+0x14/0x85
[<c038886b>] ? usb_autopm_do_device+0xb1/0xb9
[<c0388d77>] ? usb_probe_interface+0xe9/0x11c
[<c035b10a>] ? driver_probe_device+0xa0/0x136
[<c035b213>] ? __device_attach+0x8/0xa
[<c035a8fe>] ? bus_for_each_drv+0x3d/0x67
[<c035b284>] ? device_attach+0x50/0x64
[<c035b20b>] ? __device_attach+0x0/0xa
[<c035a77e>] ? bus_attach_device+0x21/0x48
[<c03598e6>] ? device_add+0x307/0x456
[<c03880b2>] ? usb_set_configuration+0x400/0x449
[<c038e44b>] ? generic_probe+0x44/0x7d
[<c038828e>] ? usb_probe_device+0x32/0x38
[<c035b10a>] ? driver_probe_device+0xa0/0x136
[<c035b213>] ? __device_attach+0x8/0xa
[<c035a8fe>] ? bus_for_each_drv+0x3d/0x67
[<c035b284>] ? device_attach+0x50/0x64
[<c035b20b>] ? __device_attach+0x0/0xa
[<c035a77e>] ? bus_attach_device+0x21/0x48
[<c03598e6>] ? device_add+0x307/0x456
[<c03831d1>] ? usb_new_device+0x4d/0x8e
[<c038433c>] ? hub_thread+0x94d/0xdcb
[<c021f6b7>] ? default_wake_function+0xb/0xd
[<c0234b83>] ? autoremove_wake_function+0x0/0x33
[<c03839ef>] ? hub_thread+0x0/0xdcb
[<c02348f9>] ? kthread+0x3b/0x61
[<c02348be>] ? kthread+0x0/0x61
[<c0203c1b>] ? kernel_thread_helper+0x7/0x10
Code: 32 fe ff ff b9 81 00 00 00 b8 03 00 00 00 f3 a5 c7 83 b8 13 00 00 03 00 00 00 59 5e 89 83 8c 0b 00 00 8d 65 f4 5b 5e 5f 5d c3 55 <8b> 90 d0 00 00 00 89 e5 85 d2 75 19 8b 90 cc 00 00 00 85 d2 75 
EIP: [<c03589b9>] dev_driver_string+0x1/0x2a SS:ESP 0068:f7159cd0
---[ end trace fe2452bf3bd83a7f ]---

Stock 2.6.28-rc2 + ec_all.patch from lkml + rt2860sta driver (though I can't see how either of these are implicated I can try without if necessary).

It looks like there are only a couple of changes since 2.6.27 to this driver so I can try reverting both of them, but neither looks especially likely? (8ef5ba63b94b04b182ac4a6009dbbf1406beb3c5 & e57b641dfafc10ce23d26cf271fd2638589fdb3f)

J.

Revd. Jonathan McDowell, ULC | Shall I call the United Nations?

Something passed a null device pointer to dev_printk().

[...]

[...]

I think that hso_create_rfkill() is the culprit here (and has been inlined into hso_create_net_device()). It's using hso_dev->dev as the first argument to dev_err() and it doesn't look like that field is initialised except by kzalloc. At a guess, it should be using &hso_dev->usb->dev.

Ben.

Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Yup, this appears to be the problem, thanks. I think &hso_net->net->dev is more intuitive for the error message, so I've used that. I've also added missing line endings on the error messages and set our local rfkill structure element to NULL on failure so we don't try to call rfkill_unregister on driver removal if we failed to register at all.

The patch below Works For Me (TM); the device is detected fine, can be removed without problems and connects ok. I'll have a prod at why the rfkill stuff isn't working next, but I believe this cleanup of the error handling is appropriate no matter what the issue with registration is.

-----

index 1164c52..9d9622b 100644
@@ -2184,19 +2184,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
 			     struct usb_interface *interface)
 {
 	struct hso_net *hso_net = dev2net(hso_dev);
-	struct device *dev = hso_dev->dev;
+	struct device *dev = &hso_net->net->dev;
 	char *rfkn;
 
 	hso_net->rfkill = rfkill_allocate(&interface_to_usbdev(interface)->dev,
 				 RFKILL_TYPE_WLAN);
 	if (!hso_net->rfkill) {
-		dev_err(dev, "%s - Out of memory", __func__);
+		dev_err(dev, "%s - Out of memory\n", __func__);
 		return;
 	}
 	rfkn = kzalloc(20, GFP_KERNEL);
 	if (!rfkn) {
 		rfkill_free(hso_net->rfkill);
-		dev_err(dev, "%s - Out of memory", __func__);
+		hso_net->rfkill = NULL;
+		dev_err(dev, "%s - Out of memory\n", __func__);
 		return;
 	}
 	snprintf(rfkn, 20, "hso-%d",

@@ -2209,7 +2210,8 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
 		kfree(rfkn);
 		hso_net->rfkill->name = NULL;
 		rfkill_free(hso_net->rfkill);
-		dev_err(dev, "%s - Failed to register rfkill", __func__);
+		hso_net->rfkill = NULL;
+		dev_err(dev, "%s - Failed to register rfkill\n", __func__);
 		return;
 	}
 }

-----

J.

/-\                             | 101 things you can't have too much
|@/  Debian GNU/Linux Developer |         of : 32 - Answers.
\-                              |

Ok, I believe this is due to the addition of rfkill_check_duplicity in rfkill and the fact that test_bit actually returns a negative value rather than the postive one expected (which is of course equally true). So when the second WLAN device (the hso device, with the EEE PC WLAN being the first) comes along rfkill_check_duplicity returns a negative value and so rfkill_register returns an error. Patch below fixes this for me.

The hso driver should probably be claiming to be a WWAN device as well, given that it's GSM/HSDPA rather than 802.11.

-----

index f949a48..1ba35a7 100644
@@ -603,7 +603,7 @@ static int rfkill_check_duplicity(const struct rfkill *rfkil
        }
 
        /* 0: first switch of its kind */
-       return test_bit(rfkill->type, seen);
+       return (test_bit(rfkill->type, seen)) ? 1 : 0;
 }
 
 static int rfkill_add_switch(struct rfkill *rfkill)

-----

J.

/-\                             |  Shall I call the United Nations?
|@/  Debian GNU/Linux Developer |
\-                              |

Good catch, I screwed up on that one. Please forward it with my ack to [email protected] and to Ivo van Doorn <[email protected]> and John W. Linville <[email protected]>.

  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

(already sent to lkml & netdev, resending here with Henrique's Ack as per his followup there)

Ok, I believe this is due to the addition of rfkill_check_duplicity in rfkill and the fact that test_bit actually returns a negative value rather than the postive one expected (which is of course equally true). So when the second WLAN device (the hso device, with the EEE PC WLAN being the first) comes along rfkill_check_duplicity returns a negative value and so rfkill_register returns an error. Patch below fixes this for me.

-----

index f949a48..1ba35a7 100644
@@ -603,7 +603,7 @@ static int rfkill_check_duplicity(const struct rfkill *rfkil
        }
 
        /* 0: first switch of its kind */
-       return test_bit(rfkill->type, seen);
+       return (test_bit(rfkill->type, seen)) ? 1 : 0;
 }
 
 static int rfkill_add_switch(struct rfkill *rfkill)

-----

J.

/-\                             |  Shall I call the United Nations?
|@/  Debian GNU/Linux Developer |
\-                              |