X86 platform drivers
 help / color / mirror / Atom feed
* [PATCH 1/3] thinkpad_acpi: Flush the workqueue before freeing tpacpi_leds
@ 2012-07-25  0:45 Li Dongyang
  2012-07-25  0:45 ` [PATCH 2/3] thinkpad_acpi: Fix a memory leak during module exit Li Dongyang
  2012-07-25  0:45 ` [PATCH 3/3] thinkpad_acpi: Free hotkey_keycode_map after unregistering tpacpi_inputdev Li Dongyang
  0 siblings, 2 replies; 3+ messages in thread
From: Li Dongyang @ 2012-07-25  0:45 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: ibm-acpi, mjg

We init work_struct within tpacpi_leds, and we should free tpacpi_leds after
the workqueue is empty, in case of the work_struct is referenced after free.

This script could trigger the OOPS:

#!/bin/sh

while true
do
    modprobe -r thinkpad_acpi
    modprobe thinkpad_acpi
done

And the OOPS looks like this:

[   73.863557] BUG: unable to handle kernel paging request at 45440000
[   73.863925] IP: [<c1051d65>] process_one_work+0x25/0x3b0
[   73.864749] *pde = 00000000
[   73.865571] Oops: 0000 [#1] PREEMPT SMP
[   73.866443] Modules linked in: thinkpad_acpi(-) nvram netconsole configfs
aes_i586 cryptd aes_generic joydev btusb bluetooth arc4 snd_hda_codec_analog
iwl4965 uhci_hcd pcmcia microcode iwlegacy mac80211 cfg80211 firewire_ohci
firewire_core kvm_intel kvm snd_hda_intel acpi_cpufreq mperf ehci_hcd yenta_socket
pcmcia_rsrc crc_itu_t sr_mod snd_hda_codec processor pcmcia_core i2c_i801 usbcore
lpc_ich cdrom serio_raw psmouse coretemp rfkill e1000e snd_pcm snd_page_alloc
snd_hwdep snd_timer snd pcspkr evdev ac battery thermal soundcore usb_common
intel_agp intel_gtt tp_smapi(O) thinkpad_ec(O) ext4 crc16 jbd2 mbcache sd_mod
ata_piix ahci libahci libata scsi_mod nouveau button video mxm_wmi wmi
i2c_algo_bit drm_kms_helper ttm drm agpgart i2c_core [last unloaded: nvram]
 [   73.866676]
 [   73.866676] Pid: 62, comm: kworker/u:4 Tainted: G           O 3.5.0-1-ARCH
 #1 LENOVO 7662CTO/7662CTO
 [   73.866676] EIP: 0060:[<c1051d65>] EFLAGS: 00010002 CPU: 1
 [   73.866676] EIP is at process_one_work+0x25/0x3b0
 [   73.866676] EAX: 45440065 EBX: f5545090 ECX: 00000088 EDX: 45440000
 [   73.866676] ESI: f568ff40 EDI: c164dd40 EBP: f5705f98 ESP: f5705f68
 [   73.866676]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
 [   73.866676] CR0: 8005003b CR2: 45440000 CR3: 357ed000 CR4: 000007d0
 [   73.866676] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
 [   73.866676] DR6: ffff0ff0 DR7: 00000400
 [   73.866676] Process kworker/u:4 (pid: 62, ti=f5704000 task=f5700540 task.ti=f5704000)
 [   73.866676] Stack:
 [   73.866676]  f56fbf24 00000001 f5705f78 c10683e0 c1294950 00000000 00000000 f568ff40
 [   73.866676]  00000000 f568ff40 f568ff50 c164dd40 f5705fb8 c1052589 c1060c7e c15b9300
 [   73.866676]  c164dd40 00000000 f568ff40 c1052490 f5705fe4 c10570b2 00000000 f568ff40
 [   73.866676] Call Trace:
 [   73.866676]  [<c10683e0>] ? default_wake_function+0x10/0x20
 [   73.866676]  [<c1294950>] ? dev_get_drvdata+0x20/0x20
 [   73.866676]  [<c1052589>] worker_thread+0xf9/0x280
 [   73.866676]  [<c1060c7e>] ? complete+0x4e/0x60
 [   73.866676]  [<c1052490>] ? manage_workers.isra.24+0x1c0/0x1c0
 [   73.866676]  [<c10570b2>] kthread+0x72/0x80
 [   73.866676]  [<c1057040>] ? kthread_freezable_should_stop+0x50/0x50
 [   73.866676]  [<c13c20fe>] kernel_thread_helper+0x6/0x10
 [   73.866676] Code: bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 24 3e 8d 74 26
 00 89 c6 8b 02 89 d3 c7 45 f0 00 00 00 00 89 c2 30 d2 a8 04 0f 44 55 f0 <8b> 02 89 55 f0 89 da c1 ea
 0a 89 45 ec 89 d8 8b 4d ec c1 e8 04
 [   73.866676] EIP: [<c1051d65>] process_one_work+0x25/0x3b0 SS:ESP 0068:f5705f68
 [   73.866676] CR2: 0000000045440000
 [   73.866676] ---[ end trace 4d8a1887edca08c5 ]---
 [   73.866676] note: kworker/u:4[62] exited with preempt_count 1

Signed-off-by: Li Dongyang <Jerry87905@gmail.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index d5fd4a1..c85b666 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5217,6 +5217,7 @@ static void led_exit(void)
 			led_classdev_unregister(&tpacpi_leds[i].led_classdev);
 	}
 
+	flush_workqueue(tpacpi_wq);
 	kfree(tpacpi_leds);
 }
 
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/3] thinkpad_acpi: Fix a memory leak during module exit
  2012-07-25  0:45 [PATCH 1/3] thinkpad_acpi: Flush the workqueue before freeing tpacpi_leds Li Dongyang
