2004-06-23 06:31:18

by Zwane Mwaikambo

[permalink] [raw]
Subject: Oops w/ USB serial + modular ipaq

Loading the ipaq module, connecting a device and then unloading ipaq.ko
oopses.

PocketPC PDA ttyUSB0: PocketPC PDA converter now disconnected from ttyUSB0
usbserial 1-2:1.0: device disconnected
Unable to handle kernel NULL pointer dereference at virtual address 00000085
printing eip:
f8990a6f
*pde = 00000000
Oops: 0000 [#1]
SMP
Modules linked in: ppp_async ppp_generic slhc ipaq usbserial vmnet vmmon
dm_mod e100 e1000 3c59x
CPU: 0
EIP: 0060:[<f8990a6f>] Tainted: P VLI
EFLAGS: 00210246 (2.6.7-rc3-mm2-slock)
EIP is at usb_serial_disconnect+0x1b/0x86 [usbserial]
eax: 00000000 ebx: 00000011 ecx: 00000002 edx: 00000000
esi: 00000000 edi: 00000001 ebp: d9d0c000 esp: d9d0cf38
ds: 007b es: 007b ss: 0068
Process rmmod (pid: 19148, threadinfo=d9d0c000 task=e62ced10)
Stack: f8993f40 c02ef8f6 c1d61b48 c1ec46d8 00000000 f8998220 f8990e04 f89924c0
f8991e8c f8996cc8 f8998480 c04631a4 00000000 c0128b94 00000000 71617069
00000000 d5d33a80 d5d33a80 c013cac9 40001000 40000000 c013ce17 40000000
Call Trace:
[<c02ef8f6>] device_release_driver+0x56/0x58
[<f8990e04>] usb_serial_deregister+0x9b/0x9f [usbserial]
[<c0128b94>] sys_delete_module+0x132/0x180
[<c013cac9>] unmap_vma_list+0xe/0x17
[<c013ce17>] do_munmap+0x10a/0x144
[<c0110df4>] do_page_fault+0x0/0x501
[<c0103a71>] sysenter_past_esp+0x52/0x71

Code: 04 24 b3 1d 99 f8 e8 89 65 78 c7 e9 8a f5 ff ff 83 ec 18 89 5c 24 0c
89 7c 24 14 89 74 24 10 8d 58 10 89 c7 a1 04 46 99 f8 85 c0 <8b> 73 74 75
48 85 f6 c7 43 74 00 00 00 00 74 08 8d 46 38 e8 47

(gdb) list * usb_serial_disconnect+0x1b
0x1a6f is in usb_serial_disconnect (device.h:298).
293 }
294
295 static inline void *
296 dev_get_drvdata (struct device *dev)
297 {
298 return dev->driver_data;
299 }


The problem is due to the following;

void usb_serial_deregister(struct usb_serial_device_type *device)
{
struct usb_serial *serial;
int i;

for(i = 0; i < SERIAL_TTY_MINORS; ++i) {
serial = serial_table[i];
if ((serial != NULL) && (serial->type == device)) {
printk("usb_serial_deregister: %p %p\n", serial, serial->interface);
usb_driver_release_interface (&usb_serial_driver, serial->interface);
usb_serial_disconnect (serial->interface); <===
}
}
...
}

It's not safe to use serial->interface after that
usb_driver_release_interface(). The following patch works as a workaround
but i don't trust it as there may be a leak.

Index: linux-2.6.7/drivers/usb/serial/usb-serial.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.7/drivers/usb/serial/usb-serial.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 usb-serial.c
--- linux-2.6.7/drivers/usb/serial/usb-serial.c 16 Jun 2004 16:49:38 -0000 1.1.1.1
+++ linux-2.6.7/drivers/usb/serial/usb-serial.c 23 Jun 2004 06:29:56 -0000
@@ -1393,7 +1393,6 @@ void usb_serial_deregister(struct usb_se
serial = serial_table[i];
if ((serial != NULL) && (serial->type == device)) {
usb_driver_release_interface (&usb_serial_driver, serial->interface);
- usb_serial_disconnect (serial->interface);
}
}


2004-06-23 08:19:44

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: Oops w/ USB serial + modular ipaq

On Wed, 23 Jun 2004, Zwane Mwaikambo wrote:

> Loading the ipaq module, connecting a device and then unloading ipaq.ko
> oopses.
>
> It's not safe to use serial->interface after that
> usb_driver_release_interface(). The following patch works as a workaround
> but i don't trust it as there may be a leak.
>
> Index: linux-2.6.7/drivers/usb/serial/usb-serial.c
> ===================================================================
> RCS file: /home/cvsroot/linux-2.6.7/drivers/usb/serial/usb-serial.c,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 usb-serial.c
> --- linux-2.6.7/drivers/usb/serial/usb-serial.c 16 Jun 2004 16:49:38 -0000 1.1.1.1
> +++ linux-2.6.7/drivers/usb/serial/usb-serial.c 23 Jun 2004 06:29:56 -0000
> @@ -1393,7 +1393,6 @@ void usb_serial_deregister(struct usb_se
> serial = serial_table[i];
> if ((serial != NULL) && (serial->type == device)) {
> usb_driver_release_interface (&usb_serial_driver, serial->interface);
> - usb_serial_disconnect (serial->interface);
> }
> }
>

I'll leave this patch going in a load/unload loop overnight and check on
slab.

Thanks,
Zwane

2004-06-23 16:18:58

by Greg KH

[permalink] [raw]
Subject: Re: Oops w/ USB serial + modular ipaq

On Wed, Jun 23, 2004 at 02:33:25AM -0400, Zwane Mwaikambo wrote:
> Loading the ipaq module, connecting a device and then unloading ipaq.ko
> oopses.
>
> PocketPC PDA ttyUSB0: PocketPC PDA converter now disconnected from ttyUSB0
> usbserial 1-2:1.0: device disconnected
> Unable to handle kernel NULL pointer dereference at virtual address 00000085
> printing eip:
> f8990a6f
> *pde = 00000000
> Oops: 0000 [#1]
> SMP
> Modules linked in: ppp_async ppp_generic slhc ipaq usbserial vmnet vmmon
> dm_mod e100 e1000 3c59x
> CPU: 0
> EIP: 0060:[<f8990a6f>] Tainted: P VLI
> EFLAGS: 00210246 (2.6.7-rc3-mm2-slock)
> EIP is at usb_serial_disconnect+0x1b/0x86 [usbserial]
> eax: 00000000 ebx: 00000011 ecx: 00000002 edx: 00000000
> esi: 00000000 edi: 00000001 ebp: d9d0c000 esp: d9d0cf38
> ds: 007b es: 007b ss: 0068
> Process rmmod (pid: 19148, threadinfo=d9d0c000 task=e62ced10)
> Stack: f8993f40 c02ef8f6 c1d61b48 c1ec46d8 00000000 f8998220 f8990e04 f89924c0
> f8991e8c f8996cc8 f8998480 c04631a4 00000000 c0128b94 00000000 71617069
> 00000000 d5d33a80 d5d33a80 c013cac9 40001000 40000000 c013ce17 40000000
> Call Trace:
> [<c02ef8f6>] device_release_driver+0x56/0x58
> [<f8990e04>] usb_serial_deregister+0x9b/0x9f [usbserial]
> [<c0128b94>] sys_delete_module+0x132/0x180
> [<c013cac9>] unmap_vma_list+0xe/0x17
> [<c013ce17>] do_munmap+0x10a/0x144
> [<c0110df4>] do_page_fault+0x0/0x501
> [<c0103a71>] sysenter_past_esp+0x52/0x71
>
> Code: 04 24 b3 1d 99 f8 e8 89 65 78 c7 e9 8a f5 ff ff 83 ec 18 89 5c 24 0c
> 89 7c 24 14 89 74 24 10 8d 58 10 89 c7 a1 04 46 99 f8 85 c0 <8b> 73 74 75
> 48 85 f6 c7 43 74 00 00 00 00 74 08 8d 46 38 e8 47
>
> (gdb) list * usb_serial_disconnect+0x1b
> 0x1a6f is in usb_serial_disconnect (device.h:298).
> 293 }
> 294
> 295 static inline void *
> 296 dev_get_drvdata (struct device *dev)
> 297 {
> 298 return dev->driver_data;
> 299 }
>
>
> The problem is due to the following;
>
> void usb_serial_deregister(struct usb_serial_device_type *device)
> {
> struct usb_serial *serial;
> int i;
>
> for(i = 0; i < SERIAL_TTY_MINORS; ++i) {
> serial = serial_table[i];
> if ((serial != NULL) && (serial->type == device)) {
> printk("usb_serial_deregister: %p %p\n", serial, serial->interface);
> usb_driver_release_interface (&usb_serial_driver, serial->interface);
> usb_serial_disconnect (serial->interface); <===
> }
> }
> ...
> }
>
> It's not safe to use serial->interface after that
> usb_driver_release_interface().

Why not? The ->interface pointer is still valid, only thing is that
device does not have a driver bound to it anymore, so the later call to
dev_info() in the usb_serial_disconnect() call might cause the oops.

How about just switching those two calls around (usb_serial_disconnect()
before calling usb_driver_release_interface())? Will that solve the
problem for you? If you just comment out usb_serial_disconnect() you
will leak memory :(

thanks,

greg k-h

2004-06-23 16:59:47

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: Oops w/ USB serial + modular ipaq

On Wed, 23 Jun 2004, Greg KH wrote:

> > It's not safe to use serial->interface after that
> > usb_driver_release_interface().
>
> Why not? The ->interface pointer is still valid, only thing is that
> device does not have a driver bound to it anymore, so the later call to
> dev_info() in the usb_serial_disconnect() call might cause the oops.

void usb_driver_release_interface(struct usb_driver *driver,
struct usb_interface *iface)
{
struct device *dev = &iface->dev;

...
dev->driver = NULL;
usb_set_intfdata(iface, NULL);
}

usb_set_intfdata() sets usb_interface->dev.driver_data to NULL, whilst in
usbserial_disconnect() we try and retrieve and use it;

void usb_serial_disconnect(struct usb_interface *interface)
{
struct usb_serial *serial = usb_get_intfdata (interface);
struct device *dev = &interface->dev;
...
}

> How about just switching those two calls around (usb_serial_disconnect()
> before calling usb_driver_release_interface())? Will that solve the
> problem for you?

I tried it last night, switching around will cause an oops in
usb_driver_release_interface()

Unable to handle kernel paging request at virtual address 6b6b6beb
printing eip:
c042da9a
*pde = 00000000
Oops: 0000 [#1]
PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in: ipaq usbserial
CPU: 1
EIP: 0060:[<c042da9a>] Not tainted
EFLAGS: 00010246 (2.6.7)
EIP is at usb_driver_release_interface+0xa/0x50
eax: 6b6b6b6b ebx: 6b6b6b7b ecx: 6b6b6b6b edx: de0a5000
esi: 00000000 edi: e0ddda60 ebp: de0a5f34 esp: de0a5f30
ds: 007b es: 007b ss: 0068
Process rmmod (pid: 1414, threadinfo=de0a5000 task=d5c85a60)
Stack: dee0bec0 de0a5f54 e0e1b11c e0e1f1c0 6b6b6b6b d72baef8 e0dddcc0 c05dd3a8
00000000 de0a5f64 e0ddbea7 e0ddda60 e0ddd9c0 de0a5fbc c0139c9e 00000000
71617069 40000000 de0a5fa0 c0156f3d d647cdc8 de801a30 d647cdfc 40001000
Call Trace:
[<c0107675>] show_stack+0x75/0x90
[<c01077d5>] show_registers+0x125/0x180
[<c010796b>] die+0xab/0x170
[<c01185d8>] do_page_fault+0x1e8/0x535
[<c01072cd>] error_code+0x2d/0x40
[<e0e1b11c>] usb_serial_deregister+0x7c/0x90 [usbserial]
[<e0ddbea7>] ipaq_exit+0x17/0x1b [ipaq]
[<c0139c9e>] sys_delete_module+0x12e/0x190
[<c0106139>] sysenter_past_esp+0x52/0x79

(gdb) list *usb_driver_release_interface+0xa
0x31a is in usb_driver_release_interface (drivers/usb/core/usb.c:347).
342 struct usb_interface *iface)
343 {
344 struct device *dev = &iface->dev;
345
346 /* this should never happen, don't release something that's not ours */
347 if (!dev->driver || dev->driver != &driver->driver)
348 return;
349
350 /* don't disconnect from disconnect(), or before dev_add() */
351 if (!list_empty (&dev->driver_list) && !list_empty (&dev->bus_list))

0x00000310 <usb_driver_release_interface+0>: push %ebp
0x00000311 <usb_driver_release_interface+1>: mov %esp,%ebp
0x00000313 <usb_driver_release_interface+3>: push %ebx
0x00000314 <usb_driver_release_interface+4>: mov 0xc(%ebp),%ecx
0x00000317 <usb_driver_release_interface+7>: lea 0x10(%ecx),%ebx
0x0000031a <usb_driver_release_interface+10>: mov 0x70(%ebx),%edx
0x0000031d <usb_driver_release_interface+13>: test %edx,%edx

hmm, iface = %ebp, dev = %ecx = 0x6b6b6b6b, how did that get
freed/poisoned since it's encapsulated by iface?

Ta,
Zwane