>-----Original Message----- >From: ext Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] >Sent: 16 February, 2010 23:43 >To: Onkalo Samu.P (Nokia-D/Tampere); oleg@xxxxxxxxxx >Cc: linux-input@xxxxxxxxxxxxxxx >Subject: Re: [PATCH] input: polldev can cause crash in case of polling >disabled > >On Tue, Feb 16, 2010 at 07:37:49PM +0100, samu.p.onkalo@xxxxxxxxx wrote: >> >> >> >-----Original Message----- >> >From: ext Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] >> >Sent: 16 February, 2010 19:51 >> >To: Onkalo Samu.P (Nokia-D/Tampere) >> >Cc: linux-input@xxxxxxxxxxxxxxx >> >Subject: Re: [PATCH] input: polldev can cause crash in case of >polling >> >disabled >> > >> >Hi Samu, >> > >> >On Tue, Feb 16, 2010 at 04:44:41PM +0200, Samu Onkalo wrote: >> >> If polling is set to disabled value and polled input device >> >> is opened and closed several times, address to workqueue will >probably >> >> change at some point. Since nothing is queued (due to polled >disabled >> >> state), content of the work struct contains pointer to the old and >> >non-existent >> >> workqueue. >> > >> >This I do not quite understand. The work struct as far as I can see >does >> >not reference workqueue at all. There is a list entry but if we do >not >> >poll the device that entry should be always detached from any lists. >We >> >properly initialize WQ entry when we create the device and it shoudl >> >remain valid until the device is destroyed. >> >> 'data' entry contains a pointer to per-cpu-workqueue which in turn >contains >> the workqueue pointer. This 'data' entry is not ok in case of failure. >I can >> Collect more data about this. >> >> > >> >> When the device is closed again, cancel_delayed_work_sync >> >> goes crazy due to pointer to nonexisting workqueue. >> >> >> > >> >What kind of failure do you see? Is there a stack trace or something? >> >> Kernel panic while in workqueue handling (paging fault with some >crappy address). It looks like this (in ARM): [ 88.974884] Unable to handle kernel paging request at virtual address 732e726f [ 88.994445] pgd = c0004000 [ 88.997192] [732e726f] *pgd=00000000 [ 89.013610] Internal error: Oops: 5 [#1] PREEMPT [ 89.079406] CPU: 0 Not tainted (2.6.32-09063-g2953cf4 #30) [ 89.085296] PC is at __cancel_work_timer+0xfc/0x1a0 [ 89.090179] LR is at __cancel_work_timer+0xec/0x1a0 [ 89.095092] pc : [<c006f880>] lr : [<c006f870>] psr: a0000153 [ 89.095092] sp : cf411c98 ip : cf410000 fp : cf411d1c [ 89.106628] r10: dfba59fc r9 : dfba59f0 r8 : dfba59e4 [ 89.111877] r7 : 00000000 r6 : 00000000 r5 : dfba59e0 r4 : dfba59c0 [ 89.118438] r3 : 732e726f r2 : d34a1b00 r1 : 00000001 r0 : dfba59f0 [ 89.125000] Flags: NzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment user [ 89.132263] Control: 10c5387d Table: 91110019 DAC: 00000015 [ 89.144866] Stack: (0xcf411c98 to 0xcf412000) [ 89.149261] 1c80: 00000002 00000000 [ 89.157470] 1ca0: c006f824 cf410000 00000000 d34a1b00 dfa3f540 ded44078 cf411cfc cf411cc8 [ 89.165710] 1cc0: c00861d8 c0084a20 00000002 00000000 00000000 c02b2428 cf410000 20000153 [ 89.173919] 1ce0: cf411cfc c00813a4 cf410000 20000153 cf411d44 dfba59c0 dfb618cc dfb2b864 [ 89.182159] 1d00: df67e960 ded44078 dfa3f540 ded44078 cf411d2c cf411d20 c006f938 c006f790 [ 89.190368] 1d20: cf411d44 cf411d30 bf10c2fc c006f930 dfb2b0c0 dfb618cc cf411d64 cf411d48 [ 89.198608] 1d40: c02b2458 bf10c2e8 dfb618c0 d1ca1080 dfb6193c df67e960 cf411d84 cf411d68 [ 89.206817] 1d60: c02b4ab8 c02b240c 00000000 cf408200 00000010 df67e960 cf411dc4 cf411d88 [ 89.215057] 1d80: c00ce2e0 c02b4a38 00000000 00000000 cf408200 df67e960 cf411db4 cf408200 [ 89.223266] 1da0: 00000000 d3bca000 0000000c d3bca008 00000001 00000000 cf411dd4 cf411dc8 [ 89.231506] 1dc0: c00ce438 c00ce1bc cf411df4 cf411dd8 c00caa74 c00ce414 00000041 00000000 [ 89.239715] 1de0: d3bca000 0000000c cf411e1c cf411df8 c005f8dc c00caa08 d34a1de0 d34a1b00 [ 89.247955] 1e00: d3bca000 00000002 cf411ee0 d349b684 cf411e3c cf411e20 c005f984 c005f85c [ 89.256164] 1e20: 00000002 d34a1b00 cf410000 00000002 cf411e54 cf411e40 c0060fa8 c005f950 [ 89.264404] 1e40: cf411e5c cf411e50 cf411e74 cf411e58 c00614ec c0060de8 c002e204 00000009 [ 89.272613] 1e60: be95167c cf410000 cf411ec4 cf411e78 c006c218 c0061460 00000400 dfb618f0 [ 89.280853] 1e80: dfb618f0 20000153 cf411ea4 cf411fb0 d349b184 d34d1680 cf411ebc c002e204 [ 89.289062] 1ea0: cf411fb0 be95167c 00000003 c002e204 cf410000 00000003 cf411fac cf411ec8 [ 89.297302] 1ec0: c0030430 c006be94 d1ca1080 cf410000 cf411f3c 00000001 c02b55a8 c0073430 [ 89.305511] 1ee0: 00000009 00000000 00000000 00000000 00000000 c00732ec cf411ef8 cf411ef8 [ 89.313751] 1f00: c00ccbfc c01e16e0 00000000 dfae9a10 d353cdd4 cf408200 be951644 cf411f70 [ 89.321960] 1f20: 00000400 00000400 cf410000 00000003 cf411f6c cf411f40 c00cd770 c02b54c4 [ 89.330200] 1f40: 00000000 00000000 cf410000 00000000 00000000 cf408200 00000400 be951644 [ 89.338409] 1f60: cf411fa4 cf411f70 c00cd8e0 c00cd6c4 cf411f8c cf411f80 c0082b24 c0082250 [ 89.346649] 1f80: 00000000 00012d98 00012818 be95167c 00000003 c002e204 cf410000 00000003 [ 89.354858] 1fa0: 00000000 cf411fb0 c002e098 c00303c0 fffffe00 be951644 00000400 00000001 [ 89.363098] 1fc0: 00012d98 00012818 be95167c 00000003 be951644 00000003 00000003 be951cd4 [ 89.371307] 1fe0: 00000002 be950a70 0000cfe4 400fbfcc 60000050 00000003 00000000 00000000 [ 89.379547] Backtrace: [ 89.382019] [<c006f784>] (__cancel_work_timer+0x0/0x1a0) from [<c006f938>] (cancel_delayed_work_sync+0x14/0x18) [ 89.392181] [<c006f924>] (cancel_delayed_work_sync+0x0/0x18) from [<bf10c2fc>] (input_close_polled_device+0x20/0x74 [input_p) [ 89.404174] [<bf10c2dc>] (input_close_polled_device+0x0/0x74 [input_polldev]) from [<c02b2458>] (input_close_device+0x58/0x7) [ 89.415618] r5:dfb618cc r4:dfb2b0c0 [ 89.419250] [<c02b2400>] (input_close_device+0x0/0x7c) from [<c02b4ab8>] (evdev_release+0x8c/0xa4) [ 89.428253] r7:df67e960 r6:dfb6193c r5:d1ca1080 r4:dfb618c0 [ 89.433990] [<c02b4a2c>] (evdev_release+0x0/0xa4) from [<c00ce2e0>] (__fput+0x130/0x258) [ 89.442108] r7:df67e960 r6:00000010 r5:cf408200 r4:00000000 [ 89.447845] [<c00ce1b0>] (__fput+0x0/0x258) from [<c00ce438>] (fput+0x30/0x34) [ 89.455108] [<c00ce408>] (fput+0x0/0x34) from [<c00caa74>] (filp_close+0x78/0x84) [ 89.462646] [<c00ca9fc>] (filp_close+0x0/0x84) from [<c005f8dc>] (put_files_struct+0x8c/0xf4) [ 89.471221] r7:0000000c r6:d3bca000 r5:00000000 r4:00000041 [ 89.476928] [<c005f850>] (put_files_struct+0x0/0xf4) from [<c005f984>] (exit_files+0x40/0x44) [ 89.485504] [<c005f944>] (exit_files+0x0/0x44) from [<c0060fa8>] (do_exit+0x1cc/0x678) [ 89.493469] r7:00000002 r6:cf410000 r5:d34a1b00 r4:00000002 [ 89.499206] [<c0060ddc>] (do_exit+0x0/0x678) from [<c00614ec>] (do_group_exit+0x98/0xc8) [ 89.507354] [<c0061454>] (do_group_exit+0x0/0xc8) from [<c006c218>] (get_signal_to_deliver+0x390/0x3d8) [ 89.516784] r7:cf410000 r6:be95167c r5:00000009 r4:c002e204 [ 89.522521] [<c006be88>] (get_signal_to_deliver+0x0/0x3d8) from [<c0030430>] (do_notify_resume+0x7c/0x594) [ 89.532226] [<c00303b4>] (do_notify_resume+0x0/0x594) from [<c002e098>] (work_pending+0x1c/0x20) [ 89.541076] Code: e5953000 e3d33003 0a000011 e593304c (e5934000) [ 90.065185] ---[ end trace d28022cc252c43c3 ]--- [ 90.075866] Kernel panic - not syncing: Fatal exception I added traces to drivers/input/input-polldev.c static int input_open_polled_device(struct input_dev *input) { struct input_polled_dev *dev = input_get_drvdata(input); int error; struct cpu_workqueue_struct *cwq; printk("INPUT_OPEN_POLLED_DEVICE - enter\n"); printk("polldev_wq before start_workqueue: %x\n", polldev_wq); if (polldev_wq != NULL) printk("cpu_wq of wq: %x\n", polldev_wq->cpu_wq); error = input_polldev_start_workqueue(); if (error) return error; printk("polldev_wq after start_workqueue: %x\n", polldev_wq); if (polldev_wq != NULL) printk("cpu_wq of wq: %x\n", polldev_wq->cpu_wq); if (dev->open) dev->open(dev); printk("poll_interval: %x\n", dev->poll_interval ); printk("addr of work: %x\n", &dev->work); cwq = atomic_long_read(&dev->work.work.data) & WORK_STRUCT_WQ_DATA_MASK; printk("data (==cwq) at work (before queueing): %x\n", cwq); if (cwq != NULL) printk("wq from cwq: %x\n", cwq->wq); /* Only start polling if polling is enabled */ if (dev->poll_interval > 0) queue_delayed_work(polldev_wq, &dev->work, 0); cwq = atomic_long_read(&dev->work.work.data) & WORK_STRUCT_WQ_DATA_MASK; printk("data (==cwq) at work (after queueing): %x\n", cwq); if (cwq != NULL) printk("wq from cwq: %x\n", cwq->wq); printk("INPUT_OPEN_POLLED_DEVICE - done\n"); return 0; } First time device open: [ 29.094879] INPUT_OPEN_POLLED_DEVICE - enter [ 29.099182] polldev_wq before start_workqueue: 0 [ 29.154449] polldev_wq after start_workqueue: ded3c580 [ 29.159637] cpu_wq of wq: dc4b8480 [ 29.255950] poll_interval: 32 [ 29.258941] addr of work: dfbe8f20 [ 29.262359] data (==cwq) at work (before queueing): 0 [ 29.267669] data (==cwq) at work (after queueing): dc4b8480 <------- CPU workqueue same as polldev_wq [ 29.273315] wq from cwq: ded3c580 [ 29.276641] INPUT_OPEN_POLLED_DEVICE - done there were other devices opened and closed between Second time device open: [ 202.372680] INPUT_OPEN_POLLED_DEVICE - enter [ 202.377258] polldev_wq before start_workqueue: dc57ba00 [ 202.382598] cpu_wq of wq: dc4ea900 [ 202.386077] polldev_wq after start_workqueue: dc57ba00 [ 202.391326] cpu_wq of wq: dc4ea900 [ 202.459259] poll_interval: 0 <-------------------------------- no queueing because of this [ 202.462188] addr of work: dfbe8f20 [ 202.465637] data (==cwq) at work (before queueing): dc4b8480 <----------- CPU workqueue not updated. [ 202.471435] wq from cwq: 6b6b006f [ 202.474853] data (==cwq) at work (after queueing): dc4b8480 <----- queueing not done -> not updated [ 202.480468] wq from cwq: 6b6b006f <-------------------------- crap [ 202.483886] INPUT_OPEN_POLLED_DEVICE - done And when cancel_delayed_work_sync is called, kernel crashes due to crap address to per-cpu workqueue. Actually problem is that "data" field in the work struct points to the non-existing per-cpu workqueue entry. When this is cancelled at device close, kernel crashes. But what is the root cause? In input-polldev, workqueue can change from time to time depending on what happens at the userspace. Work struct can still contain references to the old workqueue. Is that illegal thing to do? Or should cancellation of the work cleanup the work struct so that it is in initial state. It is said in the workqueue.c: * The caller must ensure that workqueue_struct on which this work was last * queued can't be destroyed before this function returns. This is not true here. Workqueue has been destroyed since the work has never queued to the new workqueue. Either cancel_(delayed)_work_sync should clear the data field instead of setting it to non-pending or input-polldev must clear the work struct in case of no queueing. Or do you have other proposals? Br, Samu -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
