Commit 053b47ff authored by Michael Buesch's avatar Michael Buesch Committed by Linus Torvalds

[PATCH] Workaround CAPI subsystem locking issue

I think the following patch should go into the kernel, until the ISDN/CAPI
guys create the real fix for this issue.

The issue is a concurrency issue with some internal CAPI data structure
which can crash the kernel.

On my FritzCard DSL with the AVM driver it crashes about once a day without
this workaround patch.  With this workaround patch it's rock-stable (at
least on UP, but I don't see why this shouldn't work on SMP as well.  But
maybe I'm missing something.)

This workaround is kind of a sledgehammer which inserts a global lock to
wrap around all the critical sections.  Of course, this is a scalability
issue, if you have many ISDN/CAPI cards.  But it prevents a crash.  So I
vote for this fix to get merged, until people come up with a better
solution.  Better have a stable kernel that's less scalable, than a
crashing and useless kernel.

This bug is in the kernel since 2.6.15 (at least).
Signed-off-by: default avatarMichael Buesch <mb@bu3sch.de>
Cc: Kai Germaschewski <kai.germaschewski@gmx.de>
Cc: Karsten Keil <kkeil@suse.de>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent a871fe85
...@@ -118,6 +118,15 @@ struct capiminor { ...@@ -118,6 +118,15 @@ struct capiminor {
}; };
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */ #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
/* FIXME: The following lock is a sledgehammer-workaround to a
* locking issue with the capiminor (and maybe other) data structure(s).
* Access to this data is done in a racy way and crashes the machine with
* a FritzCard DSL driver; sooner or later. This is a workaround
* which trades scalability vs stability, so it doesn't crash the kernel anymore.
* The correct (and scalable) fix for the issue seems to require
* an API change to the drivers... . */
static DEFINE_SPINLOCK(workaround_lock);
struct capincci { struct capincci {
struct capincci *next; struct capincci *next;
u32 ncci; u32 ncci;
...@@ -589,6 +598,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) ...@@ -589,6 +598,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */ #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
struct capincci *np; struct capincci *np;
u32 ncci; u32 ncci;
unsigned long flags;
if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_CONF) { if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_CONF) {
u16 info = CAPIMSG_U16(skb->data, 12); // Info field u16 info = CAPIMSG_U16(skb->data, 12); // Info field
...@@ -603,9 +613,11 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) ...@@ -603,9 +613,11 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
capincci_alloc(cdev, CAPIMSG_NCCI(skb->data)); capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
up(&cdev->ncci_list_sem); up(&cdev->ncci_list_sem);
} }
spin_lock_irqsave(&workaround_lock, flags);
if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) { if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) {
skb_queue_tail(&cdev->recvqueue, skb); skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait); wake_up_interruptible(&cdev->recvwait);
spin_unlock_irqrestore(&workaround_lock, flags);
return; return;
} }
ncci = CAPIMSG_CONTROL(skb->data); ncci = CAPIMSG_CONTROL(skb->data);
...@@ -615,6 +627,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) ...@@ -615,6 +627,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
printk(KERN_ERR "BUG: capi_signal: ncci not found\n"); printk(KERN_ERR "BUG: capi_signal: ncci not found\n");
skb_queue_tail(&cdev->recvqueue, skb); skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait); wake_up_interruptible(&cdev->recvwait);
spin_unlock_irqrestore(&workaround_lock, flags);
return; return;
} }
#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE #ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
...@@ -625,6 +638,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) ...@@ -625,6 +638,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
if (!mp) { if (!mp) {
skb_queue_tail(&cdev->recvqueue, skb); skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait); wake_up_interruptible(&cdev->recvwait);
spin_unlock_irqrestore(&workaround_lock, flags);
return; return;
} }
...@@ -660,6 +674,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb) ...@@ -660,6 +674,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
wake_up_interruptible(&cdev->recvwait); wake_up_interruptible(&cdev->recvwait);
} }
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */ #endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
spin_unlock_irqrestore(&workaround_lock, flags);
} }
/* -------- file_operations for capidev ----------------------------- */ /* -------- file_operations for capidev ----------------------------- */
...@@ -1006,6 +1021,7 @@ static struct file_operations capi_fops = ...@@ -1006,6 +1021,7 @@ static struct file_operations capi_fops =
static int capinc_tty_open(struct tty_struct * tty, struct file * file) static int capinc_tty_open(struct tty_struct * tty, struct file * file)
{ {
struct capiminor *mp; struct capiminor *mp;
unsigned long flags;
if ((mp = capiminor_find(iminor(file->f_path.dentry->d_inode))) == 0) if ((mp = capiminor_find(iminor(file->f_path.dentry->d_inode))) == 0)
return -ENXIO; return -ENXIO;
...@@ -1014,6 +1030,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file) ...@@ -1014,6 +1030,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
tty->driver_data = (void *)mp; tty->driver_data = (void *)mp;
spin_lock_irqsave(&workaround_lock, flags);
if (atomic_read(&mp->ttyopencount) == 0) if (atomic_read(&mp->ttyopencount) == 0)
mp->tty = tty; mp->tty = tty;
atomic_inc(&mp->ttyopencount); atomic_inc(&mp->ttyopencount);
...@@ -1021,6 +1038,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file) ...@@ -1021,6 +1038,7 @@ static int capinc_tty_open(struct tty_struct * tty, struct file * file)
printk(KERN_DEBUG "capinc_tty_open ocount=%d\n", atomic_read(&mp->ttyopencount)); printk(KERN_DEBUG "capinc_tty_open ocount=%d\n", atomic_read(&mp->ttyopencount));
#endif #endif
handle_minor_recv(mp); handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
return 0; return 0;
} }
...@@ -1054,6 +1072,7 @@ static int capinc_tty_write(struct tty_struct * tty, ...@@ -1054,6 +1072,7 @@ static int capinc_tty_write(struct tty_struct * tty,
{ {
struct capiminor *mp = (struct capiminor *)tty->driver_data; struct capiminor *mp = (struct capiminor *)tty->driver_data;
struct sk_buff *skb; struct sk_buff *skb;
unsigned long flags;
#ifdef _DEBUG_TTYFUNCS #ifdef _DEBUG_TTYFUNCS
printk(KERN_DEBUG "capinc_tty_write(count=%d)\n", count); printk(KERN_DEBUG "capinc_tty_write(count=%d)\n", count);
...@@ -1066,6 +1085,7 @@ static int capinc_tty_write(struct tty_struct * tty, ...@@ -1066,6 +1085,7 @@ static int capinc_tty_write(struct tty_struct * tty,
return 0; return 0;
} }
spin_lock_irqsave(&workaround_lock, flags);
skb = mp->ttyskb; skb = mp->ttyskb;
if (skb) { if (skb) {
mp->ttyskb = NULL; mp->ttyskb = NULL;
...@@ -1076,6 +1096,7 @@ static int capinc_tty_write(struct tty_struct * tty, ...@@ -1076,6 +1096,7 @@ static int capinc_tty_write(struct tty_struct * tty,
skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+count, GFP_ATOMIC); skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+count, GFP_ATOMIC);
if (!skb) { if (!skb) {
printk(KERN_ERR "capinc_tty_write: alloc_skb failed\n"); printk(KERN_ERR "capinc_tty_write: alloc_skb failed\n");
spin_unlock_irqrestore(&workaround_lock, flags);
return -ENOMEM; return -ENOMEM;
} }
...@@ -1086,6 +1107,7 @@ static int capinc_tty_write(struct tty_struct * tty, ...@@ -1086,6 +1107,7 @@ static int capinc_tty_write(struct tty_struct * tty,
mp->outbytes += skb->len; mp->outbytes += skb->len;
(void)handle_minor_send(mp); (void)handle_minor_send(mp);
(void)handle_minor_recv(mp); (void)handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
return count; return count;
} }
...@@ -1093,6 +1115,7 @@ static void capinc_tty_put_char(struct tty_struct *tty, unsigned char ch) ...@@ -1093,6 +1115,7 @@ static void capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
{ {
struct capiminor *mp = (struct capiminor *)tty->driver_data; struct capiminor *mp = (struct capiminor *)tty->driver_data;
struct sk_buff *skb; struct sk_buff *skb;
unsigned long flags;
#ifdef _DEBUG_TTYFUNCS #ifdef _DEBUG_TTYFUNCS
printk(KERN_DEBUG "capinc_put_char(%u)\n", ch); printk(KERN_DEBUG "capinc_put_char(%u)\n", ch);
...@@ -1105,10 +1128,12 @@ static void capinc_tty_put_char(struct tty_struct *tty, unsigned char ch) ...@@ -1105,10 +1128,12 @@ static void capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
return; return;
} }
spin_lock_irqsave(&workaround_lock, flags);
skb = mp->ttyskb; skb = mp->ttyskb;
if (skb) { if (skb) {
if (skb_tailroom(skb) > 0) { if (skb_tailroom(skb) > 0) {
*(skb_put(skb, 1)) = ch; *(skb_put(skb, 1)) = ch;
spin_unlock_irqrestore(&workaround_lock, flags);
return; return;
} }
mp->ttyskb = NULL; mp->ttyskb = NULL;
...@@ -1124,12 +1149,14 @@ static void capinc_tty_put_char(struct tty_struct *tty, unsigned char ch) ...@@ -1124,12 +1149,14 @@ static void capinc_tty_put_char(struct tty_struct *tty, unsigned char ch)
} else { } else {
printk(KERN_ERR "capinc_put_char: char %u lost\n", ch); printk(KERN_ERR "capinc_put_char: char %u lost\n", ch);
} }
spin_unlock_irqrestore(&workaround_lock, flags);
} }
static void capinc_tty_flush_chars(struct tty_struct *tty) static void capinc_tty_flush_chars(struct tty_struct *tty)
{ {
struct capiminor *mp = (struct capiminor *)tty->driver_data; struct capiminor *mp = (struct capiminor *)tty->driver_data;
struct sk_buff *skb; struct sk_buff *skb;
unsigned long flags;
#ifdef _DEBUG_TTYFUNCS #ifdef _DEBUG_TTYFUNCS
printk(KERN_DEBUG "capinc_tty_flush_chars\n"); printk(KERN_DEBUG "capinc_tty_flush_chars\n");
...@@ -1142,6 +1169,7 @@ static void capinc_tty_flush_chars(struct tty_struct *tty) ...@@ -1142,6 +1169,7 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
return; return;
} }
spin_lock_irqsave(&workaround_lock, flags);
skb = mp->ttyskb; skb = mp->ttyskb;
if (skb) { if (skb) {
mp->ttyskb = NULL; mp->ttyskb = NULL;
...@@ -1150,6 +1178,7 @@ static void capinc_tty_flush_chars(struct tty_struct *tty) ...@@ -1150,6 +1178,7 @@ static void capinc_tty_flush_chars(struct tty_struct *tty)
(void)handle_minor_send(mp); (void)handle_minor_send(mp);
} }
(void)handle_minor_recv(mp); (void)handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
} }
static int capinc_tty_write_room(struct tty_struct *tty) static int capinc_tty_write_room(struct tty_struct *tty)
...@@ -1220,12 +1249,15 @@ static void capinc_tty_throttle(struct tty_struct * tty) ...@@ -1220,12 +1249,15 @@ static void capinc_tty_throttle(struct tty_struct * tty)
static void capinc_tty_unthrottle(struct tty_struct * tty) static void capinc_tty_unthrottle(struct tty_struct * tty)
{ {
struct capiminor *mp = (struct capiminor *)tty->driver_data; struct capiminor *mp = (struct capiminor *)tty->driver_data;
unsigned long flags;
#ifdef _DEBUG_TTYFUNCS #ifdef _DEBUG_TTYFUNCS
printk(KERN_DEBUG "capinc_tty_unthrottle\n"); printk(KERN_DEBUG "capinc_tty_unthrottle\n");
#endif #endif
if (mp) { if (mp) {
spin_lock_irqsave(&workaround_lock, flags);
mp->ttyinstop = 0; mp->ttyinstop = 0;
handle_minor_recv(mp); handle_minor_recv(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
} }
} }
...@@ -1243,12 +1275,15 @@ static void capinc_tty_stop(struct tty_struct *tty) ...@@ -1243,12 +1275,15 @@ static void capinc_tty_stop(struct tty_struct *tty)
static void capinc_tty_start(struct tty_struct *tty) static void capinc_tty_start(struct tty_struct *tty)
{ {
struct capiminor *mp = (struct capiminor *)tty->driver_data; struct capiminor *mp = (struct capiminor *)tty->driver_data;
unsigned long flags;
#ifdef _DEBUG_TTYFUNCS #ifdef _DEBUG_TTYFUNCS
printk(KERN_DEBUG "capinc_tty_start\n"); printk(KERN_DEBUG "capinc_tty_start\n");
#endif #endif
if (mp) { if (mp) {
spin_lock_irqsave(&workaround_lock, flags);
mp->ttyoutstop = 0; mp->ttyoutstop = 0;
(void)handle_minor_send(mp); (void)handle_minor_send(mp);
spin_unlock_irqrestore(&workaround_lock, flags);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment