aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Buesch <mb@bu3sch.de>2006-06-26 03:25:30 -0400
committerLinus Torvalds <torvalds@g5.osdl.org>2006-06-26 12:58:22 -0400
commit6aa65472d18703064898eefb5eb58f7ecd0d8912 (patch)
treed7085a9599febe317937dab77abddf857910f55b
parente6f47f978bcd5413fff610613b18e9e0eab9bc1b (diff)
[PATCH] CAPI crash / race condition
I am getting more or less reproducible crashes from the CAPI subsystem using the fcdsl driver: Unable to handle kernel NULL pointer dereference at virtual address 00000010 printing eip: c39bbca4 *pde = 00000000 Oops: 0000 [#1] Modules linked in: netconsole capi capifs 3c59x mii fcdsl kernelcapi uhci_hcd usbcore ide_cd cdrom CPU: 0 EIP: 0060:[<c39bbca4>] Tainted: P VLI EFLAGS: 00010202 (2.6.16.11 #3) EIP is at handle_minor_send+0x17a/0x241 [capi] eax: c24abbc0 ebx: c0b4c980 ecx: 00000010 edx: 00000010 esi: c1679140 edi: c2783016 ebp: 0000c28d esp: c0327e24 ds: 007b es: 007b ss: 0068 Process swapper (pid: 0, threadinfo=c0326000 task=c02e1300) Stack: <0>000005b4 c1679180 00000000 c28d0000 c1ce04e0 c2f69654 c221604e c1679140 c39bc19a 00000038 c20c0400 c075c560 c1f2f800 00000000 c01dc9b5 c1e96a40 c075c560 c2ed64c0 c1e96a40 c01dcd3b c2fb94e8 c075c560 c0327f00 c1e96a40 Call Trace: [<c39bc19a>] capinc_tty_write+0xda/0xf3 [capi] [<c01dc9b5>] ppp_sync_push+0x52/0xfe [<c01dcd3b>] ppp_sync_send+0x1f5/0x204 [<c01d9bc1>] ppp_push+0x3e/0x9c [<c01dacd4>] ppp_xmit_process+0x422/0x4cc [<c01daf3f>] ppp_start_xmit+0x1c1/0x1f6 [<c0213ea5>] qdisc_restart+0xa7/0x135 [<c020b112>] dev_queue_xmit+0xba/0x19e [<c0223f69>] ip_output+0x1eb/0x236 [<c0220907>] ip_forward+0x1c1/0x21a [<c021fa6c>] ip_rcv+0x38e/0x3ea [<c020b4c2>] netif_receive_skb+0x166/0x195 [<c020b55e>] process_backlog+0x6d/0xd2 [<c020a30f>] net_rx_action+0x6a/0xff [<c0112909>] __do_softirq+0x35/0x7d [<c0112973>] do_softirq+0x22/0x26 [<c0103a9d>] do_IRQ+0x1e/0x25 [<c010255a>] common_interrupt+0x1a/0x20 [<c01013c5>] default_idle+0x2b/0x53 [<c0101426>] cpu_idle+0x39/0x4e [<c0328386>] start_kernel+0x20b/0x20d Code: c0 e8 b3 b6 77 fc 85 c0 75 10 68 d8 c8 9b c3 e8 82 3d 75 fc 8b 43 60 5a eb 50 8d 56 50 c7 00 00 00 00 00 66 89 68 04 eb 02 89 ca <8b> 0a 85 c9 75 f8 89 02 89 da ff 46 54 8b 46 10 e8 30 79 fd ff <0>Kernel panic - not syncing: Fatal exception in interrupt That oops took me to the "ackqueue" implementation in capi.c. The crash occured in capincci_add_ack() (auto-inlined by the compiler). I read the code a bit and finally decided to replace the custom linked list implementation (struct capiminor->ackqueue) by a struct list_head. That did not solve the crash, but produced the following interresting oops: Unable to handle kernel paging request at virtual address 00200200 printing eip: c39bb1f5 *pde = 00000000 Oops: 0002 [#1] Modules linked in: netconsole capi capifs 3c59x mii fcdsl kernelcapi uhci_hcd usbcore ide_cd cdrom CPU: 0 EIP: 0060:[<c39bb1f5>] Tainted: P VLI EFLAGS: 00010246 (2.6.16.11 #3) EIP is at capiminor_del_ack+0x18/0x49 [capi] eax: 00200200 ebx: c18d41a0 ecx: c1385620 edx: 00100100 esi: 0000d147 edi: 00001103 ebp: 0000d147 esp: c1093f3c ds: 007b es: 007b ss: 0068 Process events/0 (pid: 3, threadinfo=c1092000 task=c1089030) Stack: <0>c2a17580 c18d41a0 c39bbd16 00000038 c18d41e0 00000000 d147c640 c29e0b68 c29e0b90 00000212 c29e0b68 c39932b2 c29e0bb0 c10736a0 c0119ef0 c399326c c10736a8 c10736a0 c10736b0 c0119f93 c011a06e 00000001 00000000 00000000 Call Trace: [<c39bbd16>] handle_minor_send+0x1af/0x241 [capi] [<c39932b2>] recv_handler+0x46/0x5f [kernelcapi] [<c0119ef0>] run_workqueue+0x5e/0x8d [<c399326c>] recv_handler+0x0/0x5f [kernelcapi] [<c0119f93>] worker_thread+0x0/0x10b [<c011a06e>] worker_thread+0xdb/0x10b [<c010c998>] default_wake_function+0x0/0xc [<c011c399>] kthread+0x90/0xbc [<c011c309>] kthread+0x0/0xbc [<c0100a65>] kernel_thread_helper+0x5/0xb Code: 7e 02 89 ee 89 f0 5a f7 d0 c1 f8 1f 5b 21 f0 5e 5f 5d c3 56 53 8b 48 50 89 d6 89 c3 8b 11 eb 2f 66 39 71 08 75 25 8b 41 04 8b 11 <89> 10 89 42 04 c7 01 00 01 10 00 89 c8 c7 41 04 00 02 20 00 e8 The interresting part of it is the "virtual address 00200200", which is LIST_POISON2. I thought about some race condition, but as this is an UP system, it leads to questions on how it can happen. If we look at EFLAGS: 00010202, we see that interrupts are enabled at the time of the crash (eflags & 0x200). Finally, I don't understand all the capi code, but I think that handle_minor_send() is racing somehow against capi_recv_message(), which call both capiminor_del_ack(). So if an IRQ occurs in the middle of capiminor_del_ack() and another instance of it is invoked, it leads to linked list corruption. I came up with the following patch. With this, I could not reproduce the crash anymore. Clearly, this is not the correct fix for the issue. As this seems to be some locking issue, there might be more locking issues in that code. For example, doesn't the whole struct capiminor have to be locked somehow? Cc: Carsten Paeth <calle@calle.de> Cc: Kai Germaschewski <kai.germaschewski@gmx.de> Cc: Karsten Keil <kkeil@suse.de> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r--drivers/isdn/capi/capi.c54
1 files changed, 32 insertions, 22 deletions
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 173c899a1fb4..2e541fa02024 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -87,6 +87,11 @@ struct capincci;
87#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE 87#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
88struct capiminor; 88struct capiminor;
89 89
90struct datahandle_queue {
91 struct list_head list;
92 u16 datahandle;
93};
94
90struct capiminor { 95struct capiminor {
91 struct list_head list; 96 struct list_head list;
92 struct capincci *nccip; 97 struct capincci *nccip;
@@ -109,12 +114,9 @@ struct capiminor {
109 int outbytes; 114 int outbytes;
110 115
111 /* transmit path */ 116 /* transmit path */
112 struct datahandle_queue { 117 struct list_head ackqueue;
113 struct datahandle_queue *next;
114 u16 datahandle;
115 } *ackqueue;
116 int nack; 118 int nack;
117 119 spinlock_t ackqlock;
118}; 120};
119#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */ 121#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
120 122
@@ -156,48 +158,54 @@ static LIST_HEAD(capiminor_list);
156 158
157static int capincci_add_ack(struct capiminor *mp, u16 datahandle) 159static int capincci_add_ack(struct capiminor *mp, u16 datahandle)
158{ 160{
159 struct datahandle_queue *n, **pp; 161 struct datahandle_queue *n;
162 unsigned long flags;
160 163
161 n = kmalloc(sizeof(*n), GFP_ATOMIC); 164 n = kmalloc(sizeof(*n), GFP_ATOMIC);
162 if (!n) { 165 if (unlikely(!n)) {
163 printk(KERN_ERR "capi: alloc datahandle failed\n"); 166 printk(KERN_ERR "capi: alloc datahandle failed\n");
164 return -1; 167 return -1;
165 } 168 }
166 n->next = NULL;
167 n->datahandle = datahandle; 169 n->datahandle = datahandle;
168 for (pp = &mp->ackqueue; *pp; pp = &(*pp)->next) ; 170 INIT_LIST_HEAD(&n->list);
169 *pp = n; 171 spin_lock_irqsave(&mp->ackqlock, flags);
172 list_add_tail(&n->list, &mp->ackqueue);
170 mp->nack++; 173 mp->nack++;
174 spin_unlock_irqrestore(&mp->ackqlock, flags);
171 return 0; 175 return 0;
172} 176}
173 177
174static int capiminor_del_ack(struct capiminor *mp, u16 datahandle) 178static int capiminor_del_ack(struct capiminor *mp, u16 datahandle)
175{ 179{
176 struct datahandle_queue **pp, *p; 180 struct datahandle_queue *p, *tmp;
181 unsigned long flags;
177 182
178 for (pp = &mp->ackqueue; *pp; pp = &(*pp)->next) { 183 spin_lock_irqsave(&mp->ackqlock, flags);
179 if ((*pp)->datahandle == datahandle) { 184 list_for_each_entry_safe(p, tmp, &mp->ackqueue, list) {
180 p = *pp; 185 if (p->datahandle == datahandle) {
181 *pp = (*pp)->next; 186 list_del(&p->list);
182 kfree(p); 187 kfree(p);
183 mp->nack--; 188 mp->nack--;
189 spin_unlock_irqrestore(&mp->ackqlock, flags);
184 return 0; 190 return 0;
185 } 191 }
186 } 192 }
193 spin_unlock_irqrestore(&mp->ackqlock, flags);
187 return -1; 194 return -1;
188} 195}
189 196
190static void capiminor_del_all_ack(struct capiminor *mp) 197static void capiminor_del_all_ack(struct capiminor *mp)
191{ 198{
192 struct datahandle_queue **pp, *p; 199 struct datahandle_queue *p, *tmp;
200 unsigned long flags;
193 201
194 pp = &mp->ackqueue; 202 spin_lock_irqsave(&mp->ackqlock, flags);
195 while (*pp) { 203 list_for_each_entry_safe(p, tmp, &mp->ackqueue, list) {
196 p = *pp; 204 list_del(&p->list);
197 *pp = (*pp)->next;
198 kfree(p); 205 kfree(p);
199 mp->nack--; 206 mp->nack--;
200 } 207 }
208 spin_unlock_irqrestore(&mp->ackqlock, flags);
201} 209}
202 210
203 211
@@ -220,6 +228,8 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
220 mp->ncci = ncci; 228 mp->ncci = ncci;
221 mp->msgid = 0; 229 mp->msgid = 0;
222 atomic_set(&mp->ttyopencount,0); 230 atomic_set(&mp->ttyopencount,0);
231 INIT_LIST_HEAD(&mp->ackqueue);
232 spin_lock_init(&mp->ackqlock);
223 233
224 skb_queue_head_init(&mp->inqueue); 234 skb_queue_head_init(&mp->inqueue);
225 skb_queue_head_init(&mp->outqueue); 235 skb_queue_head_init(&mp->outqueue);