* weird use-after-free bug in module_put
@ 2012-10-19 14:43 Dave Jones
2012-10-19 15:34 ` Dave Jones
0 siblings, 1 reply; 15+ messages in thread
From: Dave Jones @ 2012-10-19 14:43 UTC (permalink / raw)
To: Linux Kernel; +Cc: Linus Torvalds
I've hit this twice in the last two days while fuzz testing.
(Both times on i686 only, my x86-64 tests aren't hitting it
for some reason).
BUG: unable to handle kernel paging request at 6b6b6ce3
IP: [<c10b52fe>] module_put+0x1e/0x160
*pdpt = 0000000025a4b001 *pde = 0000000000000000
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: fuse tun binfmt_misc nfnetlink nfc caif_socket caif phonet bluetooth rfkill can llc2 pppoe pppox ppp_generic slhc irda crc_ccitt rds af_key decnet rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 lockd sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 ip6table_filter ip6_tables nf_defrag_ipv4 xt_state nf_conntrack kvm_intel kvm microcode serio_raw pcspkr i2c_i801 tg3 i2c_core shpchp raid0 ata_piix
Pid: 512, comm: acpid Not tainted 3.7.0-rc1+ #11 Dell Inc. Precision WorkStation 490 /0DT031
EIP: 0060:[<c10b52fe>] EFLAGS: 00010246 CPU: 1
EIP is at module_put+0x1e/0x160
EAX: 00000000 EBX: 6b6b6b6b ECX: 00000000 EDX: c118509c
ESI: 00000010 EDI: 6b6b6b6b EBP: e5ae9f44 ESP: e5ae9f34
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 8005003b CR2: 6b6b6ce3 CR3: 25a4a000 CR4: 000007f0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process acpid (pid: 512, ti=e5ae8000 task=e6311680 task.ti=e5ae8000)
Stack:
e6062140 6b6b6b6b 00000010 f01ce540 e5ae9f50 c118509c e6062140 e5ae9f80
c11821ed 00000001 00000000 00000000 f2073410 ef256814 ef256814 e6062148
00000000 e6311a60 e6311680 e5ae9f88 c118226d e5ae9f9c c1062f19 00000002
Call Trace:
[<c118509c>] cdev_put+0x1c/0x20
[<c11821ed>] __fput+0x20d/0x280
[<c118226d>] ____fput+0xd/0x10
[<c1062f19>] task_work_run+0x89/0xb0
[<c1002c41>] do_notify_resume+0x61/0xa0
[<c15d32f0>] work_notifysig+0x29/0x31
Code: 51 00 eb df 89 f6 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 66 66 66 66 90 85 c0 89 c7 74 44 b8 01 00 00 00 e8 c2 14 52 00 <8b> 87 78 01 00 00 64 ff 40 04 8b 45 04 89 45 f0 66 66 66 66 90
It looks like the chardev went away under our feet.
How can this happen ?
Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: weird use-after-free bug in module_put
2012-10-19 14:43 weird use-after-free bug in module_put Dave Jones
@ 2012-10-19 15:34 ` Dave Jones
2012-10-19 16:33 ` Dmitry Torokhov
0 siblings, 1 reply; 15+ messages in thread
From: Dave Jones @ 2012-10-19 15:34 UTC (permalink / raw)
To: Linux Kernel; +Cc: Linus Torvalds, dmitry.torokhov
On Fri, Oct 19, 2012 at 10:43:51AM -0400, Dave Jones wrote:
> I've hit this twice in the last two days while fuzz testing.
> (Both times on i686 only, my x86-64 tests aren't hitting it
> for some reason).
>
> BUG: unable to handle kernel paging request at 6b6b6ce3
> IP: [<c10b52fe>] module_put+0x1e/0x160
> *pdpt = 0000000025a4b001 *pde = 0000000000000000
> Oops: 0000 [#1] PREEMPT SMP
> Modules linked in: fuse tun binfmt_misc nfnetlink nfc caif_socket caif phonet bluetooth rfkill can llc2 pppoe pppox ppp_generic slhc irda crc_ccitt rds af_key decnet rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 lockd sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 ip6table_filter ip6_tables nf_defrag_ipv4 xt_state nf_conntrack kvm_intel kvm microcode serio_raw pcspkr i2c_i801 tg3 i2c_core shpchp raid0 ata_piix
> Pid: 512, comm: acpid Not tainted 3.7.0-rc1+ #11 Dell Inc. Precision WorkStation 490 /0DT031
> EIP: 0060:[<c10b52fe>] EFLAGS: 00010246 CPU: 1
> EIP is at module_put+0x1e/0x160
> EAX: 00000000 EBX: 6b6b6b6b ECX: 00000000 EDX: c118509c
> ESI: 00000010 EDI: 6b6b6b6b EBP: e5ae9f44 ESP: e5ae9f34
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> CR0: 8005003b CR2: 6b6b6ce3 CR3: 25a4a000 CR4: 000007f0
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Process acpid (pid: 512, ti=e5ae8000 task=e6311680 task.ti=e5ae8000)
> Stack:
> e6062140 6b6b6b6b 00000010 f01ce540 e5ae9f50 c118509c e6062140 e5ae9f80
> c11821ed 00000001 00000000 00000000 f2073410 ef256814 ef256814 e6062148
> 00000000 e6311a60 e6311680 e5ae9f88 c118226d e5ae9f9c c1062f19 00000002
> Call Trace:
> [<c118509c>] cdev_put+0x1c/0x20
> [<c11821ed>] __fput+0x20d/0x280
> [<c118226d>] ____fput+0xd/0x10
> [<c1062f19>] task_work_run+0x89/0xb0
> [<c1002c41>] do_notify_resume+0x61/0xa0
> [<c15d32f0>] work_notifysig+0x29/0x31
> Code: 51 00 eb df 89 f6 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 66 66 66 66 90 85 c0 89 c7 74 44 b8 01 00 00 00 e8 c2 14 52 00 <8b> 87 78 01 00 00 64 ff 40 04 8b 45 04 89 45 f0 66 66 66 66 90
>
>
> It looks like the chardev went away under our feet.
> How can this happen ?
Another clue.
I was building a kernel with PAGEALLOC_DEBUG set, but didn't reboot after the above.
During the build process, it spewed this..
BUG: scheduling while atomic: acpid/512/0x00000002
INFO: lockdep is turned off.
Modules linked in: fuse tun binfmt_misc nfnetlink nfc caif_socket caif phonet bluetooth rfkill can llc2 pppoe pppox ppp_generic slhc irda crc_ccitt rds af_key decnet rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 lockd sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 ip6table_filter ip6_tables nf_defrag_ipv4 xt_state nf_conntrack kvm_intel kvm microcode serio_raw pcspkr i2c_i801 tg3 i2c_core shpchp raid0 ata_piix
Pid: 512, comm: acpid Tainted: G D W 3.7.0-rc1+ #11
Call Trace:
[<c15c77af>] __schedule_bug+0x65/0x75
[<c15d1386>] __schedule+0x916/0x9a0
[<c106372f>] ? __kernel_text_address+0x4f/0x70
[<c1005a73>] ? print_context_stack+0x63/0xd0
[<c1004b37>] ? dump_trace+0x97/0x100
[<c15d1483>] schedule+0x23/0x60
[<c15cec95>] schedule_timeout+0x145/0x2a0
[<c12e3e1f>] ? debug_object_active_state+0x3f/0x100
[<c15d08a0>] ? wait_for_common+0x30/0x120
[<c15d08a0>] ? wait_for_common+0x30/0x120
[<c15d2ea7>] ? _raw_spin_unlock_irq+0x27/0x50
[<c10ab381>] ? trace_hardirqs_on_caller+0x11/0x170
[<c15d094a>] wait_for_common+0xda/0x120
[<c107b730>] ? try_to_wake_up+0x2b0/0x2b0
[<c10e6a80>] ? kfree_call_rcu+0x20/0x20
[<c15d0a67>] wait_for_completion+0x17/0x20
[<c10632bc>] wait_rcu_gp+0x4c/0x70
[<c10632e0>] ? wait_rcu_gp+0x70/0x70
[<c1430101>] ? serio_show_modalias+0x11/0x50
[<c143a513>] ? evdev_detach_client+0x33/0x50
[<c10e5602>] synchronize_rcu+0x32/0x90
[<c143a518>] evdev_detach_client+0x38/0x50
[<c143a575>] evdev_release+0x45/0xa0
[<c11820b8>] __fput+0xd8/0x280
[<c118226d>] ____fput+0xd/0x10
[<c1062f19>] task_work_run+0x89/0xb0
[<c1045b4d>] do_exit+0x16d/0xa90
[<c12db4be>] ? __const_udelay+0x1e/0x20
[<c10635a4>] ? __rcu_read_unlock+0x54/0xa0
[<c1042d89>] ? kmsg_dump+0x1a9/0x210
[<c1042c01>] ? kmsg_dump+0x21/0x210
[<c15d3f73>] oops_end+0x83/0xc0
[<c15c68f5>] no_context+0x1b4/0x1bc
[<c15c6a27>] __bad_area_nosemaphore+0x12a/0x132
[<c107e82e>] ? local_clock+0x4e/0x60
[<c15d61e4>] ? __do_page_fault+0x264/0x4d0
[<c15d6450>] ? __do_page_fault+0x4d0/0x4d0
[<c15d6450>] ? __do_page_fault+0x4d0/0x4d0
[<c15c6a46>] bad_area_nosemaphore+0x17/0x19
[<c15d6245>] __do_page_fault+0x2c5/0x4d0
[<c15d2f25>] ? _raw_spin_unlock_irqrestore+0x55/0x70
[<c15d6755>] ? sub_preempt_count+0x55/0xc0
[<c15d2f0b>] ? _raw_spin_unlock_irqrestore+0x3b/0x70
[<c15ca16c>] ? __slab_free+0x2b2/0x31b
[<c127672f>] ? selinux_file_free_security+0x1f/0x30
[<c15d3790>] ? error_code+0x68/0x74
[<c15d6450>] ? __do_page_fault+0x4d0/0x4d0
[<c15d6450>] ? __do_page_fault+0x4d0/0x4d0
[<c15d645d>] do_page_fault+0xd/0x10
[<c15d3794>] error_code+0x6c/0x74
[<c118509c>] ? cdev_put+0x1c/0x20
[<c10b52fe>] ? module_put+0x1e/0x160
[<c118509c>] cdev_put+0x1c/0x20
[<c11821ed>] __fput+0x20d/0x280
[<c118226d>] ____fput+0xd/0x10
[<c1062f19>] task_work_run+0x89/0xb0
[<c1002c41>] do_notify_resume+0x61/0xa0
[<c15d32f0>] work_notifysig+0x29/0x31
'evdev' caught my eye there.
shortly later...
=============================================================================
BUG kmalloc-1024 (Tainted: G D W ): Poison overwritten
-----------------------------------------------------------------------------
INFO: 0xef231630-0xef231630. First byte 0x6a instead of 0x6b
INFO: Allocated in evdev_connect+0x4d/0x210 age=54802462 cpu=3 pid=41
__slab_alloc.constprop.71+0x4aa/0x4d6
kmem_cache_alloc_trace+0x1e4/0x230
evdev_connect+0x4d/0x210
input_attach_handler+0x175/0x1c0
input_register_device+0x40b/0x460
hidinput_connect+0x153a/0x2af0
hid_connect+0x2bc/0x320
hid_device_probe+0xd5/0x110
driver_probe_device+0x7f/0x370
__device_attach+0x41/0x50
bus_for_each_drv+0x3c/0x80
device_attach+0x96/0xb0
bus_probe_device+0x77/0xa0
device_add+0x5c6/0x6a0
hid_add_device+0x1d0/0x470
usbhid_probe+0x355/0x4a0
INFO: Freed in evdev_free+0x2b/0x30 age=36397979 cpu=1 pid=512
__slab_free+0x43/0x31b
kfree+0x233/0x290
evdev_free+0x2b/0x30
device_release+0x31/0xa0
kobject_cleanup+0x78/0x1b0
kobject_put+0x25/0x60
put_device+0x14/0x20
evdev_release+0x75/0xa0
__fput+0xd8/0x280
____fput+0xd/0x10
task_work_run+0x89/0xb0
do_notify_resume+0x61/0xa0
work_notifysig+0x29/0x31
INFO: Slab 0xf6d96600 objects=27 used=27 fp=0x (null) flags=0x2804080
INFO: Object 0xef2312c0 @offset=4800 fp=0xef236720
Bytes b4 ef2312b0: c2 02 00 00 7c ea fd ff 5a 5a 5a 5a 5a 5a 5a 5a ....|...ZZZZZZZZ
Object ef2312c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2312d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2312e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2312f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231300: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231310: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231320: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231330: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231340: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231350: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231360: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231370: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231380: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231390: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2313a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2313b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2313c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2313d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2313e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2313f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231400: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231410: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231420: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231430: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231440: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231450: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231460: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231470: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231480: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231490: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2314a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2314b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2314c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2314d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2314e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2314f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231500: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231510: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231520: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231530: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231540: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231550: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231560: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231570: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231580: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231590: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2315a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2315b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2315c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2315d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2315e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2315f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231600: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231610: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231620: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231630: 6a 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b jkkkkkkkkkkkkkkk
Object ef231640: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231650: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231660: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231670: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231680: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef231690: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2316a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ef2316b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
Redzone ef2316c0: bb bb bb bb ....
Padding ef231768: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
Pid: 29790, comm: gcc Tainted: G B D W 3.7.0-rc1+ #11
Call Trace:
[<c116c4f2>] print_trailer+0xe2/0x130
[<c116c653>] check_bytes_and_report+0xc3/0x100
[<c116ce39>] check_object+0x1c9/0x210
[<c15c9e16>] alloc_debug_processing+0x57/0xfb
[<c15d6755>] ? sub_preempt_count+0x55/0xc0
[<c15ca67f>] __slab_alloc.constprop.71+0x4aa/0x4d6
[<c10cf0d8>] ? audit_alloc+0xe8/0x200
[<c10cf0d8>] ? audit_alloc+0xe8/0x200
[<c116f0c4>] kmem_cache_alloc_trace+0x1e4/0x230
[<c10cf0d8>] ? audit_alloc+0xe8/0x200
[<c10cf0d8>] audit_alloc+0xe8/0x200
[<c103d6fc>] copy_process.part.28+0x56c/0x12f0
[<c1150041>] ? handle_mm_fault+0x1d1/0x250
[<c15d6450>] ? __do_page_fault+0x4d0/0x4d0
[<c103e5a1>] do_fork+0xe1/0x470
[<c119d53a>] ? __fd_install+0x5a/0xe0
[<c15d3236>] ? restore_all+0xf/0xf
[<c100b771>] sys_vfork+0x31/0x40
[<c15d3203>] syscall_call+0x7/0xb
FIX kmalloc-1024: Restoring 0xef231630-0xef231630=0x6b
FIX kmalloc-1024: Marking all objects used
OHHHH... wait. Just before going to bed last night, I yanked out the keyboard
and plugged it into another box..
X shows..
(II) config/udev: removing device DELL DELL USB Keyboard
(II) evdev: DELL DELL USB Keyboard: Close
(II) UnloadModule: "evdev"
That explains why I haven't seen this on other machines, they're all headless
Dmitry ?
Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: weird use-after-free bug in module_put
2012-10-19 15:34 ` Dave Jones
@ 2012-10-19 16:33 ` Dmitry Torokhov
2012-10-19 17:09 ` Al Viro
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-19 16:33 UTC (permalink / raw)
To: Dave Jones, Al Viro; +Cc: Linux Kernel, Linus Torvalds
Hi Dave,
On Fri, Oct 19, 2012 at 11:34:52AM -0400, Dave Jones wrote:
> On Fri, Oct 19, 2012 at 10:43:51AM -0400, Dave Jones wrote:
> > I've hit this twice in the last two days while fuzz testing.
> > (Both times on i686 only, my x86-64 tests aren't hitting it
> > for some reason).
> >
> > BUG: unable to handle kernel paging request at 6b6b6ce3
> > IP: [<c10b52fe>] module_put+0x1e/0x160
> > *pdpt = 0000000025a4b001 *pde = 0000000000000000
> > Oops: 0000 [#1] PREEMPT SMP
> > Modules linked in: fuse tun binfmt_misc nfnetlink nfc caif_socket caif phonet bluetooth rfkill can llc2 pppoe pppox ppp_generic slhc irda crc_ccitt rds af_key decnet rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 lockd sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 ip6table_filter ip6_tables nf_defrag_ipv4 xt_state nf_conntrack kvm_intel kvm microcode serio_raw pcspkr i2c_i801 tg3 i2c_core shpchp raid0 ata_piix
> > Pid: 512, comm: acpid Not tainted 3.7.0-rc1+ #11 Dell Inc. Precision WorkStation 490 /0DT031
> > EIP: 0060:[<c10b52fe>] EFLAGS: 00010246 CPU: 1
> > EIP is at module_put+0x1e/0x160
> > EAX: 00000000 EBX: 6b6b6b6b ECX: 00000000 EDX: c118509c
> > ESI: 00000010 EDI: 6b6b6b6b EBP: e5ae9f44 ESP: e5ae9f34
> > DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> > CR0: 8005003b CR2: 6b6b6ce3 CR3: 25a4a000 CR4: 000007f0
> > DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> > DR6: ffff0ff0 DR7: 00000400
> > Process acpid (pid: 512, ti=e5ae8000 task=e6311680 task.ti=e5ae8000)
> > Stack:
> > e6062140 6b6b6b6b 00000010 f01ce540 e5ae9f50 c118509c e6062140 e5ae9f80
> > c11821ed 00000001 00000000 00000000 f2073410 ef256814 ef256814 e6062148
> > 00000000 e6311a60 e6311680 e5ae9f88 c118226d e5ae9f9c c1062f19 00000002
> > Call Trace:
> > [<c118509c>] cdev_put+0x1c/0x20
> > [<c11821ed>] __fput+0x20d/0x280
> > [<c118226d>] ____fput+0xd/0x10
> > [<c1062f19>] task_work_run+0x89/0xb0
> > [<c1002c41>] do_notify_resume+0x61/0xa0
> > [<c15d32f0>] work_notifysig+0x29/0x31
> > Code: 51 00 eb df 89 f6 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 66 66 66 66 90 85 c0 89 c7 74 44 b8 01 00 00 00 e8 c2 14 52 00 <8b> 87 78 01 00 00 64 ff 40 04 8b 45 04 89 45 f0 66 66 66 66 90
> >
> >
> > It looks like the chardev went away under our feet.
> > How can this happen ?
We are now removing instance of character device corresponding to input
device when input device disappears.
Ah, I know... cdev is embedded in evdev, but lives longer.. I do want to
keep cdev embedded as it allows me to easily get to evdev in
evdev_open(), but I need to be able to add and then drop reference to
evdev from cdev's ->release() method. This means I need to override it.
Or I could have cdev separately allocated, but then I'd like to have a
void pointer in "struct cdev" so I could get from it back to
corresponding evdev.
Al, any recommendation?
Thanks!
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: weird use-after-free bug in module_put
2012-10-19 16:33 ` Dmitry Torokhov
@ 2012-10-19 17:09 ` Al Viro
2012-10-19 17:36 ` Dmitry Torokhov
0 siblings, 1 reply; 15+ messages in thread
From: Al Viro @ 2012-10-19 17:09 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Dave Jones, Linux Kernel, Linus Torvalds
On Fri, Oct 19, 2012 at 09:33:18AM -0700, Dmitry Torokhov wrote:
> We are now removing instance of character device corresponding to input
> device when input device disappears.
>
> Ah, I know... cdev is embedded in evdev, but lives longer.. I do want to
> keep cdev embedded as it allows me to easily get to evdev in
> evdev_open(), but I need to be able to add and then drop reference to
> evdev from cdev's ->release() method. This means I need to override it.
>
> Or I could have cdev separately allocated, but then I'd like to have a
> void pointer in "struct cdev" so I could get from it back to
> corresponding evdev.
Your real problem is that you have two kobjects embedded into the same
thing. It can work, but you need to make the secondary (one that does
*not* free in its ->release()) pin the primary. Sigh... Device model
sucks, film at 11...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: weird use-after-free bug in module_put
2012-10-19 17:09 ` Al Viro
@ 2012-10-19 17:36 ` Dmitry Torokhov
2012-10-19 17:50 ` Al Viro
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-19 17:36 UTC (permalink / raw)
To: Al Viro; +Cc: Dave Jones, Linux Kernel, Linus Torvalds
On Fri, Oct 19, 2012 at 06:09:51PM +0100, Al Viro wrote:
> On Fri, Oct 19, 2012 at 09:33:18AM -0700, Dmitry Torokhov wrote:
>
> > We are now removing instance of character device corresponding to input
> > device when input device disappears.
> >
> > Ah, I know... cdev is embedded in evdev, but lives longer.. I do want to
> > keep cdev embedded as it allows me to easily get to evdev in
> > evdev_open(), but I need to be able to add and then drop reference to
> > evdev from cdev's ->release() method. This means I need to override it.
> >
> > Or I could have cdev separately allocated, but then I'd like to have a
> > void pointer in "struct cdev" so I could get from it back to
> > corresponding evdev.
>
> Your real problem is that you have two kobjects embedded into the same
> thing. It can work, but you need to make the secondary (one that does
> *not* free in its ->release()) pin the primary. Sigh... Device model
> sucks, film at 11...
Right, but "cdev" is currently "sealed": it does not allow specifying a
custom release function from which I could unpin primary (evdev). You
are the author/owner of cdev code, so that is why I was asking for
your opinion as to what is the best way to proceed:
1. Allocate cdev separately and add void * to struct cdev so that it is
easy to get to corresponding structure on evdev_open.
2. Keep cdev embedded in evdev but export cdev's cleanup method and
have evdev override ->release with its own version that calls
cdev_default_release() and then unpins evdev stucture.
3. Add struct device *parent to struct cdev and have it pin and unpin it
for us (if it is set up).
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: weird use-after-free bug in module_put
2012-10-19 17:36 ` Dmitry Torokhov
@ 2012-10-19 17:50 ` Al Viro
2012-10-19 18:12 ` Dmitry Torokhov
2012-10-21 7:24 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Dmitry Torokhov
0 siblings, 2 replies; 15+ messages in thread
From: Al Viro @ 2012-10-19 17:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Dave Jones, Linux Kernel, Linus Torvalds
On Fri, Oct 19, 2012 at 10:36:39AM -0700, Dmitry Torokhov wrote:
> On Fri, Oct 19, 2012 at 06:09:51PM +0100, Al Viro wrote:
> > On Fri, Oct 19, 2012 at 09:33:18AM -0700, Dmitry Torokhov wrote:
> >
> > > We are now removing instance of character device corresponding to input
> > > device when input device disappears.
> > >
> > > Ah, I know... cdev is embedded in evdev, but lives longer.. I do want to
> > > keep cdev embedded as it allows me to easily get to evdev in
> > > evdev_open(), but I need to be able to add and then drop reference to
> > > evdev from cdev's ->release() method. This means I need to override it.
> > >
> > > Or I could have cdev separately allocated, but then I'd like to have a
> > > void pointer in "struct cdev" so I could get from it back to
> > > corresponding evdev.
> >
> > Your real problem is that you have two kobjects embedded into the same
> > thing. It can work, but you need to make the secondary (one that does
> > *not* free in its ->release()) pin the primary. Sigh... Device model
> > sucks, film at 11...
>
> Right, but "cdev" is currently "sealed": it does not allow specifying a
> custom release function from which I could unpin primary (evdev). You
> are the author/owner of cdev code, so that is why I was asking for
> your opinion as to what is the best way to proceed:
>
> 1. Allocate cdev separately and add void * to struct cdev so that it is
> easy to get to corresponding structure on evdev_open.
>
> 2. Keep cdev embedded in evdev but export cdev's cleanup method and
> have evdev override ->release with its own version that calls
> cdev_default_release() and then unpins evdev stucture.
>
> 3. Add struct device *parent to struct cdev and have it pin and unpin it
> for us (if it is set up).
The last one would be my preference, TBH, but I'm not sure how to do it
cleanly. The thing is, kobject_del() would have to be done at some point.
_Before_ we have dropped the last references. And once we have, we are
back to the original race, AFAICS.
What's pinning that cdev reference, BTW?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: weird use-after-free bug in module_put
2012-10-19 17:50 ` Al Viro
@ 2012-10-19 18:12 ` Dmitry Torokhov
2012-10-21 7:24 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Dmitry Torokhov
1 sibling, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-19 18:12 UTC (permalink / raw)
To: Al Viro; +Cc: Dave Jones, Linux Kernel, Linus Torvalds
On Fri, Oct 19, 2012 at 06:50:46PM +0100, Al Viro wrote:
> On Fri, Oct 19, 2012 at 10:36:39AM -0700, Dmitry Torokhov wrote:
> > On Fri, Oct 19, 2012 at 06:09:51PM +0100, Al Viro wrote:
> > > On Fri, Oct 19, 2012 at 09:33:18AM -0700, Dmitry Torokhov wrote:
> > >
> > > > We are now removing instance of character device corresponding to input
> > > > device when input device disappears.
> > > >
> > > > Ah, I know... cdev is embedded in evdev, but lives longer.. I do want to
> > > > keep cdev embedded as it allows me to easily get to evdev in
> > > > evdev_open(), but I need to be able to add and then drop reference to
> > > > evdev from cdev's ->release() method. This means I need to override it.
> > > >
> > > > Or I could have cdev separately allocated, but then I'd like to have a
> > > > void pointer in "struct cdev" so I could get from it back to
> > > > corresponding evdev.
> > >
> > > Your real problem is that you have two kobjects embedded into the same
> > > thing. It can work, but you need to make the secondary (one that does
> > > *not* free in its ->release()) pin the primary. Sigh... Device model
> > > sucks, film at 11...
> >
> > Right, but "cdev" is currently "sealed": it does not allow specifying a
> > custom release function from which I could unpin primary (evdev). You
> > are the author/owner of cdev code, so that is why I was asking for
> > your opinion as to what is the best way to proceed:
> >
> > 1. Allocate cdev separately and add void * to struct cdev so that it is
> > easy to get to corresponding structure on evdev_open.
> >
> > 2. Keep cdev embedded in evdev but export cdev's cleanup method and
> > have evdev override ->release with its own version that calls
> > cdev_default_release() and then unpins evdev stucture.
> >
> > 3. Add struct device *parent to struct cdev and have it pin and unpin it
> > for us (if it is set up).
>
> The last one would be my preference, TBH, but I'm not sure how to do it
> cleanly. The thing is, kobject_del() would have to be done at some point.
> _Before_ we have dropped the last references. And once we have, we are
> back to the original race, AFAICS.
If we unpin the parent as the very last thing in cdev_default_release()
and do not touch any memory [formerly] occupied by cdev afterwards I
think it should be OK.
>
> What's pinning that cdev reference, BTW?
Userspace threads that have that character device opened. They are
dropping off and releasing evdev's references, but cdev is unpinned in
vfs core, which is too late as it happens after evdev_release()
competed.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] char_dev: allow setting up and pinning parent devices
2012-10-19 17:50 ` Al Viro
2012-10-19 18:12 ` Dmitry Torokhov
@ 2012-10-21 7:24 ` Dmitry Torokhov
2012-10-21 7:24 ` [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes Dmitry Torokhov
2012-10-21 7:39 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Al Viro
1 sibling, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-21 7:24 UTC (permalink / raw)
To: Al Viro, Dave Jones; +Cc: Linus Torvalds, linux-kernel, linux-input
In certain cases (for example when a cdev structure is embedded into
another object whose lifetime is controlled by a separate device object)
it is beneficial to tie lifetime of another struct device to the lifetime
of character device so that related object is not freed until after
char_dev object is freed. To achieve this allow setting a "parent" device
for character devices and pin them when doing cdev_add() and unpin when
last reference to cdev structure is being released.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Sorry, messed up the first attempt to send...
So, how about these 2? I enabled kmemleak and added some debug printks and
it looks like we dropping references and freeing object at right times and
in proper order.
Thanks!
fs/char_dev.c | 18 +++++++++++++++++-
include/linux/cdev.h | 1 +
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/fs/char_dev.c b/fs/char_dev.c
index 3f152b9..f8c44cc 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -471,9 +471,19 @@ static int exact_lock(dev_t dev, void *data)
*/
int cdev_add(struct cdev *p, dev_t dev, unsigned count)
{
+ int error;
+
p->dev = dev;
p->count = count;
- return kobj_map(cdev_map, dev, count, NULL, exact_match, exact_lock, p);
+
+ error = kobj_map(cdev_map, dev, count, NULL,
+ exact_match, exact_lock, p);
+ if (error)
+ return error;
+
+ get_device(p->parent);
+
+ return 0;
}
static void cdev_unmap(dev_t dev, unsigned count)
@@ -498,14 +508,20 @@ void cdev_del(struct cdev *p)
static void cdev_default_release(struct kobject *kobj)
{
struct cdev *p = container_of(kobj, struct cdev, kobj);
+ struct device *parent = p->parent;
+
cdev_purge(p);
+ put_device(parent);
}
static void cdev_dynamic_release(struct kobject *kobj)
{
struct cdev *p = container_of(kobj, struct cdev, kobj);
+ struct device *parent = p->parent;
+
cdev_purge(p);
kfree(p);
+ put_device(parent);
}
static struct kobj_type ktype_cdev_default = {
diff --git a/include/linux/cdev.h b/include/linux/cdev.h
index fb45919..eba8251 100644
--- a/include/linux/cdev.h
+++ b/include/linux/cdev.h
@@ -12,6 +12,7 @@ struct module;
struct cdev {
struct kobject kobj;
struct module *owner;
+ struct device *parent;
const struct file_operations *ops;
struct list_head list;
dev_t dev;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes
2012-10-21 7:24 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Dmitry Torokhov
@ 2012-10-21 7:24 ` Dmitry Torokhov
2012-10-21 7:39 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Al Viro
1 sibling, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-21 7:24 UTC (permalink / raw)
To: Al Viro, Dave Jones; +Cc: Linus Torvalds, linux-kernel, linux-input
Commit 7f8d4cad1e4e11a45d02bd6e024cc2812963c38a made evdev, joydev and
mousedev to embed struct cdev into their respective structures representing
input devices. Unfortunately character device structure may outlive the
parent structure unless we do not set it up as parent of character device
so that it will stay pinned until character device is freed.
Also, now that parent structure is pinned while character device exists
we do not need to pin and unpin it every time user opens or closes it.
Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/evdev.c | 3 +--
drivers/input/joydev.c | 3 +--
drivers/input/mousedev.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 6ae2ac4..49d9e34 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -292,7 +292,6 @@ static int evdev_release(struct inode *inode, struct file *file)
kfree(client);
evdev_close_device(evdev);
- put_device(&evdev->dev);
return 0;
}
@@ -331,7 +330,6 @@ static int evdev_open(struct inode *inode, struct file *file)
file->private_data = client;
nonseekable_open(inode, file);
- get_device(&evdev->dev);
return 0;
err_free_client:
@@ -1001,6 +999,7 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
goto err_free_evdev;
cdev_init(&evdev->cdev, &evdev_fops);
+ evdev->cdev.parent = &evdev->dev;
error = cdev_add(&evdev->cdev, evdev->dev.devt, 1);
if (error)
goto err_unregister_handle;
diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 63e5916..00c3537 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -262,7 +262,6 @@ static int joydev_release(struct inode *inode, struct file *file)
kfree(client);
joydev_close_device(joydev);
- put_device(&joydev->dev);
return 0;
}
@@ -289,7 +288,6 @@ static int joydev_open(struct inode *inode, struct file *file)
file->private_data = client;
nonseekable_open(inode, file);
- get_device(&joydev->dev);
return 0;
err_free_client:
@@ -877,6 +875,7 @@ static int joydev_connect(struct input_handler *handler, struct input_dev *dev,
goto err_free_joydev;
cdev_init(&joydev->cdev, &joydev_fops);
+ joydev->cdev.parent = &joydev->dev;
error = cdev_add(&joydev->cdev, joydev->dev.devt, 1);
if (error)
goto err_unregister_handle;
diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index a1b4c37..b65d476 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -523,7 +523,6 @@ static int mousedev_release(struct inode *inode, struct file *file)
kfree(client);
mousedev_close_device(mousedev);
- put_device(&mousedev->dev);
return 0;
}
@@ -558,7 +557,6 @@ static int mousedev_open(struct inode *inode, struct file *file)
file->private_data = client;
nonseekable_open(inode, file);
- get_device(&mousedev->dev);
return 0;
err_free_client:
@@ -892,6 +890,7 @@ static struct mousedev *mousedev_create(struct input_dev *dev,
}
cdev_init(&mousedev->cdev, &mousedev_fops);
+ mousedev->cdev.parent = &mousedev->dev;
error = cdev_add(&mousedev->cdev, mousedev->dev.devt, 1);
if (error)
goto err_unregister_handle;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] char_dev: allow setting up and pinning parent devices
2012-10-21 7:24 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Dmitry Torokhov
2012-10-21 7:24 ` [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes Dmitry Torokhov
@ 2012-10-21 7:39 ` Al Viro
2012-10-21 8:13 ` Dmitry Torokhov
2012-10-22 0:57 ` [PATCH 1/2] char_dev: pin parent kobject Dmitry Torokhov
1 sibling, 2 replies; 15+ messages in thread
From: Al Viro @ 2012-10-21 7:39 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Dave Jones, Linus Torvalds, linux-kernel, linux-input
On Sun, Oct 21, 2012 at 12:24:30AM -0700, Dmitry Torokhov wrote:
> In certain cases (for example when a cdev structure is embedded into
> another object whose lifetime is controlled by a separate device object)
> it is beneficial to tie lifetime of another struct device to the lifetime
> of character device so that related object is not freed until after
> char_dev object is freed. To achieve this allow setting a "parent" device
> for character devices and pin them when doing cdev_add() and unpin when
> last reference to cdev structure is being released.
Why struct device and not simply struct kobject?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] char_dev: allow setting up and pinning parent devices
2012-10-21 7:39 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Al Viro
@ 2012-10-21 8:13 ` Dmitry Torokhov
2012-10-22 0:57 ` [PATCH 1/2] char_dev: pin parent kobject Dmitry Torokhov
1 sibling, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-21 8:13 UTC (permalink / raw)
To: Al Viro; +Cc: Dave Jones, Linus Torvalds, linux-kernel, linux-input
On Sun, Oct 21, 2012 at 08:39:28AM +0100, Al Viro wrote:
> On Sun, Oct 21, 2012 at 12:24:30AM -0700, Dmitry Torokhov wrote:
> > In certain cases (for example when a cdev structure is embedded into
> > another object whose lifetime is controlled by a separate device object)
> > it is beneficial to tie lifetime of another struct device to the lifetime
> > of character device so that related object is not freed until after
> > char_dev object is freed. To achieve this allow setting a "parent" device
> > for character devices and pin them when doing cdev_add() and unpin when
> > last reference to cdev structure is being released.
>
> Why struct device and not simply struct kobject?
It was more convenient for my uses and I also think that parents of
character devices will be struct devices... I however will not insist
and if you prefer using more generic kobject I can change it,
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] char_dev: pin parent kobject
2012-10-21 7:39 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Al Viro
2012-10-21 8:13 ` Dmitry Torokhov
@ 2012-10-22 0:57 ` Dmitry Torokhov
2012-10-22 0:57 ` [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes Dmitry Torokhov
2012-10-22 5:02 ` [PATCH 1/2] char_dev: pin parent kobject Linus Torvalds
1 sibling, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-22 0:57 UTC (permalink / raw)
To: Al Viro, Dave Jones; +Cc: Linus Torvalds, linux-kernel, linux-input
In certain cases (for example when a cdev structure is embedded into
another object whose lifetime is controlled by a separate kobject) it is
beneficial to tie lifetime of another object to the lifetime of character
device so that related object is not freed until after char_dev object is
freed. To achieve this let's pin kobject's parent when doing cdev_add()
and unpin when last reference to cdev structure is being released.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
OK, so it looks like cdev kobject's parent is unused so we can just have
users set themselves as parents of character device and have cdev_add()
pin it.
fs/char_dev.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/fs/char_dev.c b/fs/char_dev.c
index 3f152b9..afc2bb6 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -471,9 +471,19 @@ static int exact_lock(dev_t dev, void *data)
*/
int cdev_add(struct cdev *p, dev_t dev, unsigned count)
{
+ int error;
+
p->dev = dev;
p->count = count;
- return kobj_map(cdev_map, dev, count, NULL, exact_match, exact_lock, p);
+
+ error = kobj_map(cdev_map, dev, count, NULL,
+ exact_match, exact_lock, p);
+ if (error)
+ return error;
+
+ kobject_get(p->kobj.parent);
+
+ return 0;
}
static void cdev_unmap(dev_t dev, unsigned count)
@@ -498,14 +508,20 @@ void cdev_del(struct cdev *p)
static void cdev_default_release(struct kobject *kobj)
{
struct cdev *p = container_of(kobj, struct cdev, kobj);
+ struct kobject *parent = kobj->parent;
+
cdev_purge(p);
+ kobject_put(parent);
}
static void cdev_dynamic_release(struct kobject *kobj)
{
struct cdev *p = container_of(kobj, struct cdev, kobj);
+ struct kobject *parent = kobj->parent;
+
cdev_purge(p);
kfree(p);
+ kobject_put(parent);
}
static struct kobj_type ktype_cdev_default = {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes
2012-10-22 0:57 ` [PATCH 1/2] char_dev: pin parent kobject Dmitry Torokhov
@ 2012-10-22 0:57 ` Dmitry Torokhov
2012-10-22 5:02 ` [PATCH 1/2] char_dev: pin parent kobject Linus Torvalds
1 sibling, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2012-10-22 0:57 UTC (permalink / raw)
To: Al Viro, Dave Jones; +Cc: Linus Torvalds, linux-kernel, linux-input
Commit 7f8d4cad1e4e11a45d02bd6e024cc2812963c38a made evdev, joydev and
mousedev to embed struct cdev into their respective structures representing
input devices. Unfortunately character device structure may outlive the
parent structure unless we do not set it up as parent of character device
so that it will stay pinned until character device is freed.
Also, now that parent structure is pinned while character device exists
we do not need to pin and unpin it every time user opens or closes it.
Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/evdev.c | 3 +--
drivers/input/joydev.c | 3 +--
drivers/input/mousedev.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 6ae2ac4..f0f8928 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -292,7 +292,6 @@ static int evdev_release(struct inode *inode, struct file *file)
kfree(client);
evdev_close_device(evdev);
- put_device(&evdev->dev);
return 0;
}
@@ -331,7 +330,6 @@ static int evdev_open(struct inode *inode, struct file *file)
file->private_data = client;
nonseekable_open(inode, file);
- get_device(&evdev->dev);
return 0;
err_free_client:
@@ -1001,6 +999,7 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
goto err_free_evdev;
cdev_init(&evdev->cdev, &evdev_fops);
+ evdev->cdev.kobj.parent = &evdev->dev.kobj;
error = cdev_add(&evdev->cdev, evdev->dev.devt, 1);
if (error)
goto err_unregister_handle;
diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 63e5916..9c7526d 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -262,7 +262,6 @@ static int joydev_release(struct inode *inode, struct file *file)
kfree(client);
joydev_close_device(joydev);
- put_device(&joydev->dev);
return 0;
}
@@ -289,7 +288,6 @@ static int joydev_open(struct inode *inode, struct file *file)
file->private_data = client;
nonseekable_open(inode, file);
- get_device(&joydev->dev);
return 0;
err_free_client:
@@ -877,6 +875,7 @@ static int joydev_connect(struct input_handler *handler, struct input_dev *dev,
goto err_free_joydev;
cdev_init(&joydev->cdev, &joydev_fops);
+ joydev->cdev.kobj.parent = &joydev->dev.kobj;
error = cdev_add(&joydev->cdev, joydev->dev.devt, 1);
if (error)
goto err_unregister_handle;
diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index a1b4c37..8f02e3d 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -523,7 +523,6 @@ static int mousedev_release(struct inode *inode, struct file *file)
kfree(client);
mousedev_close_device(mousedev);
- put_device(&mousedev->dev);
return 0;
}
@@ -558,7 +557,6 @@ static int mousedev_open(struct inode *inode, struct file *file)
file->private_data = client;
nonseekable_open(inode, file);
- get_device(&mousedev->dev);
return 0;
err_free_client:
@@ -892,6 +890,7 @@ static struct mousedev *mousedev_create(struct input_dev *dev,
}
cdev_init(&mousedev->cdev, &mousedev_fops);
+ mousedev->cdev.kobj.parent = &mousedev->dev.kobj;
error = cdev_add(&mousedev->cdev, mousedev->dev.devt, 1);
if (error)
goto err_unregister_handle;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] char_dev: pin parent kobject
2012-10-22 0:57 ` [PATCH 1/2] char_dev: pin parent kobject Dmitry Torokhov
2012-10-22 0:57 ` [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes Dmitry Torokhov
@ 2012-10-22 5:02 ` Linus Torvalds
2012-10-22 5:42 ` Al Viro
1 sibling, 1 reply; 15+ messages in thread
From: Linus Torvalds @ 2012-10-22 5:02 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Al Viro, Dave Jones, linux-kernel, linux-input
This series looks good to me, and looks to fix the issue about as
cleanly as it can. Al, holler if you have any concerns, but I'm
planning on applying it ASAP,
Linus
On Mon, Oct 22, 2012 at 3:57 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> In certain cases (for example when a cdev structure is embedded into
> another object whose lifetime is controlled by a separate kobject) it is
> beneficial to tie lifetime of another object to the lifetime of character
> device so that related object is not freed until after char_dev object is
> freed. To achieve this let's pin kobject's parent when doing cdev_add()
> and unpin when last reference to cdev structure is being released.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] char_dev: pin parent kobject
2012-10-22 5:02 ` [PATCH 1/2] char_dev: pin parent kobject Linus Torvalds
@ 2012-10-22 5:42 ` Al Viro
0 siblings, 0 replies; 15+ messages in thread
From: Al Viro @ 2012-10-22 5:42 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Dmitry Torokhov, Dave Jones, linux-kernel, linux-input
On Mon, Oct 22, 2012 at 08:02:12AM +0300, Linus Torvalds wrote:
> This series looks good to me, and looks to fix the issue about as
> cleanly as it can. Al, holler if you have any concerns, but I'm
> planning on applying it ASAP,
Add my ACKed-by. And from the look of it, we'll need something similar
at least in drivers/mtd/ubi in addition to evdev patch.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-10-22 5:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-19 14:43 weird use-after-free bug in module_put Dave Jones
2012-10-19 15:34 ` Dave Jones
2012-10-19 16:33 ` Dmitry Torokhov
2012-10-19 17:09 ` Al Viro
2012-10-19 17:36 ` Dmitry Torokhov
2012-10-19 17:50 ` Al Viro
2012-10-19 18:12 ` Dmitry Torokhov
2012-10-21 7:24 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Dmitry Torokhov
2012-10-21 7:24 ` [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes Dmitry Torokhov
2012-10-21 7:39 ` [PATCH 1/2] char_dev: allow setting up and pinning parent devices Al Viro
2012-10-21 8:13 ` Dmitry Torokhov
2012-10-22 0:57 ` [PATCH 1/2] char_dev: pin parent kobject Dmitry Torokhov
2012-10-22 0:57 ` [PATCH 2/2] Input: fix use-after-free introduced with dynamic minor changes Dmitry Torokhov
2012-10-22 5:02 ` [PATCH 1/2] char_dev: pin parent kobject Linus Torvalds
2012-10-22 5:42 ` Al Viro
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.