@ 2012-07-25  0:45 ` Li Dongyang
  2012-07-25  0:45 ` [PATCH 3/3] thinkpad_acpi: Free hotkey_keycode_map after unregistering tpacpi_inputdev Li Dongyang
  1 sibling, 0 replies; 3+ messages in thread
From: Li Dongyang @ 2012-07-25  0:45 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: ibm-acpi, mjg

We should free the thinkpad_id.nummodel_str during exit as it's allocated
in get_thinkpad_module_data().

Signed-off-by: Li Dongyang <Jerry87905@gmail.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index c85b666..ae20355 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -8970,6 +8970,7 @@ static void thinkpad_acpi_module_exit(void)
 	kfree(thinkpad_id.bios_version_str);
 	kfree(thinkpad_id.ec_version_str);
 	kfree(thinkpad_id.model_str);
+	kfree(thinkpad_id.nummodel_str);
 }
 
 
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 3/3] thinkpad_acpi: Free hotkey_keycode_map after unregistering tpacpi_inputdev
  2012-07-25  0:45 [PATCH 1/3] thinkpad_acpi: Flush the workqueue before freeing tpacpi_leds Li Dongyang
  2012-07-25  0:45 ` [PATCH 2/3] thinkpad_acpi: Fix a memory leak during module exit Li Dongyang
@ 2012-07-25  0:45 ` Li Dongyang
  1 sibling, 0 replies; 3+ messages in thread
From: Li Dongyang @ 2012-07-25  0:45 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: ibm-acpi, mjg

We should free hotkey_keycode_map after unregistering tpacpi_inputdev, to aviod
use after free like this:

[   99.408388] =============================================================================
[   99.408393] BUG kmalloc-64 (Not tainted): Poison overwritten
[   99.408394] -----------------------------------------------------------------------------
[   99.408394] 
[   99.408398] INFO: 0xf2751962-0xf2751995. First byte 0x98 instead of 0x6b
[   99.408402] INFO: Allocated in 0xfdc88c28 age=79 cpu=0 pid=1329
[   99.408407]  __slab_alloc.isra.50.constprop.56+0x49f/0x533
[   99.408410]  kmem_cache_alloc_trace+0x10d/0x140
[   99.408412]  0xfdc88c28
[   99.408414]  0xfdc898cc
[   99.408417]  do_one_initcall+0x112/0x160
[   99.408420]  sys_init_module+0xe6d/0x1bc0
[   99.408422]  sysenter_do_call+0x12/0x28
[   99.408427] INFO: Freed in hotkey_exit+0x50/0xb0 [thinkpad_acpi] age=14 cpu=1 pid=1333
[   99.408429]  __slab_free+0x3d/0x30b
[   99.408431]  kfree+0x129/0x140
[   99.408435]  hotkey_exit+0x50/0xb0 [thinkpad_acpi]
[   99.408438]  ibm_exit+0xe3/0x1a0 [thinkpad_acpi]
[   99.408441]  thinkpad_acpi_module_exit+0x35/0x208 [thinkpad_acpi]
[   99.408443]  sys_delete_module+0x11f/0x280
[   99.408445]  sysenter_do_call+0x12/0x28
[   99.408447] INFO: Slab 0xf4d5ea20 objects=17 used=17 fp=0x  (null) flags=0x40000080
[   99.408449] INFO: Object 0xf2751960 @offset=2400 fp=0xf2751780
[   99.408449] 
[   99.408452] Bytes b4 f2751950: 64 02 00 00 ae ce fe ff 5a 5a 5a 5a 5a 5a 5a 5a  d.......ZZZZZZZZ
[   99.408454] Object f2751960: 6b 6b 98 00 ec 00 8e 00 ee 00 6b 6b e3 00 bf 00 kk........kk....
[   99.408456] Object f2751970: c2 00 6b 6b 6b 6b cd 00 6b 6b 6b 6b 6b 6b e1 00 ..kkkk..kkkkkk..
[   99.408458] Object f2751980: e0 00 e4 00 6b 6b 74 01 73 00 72 00 71 00 94 00 ....kkt.s.r.q...
[   99.408460] Object f2751990: 6b 6b 6b 6b f8 00 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkk..kkkkkkkkk.
[   99.408462] Redzone f27519a0: bb bb bb bb ....

Signed-off-by: Li Dongyang <Jerry87905@gmail.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index ae20355..e7f7328 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -3015,8 +3015,6 @@ static void hotkey_exit(void)
 	if (hotkey_dev_attributes)
 		delete_attr_set(hotkey_dev_attributes, &tpacpi_pdev->dev.kobj);
 
-	kfree(hotkey_keycode_map);
-
 	dbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
 		   "restoring original HKEY status and mask\n");
 	/* yes, there is a bitwise or below, we want the
@@ -8937,6 +8935,7 @@ static void thinkpad_acpi_module_exit(void)
 			input_unregister_device(tpacpi_inputdev);
 		else
 			input_free_device(tpacpi_inputdev);
+		kfree(hotkey_keycode_map);
 	}
 
 	if (tpacpi_hwmon)
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-07-25  0:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-25  0:45 [PATCH 1/3] thinkpad_acpi: Flush the workqueue before freeing tpacpi_leds Li Dongyang
2012-07-25  0:45 ` [PATCH 2/3] thinkpad_acpi: Fix a memory leak during module exit Li Dongyang
2012-07-25  0:45 ` [PATCH 3/3] thinkpad_acpi: Free hotkey_keycode_map after unregistering tpacpi_inputdev Li Dongyang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox