From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753165AbcEMNyD (ORCPT ); Fri, 13 May 2016 09:54:03 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:36144 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751709AbcEMNyA (ORCPT ); Fri, 13 May 2016 09:54:00 -0400 From: Namhyung Kim To: Steven Rostedt Cc: LKML , Ingo Molnar , x86@kernel.org, Borislav Petkov Subject: [PATCH] ftrace/x86: Fix function graph tracer reset path Date: Fri, 13 May 2016 22:53:43 +0900 Message-Id: <1463147623-2575-1-git-send-email-namhyung@kernel.org> X-Mailer: git-send-email 2.8.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On my system, simply enabling and disabling function graph tracer can crash the kernel. I don't know how it worked until now. The ftrace_disable_ftrace_graph_caller() modifies jmp instruction at ftrace_graph_call assuming it's a 5 bytes near jmp (e9 ). However it's a short jmp consisting of 2 bytes only (eb ). And ftrace_stub() is located just below the ftrace_graph_caller so modification above breaks the instruction resulting in kernel oops on the ftrace_stub() with the invalid opcode like below: # cd /sys/kernel/trace # echo function_graph > current_tracer # echo nop > current_tracer [ 78.122055] invalid opcode: 0000 [#1] SMP [ 78.125125] Modules linked in: x86_pkg_temp_thermal kvm_intel kvm irqbypass crc32c_intel pcspkr iwldvm iwlwifi [ 78.128241] CPU: 2 PID: 17 Comm: migration/2 Not tainted 4.6.0-rc4+ #36 [ 78.131310] Hardware name: LENOVO 4286A74/4286A74, BIOS 8DET56WW (1.26 ) 12/01/2011 [ 78.134369] task: ffff88040bec4240 ti: ffff88040bee4000 task.ti: ffff88040bee4000 [ 78.137412] RIP: 0010:[] [] ftrace_stub+0x0/0x8 [ 78.140477] RSP: 0018:ffff88040bee7e48 EFLAGS: 00010246 [ 78.143489] RAX: ffff88040bec4240 RBX: ffffffff8107a7b0 RCX: 000000000000001f [ 78.146512] RDX: 0000000000000000 RSI: ffff88041e2929d8 RDI: ffff88040bee7e50 [ 78.149581] RBP: ffff88040bee7e80 R08: ffff88040bee4000 R09: 0000000000000000 [ 78.152647] R10: 00000000000318b7 R11: ffff8800d661f800 R12: ffff88040d8011b0 [ 78.155679] R13: ffffffff81e43620 R14: ffff88040bda8588 R15: ffffffff81e503e0 [ 78.158675] FS: 0000000000000000(0000) GS:ffff88041e280000(0000) knlGS:0000000000000000 [ 78.161699] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 78.164690] CR2: 00007fadb22dde1d CR3: 00000000d5ce2000 CR4: 00000000000406e0 [ 78.167691] Stack: [ 78.170658] ffffffff8110b3ee ffffffff8188de90 00000009161d55f6 00000012306fc2e4 [ 78.173710] 0000000000000000 ffff880400000000 ffff88040bec4240 ffff88040bee7ec8 [ 78.176783] ffffffff81893bbd 0000000000000000 ffff88040bec4240 ffffffff81893ba8 [ 78.179863] Call Trace: [ 78.182853] [] ? ftrace_return_to_handler+0x8e/0x100 [ 78.185909] [] ? __schedule+0xae0/0xae0 [ 78.188941] [] return_to_handler+0x15/0x27 [ 78.192001] [] ? ftrace_graph_caller+0xa8/0xa8 [ 78.195091] [] ? sort_range+0x30/0x30 [ 78.198138] [] kthread+0xc9/0xe0 [ 78.201143] [] ret_from_fork+0x22/0x40 [ 78.204138] [] ? kthread_worker_fn+0x170/0x170 [ 78.207129] Code: 8b 44 24 48 48 8b 7c 24 70 48 8b 74 24 68 48 8b 54 24 60 48 8b 4c 24 58 48 8b 44 24 50 48 8b 6c 24 20 48 81 c4 d0 00 00 00 e9 fd ff ff 80 00 00 00 00 9c 55 ff 74 24 18 55 48 89 e5 ff 74 24 [ 78.213997] RIP [] ftrace_stub+0x0/0x8 [ 78.217190] RSP [ 78.220374] ---[ end trace 0af2f0d9f7301011 ]--- Looking at the code dump, the ftrace_stub instruction was overwritten as . Below is disassembly output of related code. $ objdump -d --start-address=0xffffffff818939a6 --stop-address=0xffffffff818939b0 vmlinux vmlinux: file format elf64-x86-64 Disassembly of section .text: ffffffff818939a6 : ffffffff818939a6: eb 00 jmp ffffffff818939a8 ffffffff818939a8 : ffffffff818939a8: c3 retq ffffffff818939a9: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) As you can see ftrace_epilogue (same as ftrace_graph_caller) is 2 byte ahead of ftrace_stub. And it's replaced by a jump to ftrace_stub() by ftrace_disable_ftrace_graph_caller: "e9 <0xfffffffd>". Pads 3 bytes after ftrace_epilogue to prevent ftrace_stub from being overwritten. Signed-off-by: Namhyung Kim --- arch/x86/kernel/mcount_64.S | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/mcount_64.S index ed48a9f465f8..0e6af57a713a 100644 --- a/arch/x86/kernel/mcount_64.S +++ b/arch/x86/kernel/mcount_64.S @@ -180,6 +180,15 @@ GLOBAL(ftrace_epilogue) #ifdef CONFIG_FUNCTION_GRAPH_TRACER GLOBAL(ftrace_graph_call) jmp ftrace_stub + /* + * The above jmp is generated as a short jump which occupies 2 bytes + * but ftrace_enable/disable_ftrace_graph_caller() assumes it's a + * near jump which occupies 5 bytes so breaks ftrace_stub() below. + * Add 3 bytes padding to avoid that. + */ + nop + nop + nop #endif GLOBAL(ftrace_stub) -- 2.8.0