* [PATCH v3 1/2] power_supply: Fix Oops from NULL pointer dereference from wakeup_source_activate [not found] <cover.1385138418.git.shuah.kh@samsung.com> @ 2013-11-22 17:54 ` Shuah Khan 2013-12-01 21:50 ` Anton Vorontsov 2013-11-22 17:54 ` [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str Shuah Khan 1 sibling, 1 reply; 6+ messages in thread From: Shuah Khan @ 2013-11-22 17:54 UTC (permalink / raw) To: anton, dwmw2, rostedt, fweisbec, mingo, rjw, gregkh Cc: Shuah Khan, linux-kernel, shuahkhan, stable power_supply_register() calls device_init_wakeup() to register a wakeup source before initializing dev_name. As a result, device_wakeup_enable() end up registering wakeup source with a null name when wakeup_source_register() gets called with dev_name(dev) which is null at the time. When kernel is booted with wakeup_source_activate enabled, it will panic when the trace point code tries to dereference ws->name. Fixed the problem by moving up the kobject_set_name() call prior to accesses to dev_name(). Replaced kobject_set_name() with dev_set_name() which is the right interface to be called from drivers. Fixed the call to device_del() prior to device_add() in for wakeup_init_failed error handling code. Trace after the change: bash-2143 [003] d... 132.280697: wakeup_source_activate: BAT1 state=0x20001 kworker/3:2-1169 [003] d... 132.281305: wakeup_source_deactivate: BAT1 state=0x30000 Oops message: [ 819.769934] device: 'BAT1': device_add [ 819.770078] PM: Adding info for No Bus:BAT1 [ 819.770235] BUG: unable to handle kernel NULL pointer dereference at (null) [ 819.770435] IP: [<ffffffff813381c0>] skip_spaces+0x30/0x30 [ 819.770572] PGD 3efd90067 PUD 3eff61067 PMD 0 [ 819.770716] Oops: 0000 [#1] SMP [ 819.770829] Modules linked in: arc4 iwldvm mac80211 x86_pkg_temp_thermal coretemp kvm_intel joydev i915 kvm uvcvideo ghash_clmulni_intel videobuf2_vmalloc aesni_intel videobuf2_memops videobuf2_core aes_x86_64 ablk_helper cryptd videodev iwlwifi lrw rfcomm gf128mul glue_helper bnep btusb media bluetooth parport_pc hid_generic ppdev snd_hda_codec_hdmi drm_kms_helper snd_hda_codec_realtek cfg80211 drm tpm_infineon samsung_laptop snd_hda_intel usbhid snd_hda_codec hid snd_hwdep snd_pcm microcode snd_page_alloc snd_timer psmouse i2c_algo_bit lpc_ich tpm_tis video wmi mac_hid serio_raw ext2 lp parport r8169 mii [ 819.771802] CPU: 0 PID: 2167 Comm: bash Not tainted 3.12.0+ #25 [ 819.771876] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X4C/900X4D/SAMSUNG_NP1234567890, BIOS P03AAC 07/12/2012 [ 819.772022] task: ffff88002e6ddcc0 ti: ffff8804015ca000 task.ti: ffff8804015ca000 [ 819.772119] RIP: 0010:[<ffffffff813381c0>] [<ffffffff813381c0>] skip_spaces+0x30/0x30 [ 819.772242] RSP: 0018:ffff8804015cbc70 EFLAGS: 00010046 [ 819.772310] RAX: 0000000000000003 RBX: ffff88040cfd6d40 RCX: 0000000000000018 [ 819.772397] RDX: 0000000000020001 RSI: 0000000000000000 RDI: 0000000000000000 [ 819.772484] RBP: ffff8804015cbcc0 R08: 0000000000000000 R09: ffff8803f0768d40 [ 819.772570] R10: ffffea001033b800 R11: 0000000000000000 R12: ffffffff81c519c0 [ 819.772656] R13: 0000000000020001 R14: 0000000000000000 R15: 0000000000020001 [ 819.772744] FS: 00007ff98309b740(0000) GS:ffff88041f200000(0000) knlGS:0000000000000000 [ 819.772845] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 819.772917] CR2: 0000000000000000 CR3: 00000003f59dc000 CR4: 00000000001407f0 [ 819.773001] Stack: [ 819.773030] ffffffff81114003 ffff8804015cbcb0 0000000000000000 0000000000000046 [ 819.773146] ffff880409757a18 ffff8803f065a160 0000000000000000 0000000000020001 [ 819.773273] 0000000000000000 0000000000000000 ffff8804015cbce8 ffffffff8143e388 [ 819.773387] Call Trace: [ 819.773434] [<ffffffff81114003>] ? ftrace_raw_event_wakeup_source+0x43/0xe0 [ 819.773520] [<ffffffff8143e388>] wakeup_source_report_event+0xb8/0xd0 [ 819.773595] [<ffffffff8143e3cd>] __pm_stay_awake+0x2d/0x50 [ 819.773724] [<ffffffff8153395c>] power_supply_changed+0x3c/0x90 [ 819.773795] [<ffffffff8153407c>] power_supply_register+0x18c/0x250 [ 819.773869] [<ffffffff813d8d18>] sysfs_add_battery+0x61/0x7b [ 819.773935] [<ffffffff813d8d69>] battery_notify+0x37/0x3f [ 819.774001] [<ffffffff816ccb7c>] notifier_call_chain+0x4c/0x70 [ 819.774071] [<ffffffff81073ded>] __blocking_notifier_call_chain+0x4d/0x70 [ 819.774149] [<ffffffff81073e26>] blocking_notifier_call_chain+0x16/0x20 [ 819.774227] [<ffffffff8109397a>] pm_notifier_call_chain+0x1a/0x40 [ 819.774316] [<ffffffff81095b66>] hibernate+0x66/0x1c0 [ 819.774407] [<ffffffff81093931>] state_store+0x71/0xa0 [ 819.774507] [<ffffffff81331d8f>] kobj_attr_store+0xf/0x20 [ 819.774613] [<ffffffff811f8618>] sysfs_write_file+0x128/0x1c0 [ 819.774735] [<ffffffff8118579d>] vfs_write+0xbd/0x1e0 [ 819.774841] [<ffffffff811861d9>] SyS_write+0x49/0xa0 [ 819.774939] [<ffffffff816d1052>] system_call_fastpath+0x16/0x1b [ 819.775055] Code: 89 f8 48 89 e5 f6 82 c0 a6 84 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 c0 a6 84 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 819.775760] RIP [<ffffffff813381c0>] skip_spaces+0x30/0x30 [ 819.775881] RSP <ffff8804015cbc70> [ 819.775949] CR2: 0000000000000000 [ 819.794175] ---[ end trace c4ef25127039952e ]--- Signed-off-by: Shuah Khan <shuah.kh@samsung.com> Acked-by: Anton Vorontsov <anton@enomsg.org> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: stable@vger.kernel.org --- drivers/power/power_supply_core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c index 00e6672..557af94 100644 --- a/drivers/power/power_supply_core.c +++ b/drivers/power/power_supply_core.c @@ -511,6 +511,10 @@ int power_supply_register(struct device *parent, struct power_supply *psy) dev_set_drvdata(dev, psy); psy->dev = dev; + rc = dev_set_name(dev, "%s", psy->name); + if (rc) + goto dev_set_name_failed; + INIT_WORK(&psy->changed_work, power_supply_changed_work); rc = power_supply_check_supplies(psy); @@ -524,10 +528,6 @@ int power_supply_register(struct device *parent, struct power_supply *psy) if (rc) goto wakeup_init_failed; - rc = kobject_set_name(&dev->kobj, "%s", psy->name); - if (rc) - goto kobject_set_name_failed; - rc = device_add(dev); if (rc) goto device_add_failed; @@ -553,11 +553,11 @@ create_triggers_failed: register_cooler_failed: psy_unregister_thermal(psy); register_thermal_failed: -wakeup_init_failed: device_del(dev); -kobject_set_name_failed: device_add_failed: +wakeup_init_failed: check_supplies_failed: +dev_set_name_failed: put_device(dev); success: return rc; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] power_supply: Fix Oops from NULL pointer dereference from wakeup_source_activate 2013-11-22 17:54 ` [PATCH v3 1/2] power_supply: Fix Oops from NULL pointer dereference from wakeup_source_activate Shuah Khan @ 2013-12-01 21:50 ` Anton Vorontsov 0 siblings, 0 replies; 6+ messages in thread From: Anton Vorontsov @ 2013-12-01 21:50 UTC (permalink / raw) To: Shuah Khan Cc: dwmw2, rostedt, fweisbec, mingo, rjw, gregkh, linux-kernel, shuahkhan, stable On Fri, Nov 22, 2013 at 10:54:28AM -0700, Shuah Khan wrote: > power_supply_register() calls device_init_wakeup() to register a wakeup > source before initializing dev_name. As a result, device_wakeup_enable() > end up registering wakeup source with a null name when wakeup_source_register() > gets called with dev_name(dev) which is null at the time. > > When kernel is booted with wakeup_source_activate enabled, it will panic > when the trace point code tries to dereference ws->name. > > Fixed the problem by moving up the kobject_set_name() call prior to accesses > to dev_name(). Replaced kobject_set_name() with dev_set_name() which is the > right interface to be called from drivers. Fixed the call to device_del() prior > to device_add() in for wakeup_init_failed error handling code. Applied, thanks a lot! Anton ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str [not found] <cover.1385138418.git.shuah.kh@samsung.com> 2013-11-22 17:54 ` [PATCH v3 1/2] power_supply: Fix Oops from NULL pointer dereference from wakeup_source_activate Shuah Khan @ 2013-11-22 17:54 ` Shuah Khan 2013-11-22 22:18 ` Rafael J. Wysocki 1 sibling, 1 reply; 6+ messages in thread From: Shuah Khan @ 2013-11-22 17:54 UTC (permalink / raw) To: anton, dwmw2, rostedt, fweisbec, mingo, rjw, gregkh Cc: Shuah Khan, linux-kernel, shuahkhan, stable Tracing infrastructure routine __assign_str doesn't handle null strings. As a result when an trace event passes in a null string, kernel panics when skip_spaces() is invoked on the string. The following oops occurred when a null wakeup source name is specified. power_supply_register() calls device_init_wakeup() to register a wakeup source before initializing dev_name. As a result, device_wakeup_enable() end up registering wakeup source with a null name when wakeup_source_register() gets called with dev_name(dev) which is null at the time. When kernel is booted with wakeup_source_activate enabled, it will panic when the trace point code tries to dereference ws->name. Registering a a wakeup source without a name should be possible. Fix tracing infrastructure to be more robust in handling null strings in __assign_string() and __string(). With this change null string is handled gracefully and replacing it with "(null)" when trace is generated. This will address the problem at the tracing infrastructure level which is better than fixing individual tracepoint code. Trace after the fix: bash-2177 [000] d... 583.560106: wakeup_source_activate: (null) state=0x20001 kworker/0:2-378 [000] d... 583.560714: wakeup_source_deactivate: (null) state=0x30000 Oops message: [ 819.769934] device: 'BAT1': device_add [ 819.770078] PM: Adding info for No Bus:BAT1 [ 819.770235] BUG: unable to handle kernel NULL pointer dereference at (null) [ 819.770435] IP: [<ffffffff813381c0>] skip_spaces+0x30/0x30 [ 819.770572] PGD 3efd90067 PUD 3eff61067 PMD 0 [ 819.770716] Oops: 0000 [#1] SMP [ 819.770829] Modules linked in: arc4 iwldvm mac80211 x86_pkg_temp_thermal coretemp kvm_intel joydev i915 kvm uvcvideo ghash_clmulni_intel videobuf2_vmalloc aesni_intel videobuf2_memops videobuf2_core aes_x86_64 ablk_helper cryptd videodev iwlwifi lrw rfcomm gf128mul glue_helper bnep btusb media bluetooth parport_pc hid_generic ppdev snd_hda_codec_hdmi drm_kms_helper snd_hda_codec_realtek cfg80211 drm tpm_infineon samsung_laptop snd_hda_intel usbhid snd_hda_codec hid snd_hwdep snd_pcm microcode snd_page_alloc snd_timer psmouse i2c_algo_bit lpc_ich tpm_tis video wmi mac_hid serio_raw ext2 lp parport r8169 mii [ 819.771802] CPU: 0 PID: 2167 Comm: bash Not tainted 3.12.0+ #25 [ 819.771876] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X4C/900X4D/SAMSUNG_NP1234567890, BIOS P03AAC 07/12/2012 [ 819.772022] task: ffff88002e6ddcc0 ti: ffff8804015ca000 task.ti: ffff8804015ca000 [ 819.772119] RIP: 0010:[<ffffffff813381c0>] [<ffffffff813381c0>] skip_spaces+0x30/0x30 [ 819.772242] RSP: 0018:ffff8804015cbc70 EFLAGS: 00010046 [ 819.772310] RAX: 0000000000000003 RBX: ffff88040cfd6d40 RCX: 0000000000000018 [ 819.772397] RDX: 0000000000020001 RSI: 0000000000000000 RDI: 0000000000000000 [ 819.772484] RBP: ffff8804015cbcc0 R08: 0000000000000000 R09: ffff8803f0768d40 [ 819.772570] R10: ffffea001033b800 R11: 0000000000000000 R12: ffffffff81c519c0 [ 819.772656] R13: 0000000000020001 R14: 0000000000000000 R15: 0000000000020001 [ 819.772744] FS: 00007ff98309b740(0000) GS:ffff88041f200000(0000) knlGS:0000000000000000 [ 819.772845] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 819.772917] CR2: 0000000000000000 CR3: 00000003f59dc000 CR4: 00000000001407f0 [ 819.773001] Stack: [ 819.773030] ffffffff81114003 ffff8804015cbcb0 0000000000000000 0000000000000046 [ 819.773146] ffff880409757a18 ffff8803f065a160 0000000000000000 0000000000020001 [ 819.773273] 0000000000000000 0000000000000000 ffff8804015cbce8 ffffffff8143e388 [ 819.773387] Call Trace: [ 819.773434] [<ffffffff81114003>] ? ftrace_raw_event_wakeup_source+0x43/0xe0 [ 819.773520] [<ffffffff8143e388>] wakeup_source_report_event+0xb8/0xd0 [ 819.773595] [<ffffffff8143e3cd>] __pm_stay_awake+0x2d/0x50 [ 819.773724] [<ffffffff8153395c>] power_supply_changed+0x3c/0x90 [ 819.773795] [<ffffffff8153407c>] power_supply_register+0x18c/0x250 [ 819.773869] [<ffffffff813d8d18>] sysfs_add_battery+0x61/0x7b [ 819.773935] [<ffffffff813d8d69>] battery_notify+0x37/0x3f [ 819.774001] [<ffffffff816ccb7c>] notifier_call_chain+0x4c/0x70 [ 819.774071] [<ffffffff81073ded>] __blocking_notifier_call_chain+0x4d/0x70 [ 819.774149] [<ffffffff81073e26>] blocking_notifier_call_chain+0x16/0x20 [ 819.774227] [<ffffffff8109397a>] pm_notifier_call_chain+0x1a/0x40 [ 819.774316] [<ffffffff81095b66>] hibernate+0x66/0x1c0 [ 819.774407] [<ffffffff81093931>] state_store+0x71/0xa0 [ 819.774507] [<ffffffff81331d8f>] kobj_attr_store+0xf/0x20 [ 819.774613] [<ffffffff811f8618>] sysfs_write_file+0x128/0x1c0 [ 819.774735] [<ffffffff8118579d>] vfs_write+0xbd/0x1e0 [ 819.774841] [<ffffffff811861d9>] SyS_write+0x49/0xa0 [ 819.774939] [<ffffffff816d1052>] system_call_fastpath+0x16/0x1b [ 819.775055] Code: 89 f8 48 89 e5 f6 82 c0 a6 84 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 c0 a6 84 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 819.775760] RIP [<ffffffff813381c0>] skip_spaces+0x30/0x30 [ 819.775881] RSP <ffff8804015cbc70> [ 819.775949] CR2: 0000000000000000 [ 819.794175] ---[ end trace c4ef25127039952e ]--- Signed-off-by: Shuah Khan <shuah.kh@samsung.com> Cc: stable@vger.kernel.org --- include/trace/ftrace.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h index 52594b2..79f4639 100644 --- a/include/trace/ftrace.h +++ b/include/trace/ftrace.h @@ -372,7 +372,9 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call) \ __data_size += (len) * sizeof(type); #undef __string -#define __string(item, src) __dynamic_array(char, item, strlen(src) + 1) +#define __string(item, src) \ + __dynamic_array(char, item, \ + strlen((const char *)src ? (const char *)src : "(null)") + 1) #undef DECLARE_EVENT_CLASS #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \ @@ -501,7 +503,8 @@ static inline notrace int ftrace_get_offsets_##call( \ #undef __assign_str #define __assign_str(dst, src) \ - strcpy(__get_str(dst), src); + strcpy(__get_str(dst), \ + ((const char *)src ? (const char *)src : "(null)")) #undef TP_fast_assign #define TP_fast_assign(args...) args -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str 2013-11-22 17:54 ` [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str Shuah Khan @ 2013-11-22 22:18 ` Rafael J. Wysocki 2013-11-26 14:46 ` Steven Rostedt 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2013-11-22 22:18 UTC (permalink / raw) To: Shuah Khan Cc: anton, dwmw2, rostedt, fweisbec, mingo, gregkh, linux-kernel, shuahkhan, stable On Friday, November 22, 2013 10:54:29 AM Shuah Khan wrote: > Tracing infrastructure routine __assign_str doesn't handle null strings. > As a result when an trace event passes in a null string, kernel panics > when skip_spaces() is invoked on the string. The following oops occurred > when a null wakeup source name is specified. > > power_supply_register() calls device_init_wakeup() to register a wakeup > source before initializing dev_name. As a result, device_wakeup_enable() > end up registering wakeup source with a null name when wakeup_source_register() > gets called with dev_name(dev) which is null at the time. > > When kernel is booted with wakeup_source_activate enabled, it will panic > when the trace point code tries to dereference ws->name. Registering a > a wakeup source without a name should be possible. > > Fix tracing infrastructure to be more robust in handling null strings in > __assign_string() and __string(). With this change null string is handled > gracefully and replacing it with "(null)" when trace is generated. This will > address the problem at the tracing infrastructure level which is better than > fixing individual tracepoint code. > > Trace after the fix: > bash-2177 [000] d... 583.560106: wakeup_source_activate: (null) state=0x20001 > kworker/0:2-378 [000] d... 583.560714: wakeup_source_deactivate: (null) state=0x30000 > > Oops message: > > [ 819.769934] device: 'BAT1': device_add > [ 819.770078] PM: Adding info for No Bus:BAT1 > [ 819.770235] BUG: unable to handle kernel NULL pointer dereference at (null) > [ 819.770435] IP: [<ffffffff813381c0>] skip_spaces+0x30/0x30 > [ 819.770572] PGD 3efd90067 PUD 3eff61067 PMD 0 > [ 819.770716] Oops: 0000 [#1] SMP > [ 819.770829] Modules linked in: arc4 iwldvm mac80211 x86_pkg_temp_thermal coretemp kvm_intel joydev i915 kvm uvcvideo ghash_clmulni_intel videobuf2_vmalloc aesni_intel videobuf2_memops videobuf2_core aes_x86_64 ablk_helper cryptd videodev iwlwifi lrw rfcomm gf128mul glue_helper bnep btusb media bluetooth parport_pc hid_generic ppdev snd_hda_codec_hdmi drm_kms_helper snd_hda_codec_realtek cfg80211 drm tpm_infineon samsung_laptop snd_hda_intel usbhid snd_hda_codec hid snd_hwdep snd_pcm microcode snd_page_alloc snd_timer psmouse i2c_algo_bit lpc_ich tpm_tis video wmi mac_hid serio_raw ext2 lp parport r8169 mii > [ 819.771802] CPU: 0 PID: 2167 Comm: bash Not tainted 3.12.0+ #25 > [ 819.771876] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X4C/900X4D/SAMSUNG_NP1234567890, BIOS P03AAC 07/12/2012 > [ 819.772022] task: ffff88002e6ddcc0 ti: ffff8804015ca000 task.ti: ffff8804015ca000 > [ 819.772119] RIP: 0010:[<ffffffff813381c0>] [<ffffffff813381c0>] skip_spaces+0x30/0x30 > [ 819.772242] RSP: 0018:ffff8804015cbc70 EFLAGS: 00010046 > [ 819.772310] RAX: 0000000000000003 RBX: ffff88040cfd6d40 RCX: 0000000000000018 > [ 819.772397] RDX: 0000000000020001 RSI: 0000000000000000 RDI: 0000000000000000 > [ 819.772484] RBP: ffff8804015cbcc0 R08: 0000000000000000 R09: ffff8803f0768d40 > [ 819.772570] R10: ffffea001033b800 R11: 0000000000000000 R12: ffffffff81c519c0 > [ 819.772656] R13: 0000000000020001 R14: 0000000000000000 R15: 0000000000020001 > [ 819.772744] FS: 00007ff98309b740(0000) GS:ffff88041f200000(0000) knlGS:0000000000000000 > [ 819.772845] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 819.772917] CR2: 0000000000000000 CR3: 00000003f59dc000 CR4: 00000000001407f0 > [ 819.773001] Stack: > [ 819.773030] ffffffff81114003 ffff8804015cbcb0 0000000000000000 0000000000000046 > [ 819.773146] ffff880409757a18 ffff8803f065a160 0000000000000000 0000000000020001 > [ 819.773273] 0000000000000000 0000000000000000 ffff8804015cbce8 ffffffff8143e388 > [ 819.773387] Call Trace: > [ 819.773434] [<ffffffff81114003>] ? ftrace_raw_event_wakeup_source+0x43/0xe0 > [ 819.773520] [<ffffffff8143e388>] wakeup_source_report_event+0xb8/0xd0 > [ 819.773595] [<ffffffff8143e3cd>] __pm_stay_awake+0x2d/0x50 > [ 819.773724] [<ffffffff8153395c>] power_supply_changed+0x3c/0x90 > [ 819.773795] [<ffffffff8153407c>] power_supply_register+0x18c/0x250 > [ 819.773869] [<ffffffff813d8d18>] sysfs_add_battery+0x61/0x7b > [ 819.773935] [<ffffffff813d8d69>] battery_notify+0x37/0x3f > [ 819.774001] [<ffffffff816ccb7c>] notifier_call_chain+0x4c/0x70 > [ 819.774071] [<ffffffff81073ded>] __blocking_notifier_call_chain+0x4d/0x70 > [ 819.774149] [<ffffffff81073e26>] blocking_notifier_call_chain+0x16/0x20 > [ 819.774227] [<ffffffff8109397a>] pm_notifier_call_chain+0x1a/0x40 > [ 819.774316] [<ffffffff81095b66>] hibernate+0x66/0x1c0 > [ 819.774407] [<ffffffff81093931>] state_store+0x71/0xa0 > [ 819.774507] [<ffffffff81331d8f>] kobj_attr_store+0xf/0x20 > [ 819.774613] [<ffffffff811f8618>] sysfs_write_file+0x128/0x1c0 > [ 819.774735] [<ffffffff8118579d>] vfs_write+0xbd/0x1e0 > [ 819.774841] [<ffffffff811861d9>] SyS_write+0x49/0xa0 > [ 819.774939] [<ffffffff816d1052>] system_call_fastpath+0x16/0x1b > [ 819.775055] Code: 89 f8 48 89 e5 f6 82 c0 a6 84 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 c0 a6 84 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 > [ 819.775760] RIP [<ffffffff813381c0>] skip_spaces+0x30/0x30 > [ 819.775881] RSP <ffff8804015cbc70> > [ 819.775949] CR2: 0000000000000000 > [ 819.794175] ---[ end trace c4ef25127039952e ]--- > > Signed-off-by: Shuah Khan <shuah.kh@samsung.com> > Cc: stable@vger.kernel.org Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > include/trace/ftrace.h | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h > index 52594b2..79f4639 100644 > --- a/include/trace/ftrace.h > +++ b/include/trace/ftrace.h > @@ -372,7 +372,9 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call) \ > __data_size += (len) * sizeof(type); > > #undef __string > -#define __string(item, src) __dynamic_array(char, item, strlen(src) + 1) > +#define __string(item, src) \ > + __dynamic_array(char, item, \ > + strlen((const char *)src ? (const char *)src : "(null)") + 1) > > #undef DECLARE_EVENT_CLASS > #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \ > @@ -501,7 +503,8 @@ static inline notrace int ftrace_get_offsets_##call( \ > > #undef __assign_str > #define __assign_str(dst, src) \ > - strcpy(__get_str(dst), src); > + strcpy(__get_str(dst), \ > + ((const char *)src ? (const char *)src : "(null)")) > > #undef TP_fast_assign > #define TP_fast_assign(args...) args > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str 2013-11-22 22:18 ` Rafael J. Wysocki @ 2013-11-26 14:46 ` Steven Rostedt 2013-11-26 15:07 ` Shuah Khan 0 siblings, 1 reply; 6+ messages in thread From: Steven Rostedt @ 2013-11-26 14:46 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Shuah Khan, anton, dwmw2, fweisbec, mingo, gregkh, linux-kernel, shuahkhan, stable On Fri, 22 Nov 2013 23:18:09 +0100 "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote: > On Friday, November 22, 2013 10:54:29 AM Shuah Khan wrote: > > Tracing infrastructure routine __assign_str doesn't handle null strings. > > As a result when an trace event passes in a null string, kernel panics > > when skip_spaces() is invoked on the string. The following oops occurred > > when a null wakeup source name is specified. > > > > power_supply_register() calls device_init_wakeup() to register a wakeup > > source before initializing dev_name. As a result, device_wakeup_enable() > > end up registering wakeup source with a null name when wakeup_source_register() > > gets called with dev_name(dev) which is null at the time. > > > > When kernel is booted with wakeup_source_activate enabled, it will panic > > when the trace point code tries to dereference ws->name. Registering a > > a wakeup source without a name should be possible. > > > > Fix tracing infrastructure to be more robust in handling null strings in > > __assign_string() and __string(). With this change null string is handled > > gracefully and replacing it with "(null)" when trace is generated. This will > > address the problem at the tracing infrastructure level which is better than > > fixing individual tracepoint code. > > > > Trace after the fix: > > bash-2177 [000] d... 583.560106: wakeup_source_activate: (null) state=0x20001 > > kworker/0:2-378 [000] d... 583.560714: wakeup_source_deactivate: (null) state=0x30000 > > > > Oops message: > > > > [ 819.769934] device: 'BAT1': device_add > > [ 819.770078] PM: Adding info for No Bus:BAT1 > > [ 819.770235] BUG: unable to handle kernel NULL pointer dereference at (null) > > [ 819.770435] IP: [<ffffffff813381c0>] skip_spaces+0x30/0x30 > > [ 819.770572] PGD 3efd90067 PUD 3eff61067 PMD 0 > > [ 819.770716] Oops: 0000 [#1] SMP > > [ 819.770829] Modules linked in: arc4 iwldvm mac80211 x86_pkg_temp_thermal coretemp kvm_intel joydev i915 kvm uvcvideo ghash_clmulni_intel videobuf2_vmalloc aesni_intel videobuf2_memops videobuf2_core aes_x86_64 ablk_helper cryptd videodev iwlwifi lrw rfcomm gf128mul glue_helper bnep btusb media bluetooth parport_pc hid_generic ppdev snd_hda_codec_hdmi drm_kms_helper snd_hda_codec_realtek cfg80211 drm tpm_infineon samsung_laptop snd_hda_intel usbhid snd_hda_codec hid snd_hwdep snd_pcm microcode snd_page_alloc snd_timer psmouse i2c_algo_bit lpc_ich tpm_tis video wmi mac_hid serio_raw ext2 lp parport r8169 mii > > [ 819.771802] CPU: 0 PID: 2167 Comm: bash Not tainted 3.12.0+ #25 > > [ 819.771876] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X4C/900X4D/SAMSUNG_NP1234567890, BIOS P03AAC 07/12/2012 > > [ 819.772022] task: ffff88002e6ddcc0 ti: ffff8804015ca000 task.ti: ffff8804015ca000 > > [ 819.772119] RIP: 0010:[<ffffffff813381c0>] [<ffffffff813381c0>] skip_spaces+0x30/0x30 > > [ 819.772242] RSP: 0018:ffff8804015cbc70 EFLAGS: 00010046 > > [ 819.772310] RAX: 0000000000000003 RBX: ffff88040cfd6d40 RCX: 0000000000000018 > > [ 819.772397] RDX: 0000000000020001 RSI: 0000000000000000 RDI: 0000000000000000 > > [ 819.772484] RBP: ffff8804015cbcc0 R08: 0000000000000000 R09: ffff8803f0768d40 > > [ 819.772570] R10: ffffea001033b800 R11: 0000000000000000 R12: ffffffff81c519c0 > > [ 819.772656] R13: 0000000000020001 R14: 0000000000000000 R15: 0000000000020001 > > [ 819.772744] FS: 00007ff98309b740(0000) GS:ffff88041f200000(0000) knlGS:0000000000000000 > > [ 819.772845] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > [ 819.772917] CR2: 0000000000000000 CR3: 00000003f59dc000 CR4: 00000000001407f0 > > [ 819.773001] Stack: > > [ 819.773030] ffffffff81114003 ffff8804015cbcb0 0000000000000000 0000000000000046 > > [ 819.773146] ffff880409757a18 ffff8803f065a160 0000000000000000 0000000000020001 > > [ 819.773273] 0000000000000000 0000000000000000 ffff8804015cbce8 ffffffff8143e388 > > [ 819.773387] Call Trace: > > [ 819.773434] [<ffffffff81114003>] ? ftrace_raw_event_wakeup_source+0x43/0xe0 > > [ 819.773520] [<ffffffff8143e388>] wakeup_source_report_event+0xb8/0xd0 > > [ 819.773595] [<ffffffff8143e3cd>] __pm_stay_awake+0x2d/0x50 > > [ 819.773724] [<ffffffff8153395c>] power_supply_changed+0x3c/0x90 > > [ 819.773795] [<ffffffff8153407c>] power_supply_register+0x18c/0x250 > > [ 819.773869] [<ffffffff813d8d18>] sysfs_add_battery+0x61/0x7b > > [ 819.773935] [<ffffffff813d8d69>] battery_notify+0x37/0x3f > > [ 819.774001] [<ffffffff816ccb7c>] notifier_call_chain+0x4c/0x70 > > [ 819.774071] [<ffffffff81073ded>] __blocking_notifier_call_chain+0x4d/0x70 > > [ 819.774149] [<ffffffff81073e26>] blocking_notifier_call_chain+0x16/0x20 > > [ 819.774227] [<ffffffff8109397a>] pm_notifier_call_chain+0x1a/0x40 > > [ 819.774316] [<ffffffff81095b66>] hibernate+0x66/0x1c0 > > [ 819.774407] [<ffffffff81093931>] state_store+0x71/0xa0 > > [ 819.774507] [<ffffffff81331d8f>] kobj_attr_store+0xf/0x20 > > [ 819.774613] [<ffffffff811f8618>] sysfs_write_file+0x128/0x1c0 > > [ 819.774735] [<ffffffff8118579d>] vfs_write+0xbd/0x1e0 > > [ 819.774841] [<ffffffff811861d9>] SyS_write+0x49/0xa0 > > [ 819.774939] [<ffffffff816d1052>] system_call_fastpath+0x16/0x1b > > [ 819.775055] Code: 89 f8 48 89 e5 f6 82 c0 a6 84 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 c0 a6 84 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 > > [ 819.775760] RIP [<ffffffff813381c0>] skip_spaces+0x30/0x30 > > [ 819.775881] RSP <ffff8804015cbc70> > > [ 819.775949] CR2: 0000000000000000 > > [ 819.794175] ---[ end trace c4ef25127039952e ]--- > > > > Signed-off-by: Shuah Khan <shuah.kh@samsung.com> > > Cc: stable@vger.kernel.org > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The below is my patch. I have it in my queue and I will be pushing it after it succeeds all my testing. -- Steve > > > --- > > include/trace/ftrace.h | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h > > index 52594b2..79f4639 100644 > > --- a/include/trace/ftrace.h > > +++ b/include/trace/ftrace.h > > @@ -372,7 +372,9 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call) \ > > __data_size += (len) * sizeof(type); > > > > #undef __string > > -#define __string(item, src) __dynamic_array(char, item, strlen(src) + 1) > > +#define __string(item, src) \ > > + __dynamic_array(char, item, \ > > + strlen((const char *)src ? (const char *)src : "(null)") + 1) > > > > #undef DECLARE_EVENT_CLASS > > #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \ > > @@ -501,7 +503,8 @@ static inline notrace int ftrace_get_offsets_##call( \ > > > > #undef __assign_str > > #define __assign_str(dst, src) \ > > - strcpy(__get_str(dst), src); > > + strcpy(__get_str(dst), \ > > + ((const char *)src ? (const char *)src : "(null)")) > > > > #undef TP_fast_assign > > #define TP_fast_assign(args...) args > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str 2013-11-26 14:46 ` Steven Rostedt @ 2013-11-26 15:07 ` Shuah Khan 0 siblings, 0 replies; 6+ messages in thread From: Shuah Khan @ 2013-11-26 15:07 UTC (permalink / raw) To: Steven Rostedt, Rafael J. Wysocki Cc: anton, dwmw2, fweisbec, mingo, gregkh, linux-kernel, shuahkhan, stable, Shuah Khan On 11/26/2013 07:46 AM, Steven Rostedt wrote: > On Fri, 22 Nov 2013 23:18:09 +0100 > "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote: > >> On Friday, November 22, 2013 10:54:29 AM Shuah Khan wrote: >>> Tracing infrastructure routine __assign_str doesn't handle null strings. >>> As a result when an trace event passes in a null string, kernel panics >>> when skip_spaces() is invoked on the string. The following oops occurred >>> when a null wakeup source name is specified. >>> >>> power_supply_register() calls device_init_wakeup() to register a wakeup >>> source before initializing dev_name. As a result, device_wakeup_enable() >>> end up registering wakeup source with a null name when wakeup_source_register() >>> gets called with dev_name(dev) which is null at the time. >>> >>> When kernel is booted with wakeup_source_activate enabled, it will panic >>> when the trace point code tries to dereference ws->name. Registering a >>> a wakeup source without a name should be possible. >>> >>> Fix tracing infrastructure to be more robust in handling null strings in >>> __assign_string() and __string(). With this change null string is handled >>> gracefully and replacing it with "(null)" when trace is generated. This will >>> address the problem at the tracing infrastructure level which is better than >>> fixing individual tracepoint code. >>> >>> Trace after the fix: >>> bash-2177 [000] d... 583.560106: wakeup_source_activate: (null) state=0x20001 >>> kworker/0:2-378 [000] d... 583.560714: wakeup_source_deactivate: (null) state=0x30000 >>> >>> Oops message: >>> >>> [ 819.769934] device: 'BAT1': device_add >>> [ 819.770078] PM: Adding info for No Bus:BAT1 >>> [ 819.770235] BUG: unable to handle kernel NULL pointer dereference at (null) >>> [ 819.770435] IP: [<ffffffff813381c0>] skip_spaces+0x30/0x30 >>> [ 819.770572] PGD 3efd90067 PUD 3eff61067 PMD 0 >>> [ 819.770716] Oops: 0000 [#1] SMP >>> [ 819.770829] Modules linked in: arc4 iwldvm mac80211 x86_pkg_temp_thermal coretemp kvm_intel joydev i915 kvm uvcvideo ghash_clmulni_intel videobuf2_vmalloc aesni_intel videobuf2_memops videobuf2_core aes_x86_64 ablk_helper cryptd videodev iwlwifi lrw rfcomm gf128mul glue_helper bnep btusb media bluetooth parport_pc hid_generic ppdev snd_hda_codec_hdmi drm_kms_helper snd_hda_codec_realtek cfg80211 drm tpm_infineon samsung_laptop snd_hda_intel usbhid snd_hda_codec hid snd_hwdep snd_pcm microcode snd_page_alloc snd_timer psmouse i2c_algo_bit lpc_ich tpm_tis video wmi mac_hid serio_raw ext2 lp parport r8169 mii >>> [ 819.771802] CPU: 0 PID: 2167 Comm: bash Not tainted 3.12.0+ #25 >>> [ 819.771876] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X4C/900X4D/SAMSUNG_NP1234567890, BIOS P03AAC 07/12/2012 >>> [ 819.772022] task: ffff88002e6ddcc0 ti: ffff8804015ca000 task.ti: ffff8804015ca000 >>> [ 819.772119] RIP: 0010:[<ffffffff813381c0>] [<ffffffff813381c0>] skip_spaces+0x30/0x30 >>> [ 819.772242] RSP: 0018:ffff8804015cbc70 EFLAGS: 00010046 >>> [ 819.772310] RAX: 0000000000000003 RBX: ffff88040cfd6d40 RCX: 0000000000000018 >>> [ 819.772397] RDX: 0000000000020001 RSI: 0000000000000000 RDI: 0000000000000000 >>> [ 819.772484] RBP: ffff8804015cbcc0 R08: 0000000000000000 R09: ffff8803f0768d40 >>> [ 819.772570] R10: ffffea001033b800 R11: 0000000000000000 R12: ffffffff81c519c0 >>> [ 819.772656] R13: 0000000000020001 R14: 0000000000000000 R15: 0000000000020001 >>> [ 819.772744] FS: 00007ff98309b740(0000) GS:ffff88041f200000(0000) knlGS:0000000000000000 >>> [ 819.772845] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> [ 819.772917] CR2: 0000000000000000 CR3: 00000003f59dc000 CR4: 00000000001407f0 >>> [ 819.773001] Stack: >>> [ 819.773030] ffffffff81114003 ffff8804015cbcb0 0000000000000000 0000000000000046 >>> [ 819.773146] ffff880409757a18 ffff8803f065a160 0000000000000000 0000000000020001 >>> [ 819.773273] 0000000000000000 0000000000000000 ffff8804015cbce8 ffffffff8143e388 >>> [ 819.773387] Call Trace: >>> [ 819.773434] [<ffffffff81114003>] ? ftrace_raw_event_wakeup_source+0x43/0xe0 >>> [ 819.773520] [<ffffffff8143e388>] wakeup_source_report_event+0xb8/0xd0 >>> [ 819.773595] [<ffffffff8143e3cd>] __pm_stay_awake+0x2d/0x50 >>> [ 819.773724] [<ffffffff8153395c>] power_supply_changed+0x3c/0x90 >>> [ 819.773795] [<ffffffff8153407c>] power_supply_register+0x18c/0x250 >>> [ 819.773869] [<ffffffff813d8d18>] sysfs_add_battery+0x61/0x7b >>> [ 819.773935] [<ffffffff813d8d69>] battery_notify+0x37/0x3f >>> [ 819.774001] [<ffffffff816ccb7c>] notifier_call_chain+0x4c/0x70 >>> [ 819.774071] [<ffffffff81073ded>] __blocking_notifier_call_chain+0x4d/0x70 >>> [ 819.774149] [<ffffffff81073e26>] blocking_notifier_call_chain+0x16/0x20 >>> [ 819.774227] [<ffffffff8109397a>] pm_notifier_call_chain+0x1a/0x40 >>> [ 819.774316] [<ffffffff81095b66>] hibernate+0x66/0x1c0 >>> [ 819.774407] [<ffffffff81093931>] state_store+0x71/0xa0 >>> [ 819.774507] [<ffffffff81331d8f>] kobj_attr_store+0xf/0x20 >>> [ 819.774613] [<ffffffff811f8618>] sysfs_write_file+0x128/0x1c0 >>> [ 819.774735] [<ffffffff8118579d>] vfs_write+0xbd/0x1e0 >>> [ 819.774841] [<ffffffff811861d9>] SyS_write+0x49/0xa0 >>> [ 819.774939] [<ffffffff816d1052>] system_call_fastpath+0x16/0x1b >>> [ 819.775055] Code: 89 f8 48 89 e5 f6 82 c0 a6 84 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 c0 a6 84 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 >>> [ 819.775760] RIP [<ffffffff813381c0>] skip_spaces+0x30/0x30 >>> [ 819.775881] RSP <ffff8804015cbc70> >>> [ 819.775949] CR2: 0000000000000000 >>> [ 819.794175] ---[ end trace c4ef25127039952e ]--- >>> >>> Signed-off-by: Shuah Khan <shuah.kh@samsung.com> >>> Cc: stable@vger.kernel.org >> >> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The below is my patch. I have it in my queue and I will be pushing it > after it succeeds all my testing. > > -- Steve I am happy as long as the problem is fixed. :) -- Shuah > >> >>> --- >>> include/trace/ftrace.h | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h >>> index 52594b2..79f4639 100644 >>> --- a/include/trace/ftrace.h >>> +++ b/include/trace/ftrace.h >>> @@ -372,7 +372,9 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call) \ >>> __data_size += (len) * sizeof(type); >>> >>> #undef __string >>> -#define __string(item, src) __dynamic_array(char, item, strlen(src) + 1) >>> +#define __string(item, src) \ >>> + __dynamic_array(char, item, \ >>> + strlen((const char *)src ? (const char *)src : "(null)") + 1) >>> >>> #undef DECLARE_EVENT_CLASS >>> #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \ >>> @@ -501,7 +503,8 @@ static inline notrace int ftrace_get_offsets_##call( \ >>> >>> #undef __assign_str >>> #define __assign_str(dst, src) \ >>> - strcpy(__get_str(dst), src); >>> + strcpy(__get_str(dst), \ >>> + ((const char *)src ? (const char *)src : "(null)")) >>> >>> #undef TP_fast_assign >>> #define TP_fast_assign(args...) args >>> > -- Shuah Khan Senior Linux Kernel Developer - Open Source Group Samsung Research America(Silicon Valley) shuah.kh@samsung.com | (970) 672-0658 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-01 21:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1385138418.git.shuah.kh@samsung.com>
2013-11-22 17:54 ` [PATCH v3 1/2] power_supply: Fix Oops from NULL pointer dereference from wakeup_source_activate Shuah Khan
2013-12-01 21:50 ` Anton Vorontsov
2013-11-22 17:54 ` [PATCH v3 2/2] tracing: Fix Oops from NULL pointer dereference from __assign_str Shuah Khan
2013-11-22 22:18 ` Rafael J. Wysocki
2013-11-26 14:46 ` Steven Rostedt
2013-11-26 15:07 ` Shuah Khan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox