Skip to contentSkip to conversation

digi_acceleport.c has bogus "address of" operator

PATCH In review Kurtis D. Rader 3 participants 4 messages Oct 16, 2003 – Oct 24, 2003

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 digi_acceleport.c has bogus "address of" operator

http://bugme.osdl.org/show_bug.cgi?id=1365

The digi_acceleport.c USB serial driver has a bogus "address of" operator that results in panics like this:

kernel BUG at include/asm/spinlock.h:120!
invalid operand: 0000 [#1]

Call Trace:
 [<f88e9000>] digi_wakeup_write_lock+0x0/0xaa [digi_acceleport]
 [<c0236842>] console_callback+0xa0/0xc2
 [<c0136ebc>] worker_thread+0x228/0x3ce
 [<f88e9000>] digi_wakeup_write_lock+0x0/0xaa [digi_acceleport]
 [<c011e8b6>] default_wake_function+0x0/0x2e
 [<c010993e>] ret_from_fork+0x6/0x14
 [<c011e8b6>] default_wake_function+0x0/0x2e
 [<c0136c94>] worker_thread+0x0/0x3ce
 [<c010740d>] kernel_thread_helper+0x5/0xc

The problem is that digi_wakeup_write_lock() takes a pointer to a struct usb_serial_port. However, what gets passed is a pointer to a pointer to a struct usb_serial_port.

=== diff -rup drivers/usb/serial/digi_acceleport.c.orig drivers/usb/serial/digi_acceleport.c

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700 renamed from usb/serial/digi_acceleport.c.orig 2003-10-15 22:03:26.000000000 -0700 +1 -2
@@ -1728,8 +1728,7 @@ dbg( "digi_startup: TOP" );
                init_waitqueue_head( &priv->dp_flush_wait );
                priv->dp_in_close = 0;
                init_waitqueue_head( &priv->dp_close_wait );
-               INIT_WORK(&priv->dp_wakeup_work, (void *)digi_wakeup_write_lock,
-                               (void *)(&serial->port[i]));
+               INIT_WORK(&priv->dp_wakeup_work, digi_wakeup_write_lock, serial->port[i]);
 
                /* initialize write wait queue for this port */
                init_waitqueue_head( &serial->port[i]->write_wait );
Signature · 4 lines
Kurtis D. Rader, Systems Support Engineer    service: 800-IBM-SERV
IBM Integrated Technology Services           email: [email protected]
15300 SW Koll Pkwy, MS RHE2-O2               DID: +1 503-578-3714
Beaverton, OR 97006-6063                     http://www.ibm.com
#1 lore raw

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700

Quoted 4 lines · Kurtis D. Rader

=== diff -rup drivers/usb/serial/digi_acceleport.c.orig drivers/usb/serial/digi_acceleport.c

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700 renamed from usb/serial/digi_acceleport.c.orig 2003-10-15 22:03:26.000000000 -0700 +0 -0

This patch does not apply. It looks like the tabs are converted to spaces. Can you send it to me again, so that I can apply it?

thanks,

greg k-h

#2 lore raw
Quoted 9 lines · Greg KH

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700

Quoted 4 lines · Kurtis D. Rader

=== diff -rup drivers/usb/serial/digi_acceleport.c.orig drivers/usb/serial/digi_acceleport.c

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700 renamed from usb/serial/digi_acceleport.c.orig 2003-10-15 22:03:26.000000000 -0700 +0 -0

This patch does not apply. It looks like the tabs are converted to spaces. Can you send it to me again, so that I can apply it?

It was applied. But it added a compile-time warning, which this fixes:

drivers/usb/serial/digi_acceleport.c renamed from drivers/usb/serial/digi_acceleport.c~digi_acceleport-warning-fix +3 -3
diff -puN drivers/usb/serial/digi_acceleport.c~digi_acceleport-warning-fix drivers/usb/serial/digi_acceleport.c
@@ -444,7 +444,7 @@ struct digi_port {
 /* Local Function Declarations */
 
 static void digi_wakeup_write( struct usb_serial_port *port );
-static void digi_wakeup_write_lock( struct usb_serial_port *port );
+static void digi_wakeup_write_lock(void *);
 static int digi_write_oob_command( struct usb_serial_port *port,
 	unsigned char *buf, int count, int interruptible );
 static int digi_write_inb_command( struct usb_serial_port *port,
@@ -608,9 +608,9 @@ static inline long cond_wait_interruptib
 *  on writes.
 */
 
-static void digi_wakeup_write_lock( struct usb_serial_port *port )
+static void digi_wakeup_write_lock(void *arg)
 {
-
+	struct usb_serial_port *port = arg;
 	unsigned long flags;
 	struct digi_port *priv = usb_get_serial_port_data(port);
 

_

#3 lore raw
Quoted 11 lines · Andrew Morton
Quoted 9 lines · Greg KH

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700

Quoted 4 lines · Kurtis D. Rader

=== diff -rup drivers/usb/serial/digi_acceleport.c.orig drivers/usb/serial/digi_acceleport.c

usb/serial/digi_acceleport.c 2003-10-15 21:10:21.000000000 -0700 renamed from usb/serial/digi_acceleport.c.orig 2003-10-15 22:03:26.000000000 -0700 +0 -0

This patch does not apply. It looks like the tabs are converted to spaces. Can you send it to me again, so that I can apply it?

Quoted 2 lines

It was applied. But it added a compile-time warning, which this fixes:

My apologies. Even though I knew, in the back of my mind, it was the wrong thing to do I still "cut-and-pasted" the output of the diff :-( I won't make that mistake again.

Also, note that even with the aforementioned change applied the driver is still broken and results in oopses. It's now failing (see the oops included below) a random interval after the device is opened. As soon as I resolve my current customer crit-sit involving the O(1) scheduler I'll get to the root cause of that problem, as well as any others that rear their ugly heads, and submit a proper patch against the digi_acceleport driver that makes it work in a v2.6 kernel.

The following trace is included for the benefit of the documented driver maintainers in the hope they have an idea as to the proper fix. The tainting is due to use of VMware, the use of which is unlikely to have anything to do with the digi_accelport driver problems (probability =~ 0.0).

Unable to handle kernel NULL pointer dereference at virtual address 00000050
 printing eip:
f88ee3f0
*pde = 31ce8067
*pte = 00000000
Oops: 0000 [#1]
CPU:    0
EIP:    0060:[<f88ee3f0>]    Tainted: PF 
EFLAGS: 00010002

EIP is at digi_read_oob_callback+0x2a4/0x339 [digi_acceleport]
eax: 00000001 ebx: f544adf8 ecx: 00000000 edx: 00000000
esi: f5444df8 edi: 00000004 ebp: c047fe70 esp: c047fe38
ds: 007b es: 007b ss: 0068
Process swapper (pid: 0, threadinfo=c047e000 task=c03f4ea0)

Stack: 00000000 00000046 3ccba773 f2077dc4 f56c23c0 f621fbf8 c047e000 c047e000 
       00000040 00000008 f545821c f5447df8 f5458b68 f5441df8 c047fe98 f88edda1 
       f5458b68 f5f2f860 f7f0aef8 00000082 f5458b68 f5f2f864 f5458b68 c047ff8c 
Call Trace:
 [<f88edda1>] digi_read_bulk_callback+0xdd/0x158 [digi_acceleport]
 [<f8a86965>] usb_hcd_giveback_urb+0x27/0x3c [usbcore]
 [<f88cdf35>] dl_done_list+0x1cf/0x22e [ohci_hcd]
 [<f88cef4f>] ohci_irq+0x1d3/0x2da [ohci_hcd]
 [<f8a869af>] usb_hcd_irq+0x35/0x5c [usbcore]
 [<c010bd7d>] handle_IRQ_event+0x39/0x62
 [<c010c207>] do_IRQ+0xf7/0x23e
 [<c010717a>] default_idle+0x0/0x32
 [<c010a3d4>] common_interrupt+0x18/0x20
 [<c010717a>] default_idle+0x0/0x32
 [<c01071a7>] default_idle+0x2d/0x32
 [<c0107226>] cpu_idle+0x3a/0x44
 [<c0105000>] rest_init+0x0/0x94
 [<c0480870>] start_kernel+0x1a4/0x1e6
 [<c0480428>] unknown_bootoption+0x0/0xf6
Signature · 3 lines
Kurtis D. Rader, Level 3 Linux Support and Customer Service Tool Developer
Beaverton Service Center/Linux Change Team
T/L 775-3714, DID +1 503-578-3714