| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1159181501.5018.17.camel@lappy>
Date: Mon, 25 Sep 2006 12:51:41 +0200
From: Peter Zijlstra <a.p.zijlstra@...llo.nl>
To: gregkh@...e.de, Andrew Morton <akpm@...l.org>
Cc: linux-usb-devel@...ts.sourceforge.net,
linux-kernel <linux-kernel@...r.kernel.org>,
Dave Jones <davej@...hat.com>, Ingo Molnar <mingo@...e.hu>,
arjan <arjan@...radead.org>
Subject: [PATCH] usb-serial: possible irq lock inversion (PPP vs.
usb/serial)
=========================================================
[ INFO: possible irq lock inversion dependency detected ]
---------------------------------------------------------
ksoftirqd/0/3 just changed the state of lock:
(&ap->xmit_lock){-+..}, at: [<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
but this lock took another, soft-irq-unsafe lock in the past:
(&port->lock){--..}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
no locks held by ksoftirqd/0/3.
the first lock's dependencies:
-> (&ap->xmit_lock){-+..} ops: 0 {
initial-use at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c06086a8>] _spin_lock_bh+0x1e/0x2d
[<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
[<f93375b8>] ppp_async_send+0x10/0x3d [ppp_async]
[<f932f071>] ppp_channel_push+0x3a/0x94 [ppp_generic]
[<f9330395>] ppp_write+0xd5/0xe1 [ppp_generic]
[<c0471f23>] vfs_write+0xab/0x157
[<c0472568>] sys_write+0x3b/0x60
[<c0403faf>] syscall_call+0x7/0xb
in-softirq-W at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c06086a8>] _spin_lock_bh+0x1e/0x2d
[<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
[<f9337aea>] ppp_async_process+0x48/0x5b [ppp_async]
[<c04294b4>] tasklet_action+0x65/0xca
[<c04293d5>] __do_softirq+0x78/0xf2
[<c040662f>] do_softirq+0x5a/0xbe
hardirq-on-W at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c06086a8>] _spin_lock_bh+0x1e/0x2d
[<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
[<f93375b8>] ppp_async_send+0x10/0x3d [ppp_async]
[<f932f071>] ppp_channel_push+0x3a/0x94 [ppp_generic]
[<f9330395>] ppp_write+0xd5/0xe1 [ppp_generic]
[<c0471f23>] vfs_write+0xab/0x157
[<c0472568>] sys_write+0x3b/0x60
[<c0403faf>] syscall_call+0x7/0xb
}
... key at: [<f933b208>] __key.19284+0x0/0xffffce72 [ppp_async]
-> (&port->lock){--..} ops: 0 {
initial-use at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c060867b>] _spin_lock+0x19/0x28
[<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
[<f9322531>] serial_write+0x8a/0x99 [usbserial]
[<c052dbed>] write_chan+0x22e/0x2a8
[<c052b530>] tty_write+0x148/0x1ce
[<c0471f23>] vfs_write+0xab/0x157
[<c0472568>] sys_write+0x3b/0x60
[<c0403faf>] syscall_call+0x7/0xb
softirq-on-W at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c060867b>] _spin_lock+0x19/0x28
[<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
[<f9322531>] serial_write+0x8a/0x99 [usbserial]
[<c052dbed>] write_chan+0x22e/0x2a8
[<c052b530>] tty_write+0x148/0x1ce
[<c0471f23>] vfs_write+0xab/0x157
[<c0472568>] sys_write+0x3b/0x60
[<c0403faf>] syscall_call+0x7/0xb
hardirq-on-W at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c060867b>] _spin_lock+0x19/0x28
[<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
[<f9322531>] serial_write+0x8a/0x99 [usbserial]
[<c052dbed>] write_chan+0x22e/0x2a8
[<c052b530>] tty_write+0x148/0x1ce
[<c0471f23>] vfs_write+0xab/0x157
[<c0472568>] sys_write+0x3b/0x60
[<c0403faf>] syscall_call+0x7/0xb
}
... key at: [<f932b08c>] __key.15523+0x0/0xffff9965 [usbserial]
... acquired at:
[<c043bf43>] lock_acquire+0x4b/0x6c
[<c060867b>] _spin_lock+0x19/0x28
[<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
[<f9322531>] serial_write+0x8a/0x99 [usbserial]
[<f933729c>] ppp_async_push+0xa7/0x3b3 [ppp_async]
[<f93375da>] ppp_async_send+0x32/0x3d [ppp_async]
[<f932f071>] ppp_channel_push+0x3a/0x94 [ppp_generic]
[<f9330395>] ppp_write+0xd5/0xe1 [ppp_generic]
[<c0471f23>] vfs_write+0xab/0x157
[<c0472568>] sys_write+0x3b/0x60
[<c0403faf>] syscall_call+0x7/0xb
Change port->lock locking to softirq-safe, other locks down the
call-chain appear to be hardirq-safe so this should do.
(compile tested only due to lack of hardware)
Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>
---
drivers/usb/serial/cyberjack.c | 6 +++---
drivers/usb/serial/generic.c | 6 +++---
drivers/usb/serial/ipw.c | 6 +++---
drivers/usb/serial/ir-usb.c | 6 +++---
drivers/usb/serial/keyspan_pda.c | 6 +++---
drivers/usb/serial/omninet.c | 6 +++---
drivers/usb/serial/safe_serial.c | 6 +++---
7 files changed, 21 insertions(+), 21 deletions(-)
Index: linux-2.6-mm/drivers/usb/serial/cyberjack.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/cyberjack.c
+++ linux-2.6-mm/drivers/usb/serial/cyberjack.c
@@ -214,14 +214,14 @@ static int cyberjack_write (struct usb_s
return (0);
}
- spin_lock(&port->lock);
+ spin_lock_bh(&port->lock);
if (port->write_urb_busy) {
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
}
port->write_urb_busy = 1;
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
spin_lock_irqsave(&priv->lock, flags);
Index: linux-2.6-mm/drivers/usb/serial/generic.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/generic.c
+++ linux-2.6-mm/drivers/usb/serial/generic.c
@@ -175,14 +175,14 @@ int usb_serial_generic_write(struct usb_
/* only do something if we have a bulk out endpoint */
if (serial->num_bulk_out) {
- spin_lock(&port->lock);
+ spin_lock_bh(&port->lock);
if (port->write_urb_busy) {
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
}
port->write_urb_busy = 1;
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
count = (count > port->bulk_out_size) ? port->bulk_out_size : count;
Index: linux-2.6-mm/drivers/usb/serial/ipw.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/ipw.c
+++ linux-2.6-mm/drivers/usb/serial/ipw.c
@@ -394,14 +394,14 @@ static int ipw_write(struct usb_serial_p
return 0;
}
- spin_lock(&port->lock);
+ spin_lock_bh(&port->lock);
if (port->write_urb_busy) {
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
}
port->write_urb_busy = 1;
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
count = min(count, port->bulk_out_size);
memcpy(port->bulk_out_buffer, buf, count);
Index: linux-2.6-mm/drivers/usb/serial/ir-usb.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/ir-usb.c
+++ linux-2.6-mm/drivers/usb/serial/ir-usb.c
@@ -342,14 +342,14 @@ static int ir_write (struct usb_serial_p
if (count == 0)
return 0;
- spin_lock(&port->lock);
+ spin_lock_bh(&port->lock);
if (port->write_urb_busy) {
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
}
port->write_urb_busy = 1;
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
transfer_buffer = port->write_urb->transfer_buffer;
transfer_size = min(count, port->bulk_out_size - 1);
Index: linux-2.6-mm/drivers/usb/serial/keyspan_pda.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/keyspan_pda.c
+++ linux-2.6-mm/drivers/usb/serial/keyspan_pda.c
@@ -518,13 +518,13 @@ static int keyspan_pda_write(struct usb_
the TX urb is in-flight (wait until it completes)
the device is full (wait until it says there is room)
*/
- spin_lock(&port->lock);
+ spin_lock_bh(&port->lock);
if (port->write_urb_busy || priv->tx_throttled) {
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
return 0;
}
port->write_urb_busy = 1;
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
/* At this point the URB is in our control, nobody else can submit it
again (the only sudden transition was the one from EINPROGRESS to
Index: linux-2.6-mm/drivers/usb/serial/omninet.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/omninet.c
+++ linux-2.6-mm/drivers/usb/serial/omninet.c
@@ -256,14 +256,14 @@ static int omninet_write (struct usb_ser
return (0);
}
- spin_lock(&wport->lock);
+ spin_lock_bh(&wport->lock);
if (wport->write_urb_busy) {
- spin_unlock(&wport->lock);
+ spin_unlock_bh(&wport->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
}
wport->write_urb_busy = 1;
- spin_unlock(&wport->lock);
+ spin_unlock_bh(&wport->lock);
count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count;
Index: linux-2.6-mm/drivers/usb/serial/safe_serial.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/safe_serial.c
+++ linux-2.6-mm/drivers/usb/serial/safe_serial.c
@@ -298,14 +298,14 @@ static int safe_write (struct usb_serial
dbg ("%s - write request of 0 bytes", __FUNCTION__);
return (0);
}
- spin_lock(&port->lock);
+ spin_lock_bh(&port->lock);
if (port->write_urb_busy) {
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
}
port->write_urb_busy = 1;
- spin_unlock(&port->lock);
+ spin_unlock_bh(&port->lock);
packet_length = port->bulk_out_size; // get max packetsize
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists