* [PATCH 0/4] x86: fix kernel address printk exposures @ 2016-10-25 14:51 Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 1/4] scripts/faddr2line: fix "size mismatch" error Josh Poimboeuf ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Josh Poimboeuf @ 2016-10-25 14:51 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Linus Torvalds These are some changes suggested by Linus to remove some kernel address exposures in printk. Most notably, the x86 stack dump no longer prints full kernel text addresses. There's also a fix for the faddr2line script which is used for converting a function offset into a source code file name and line number. Josh Poimboeuf (4): scripts/faddr2line: fix "size mismatch" error x86/dumpstack: remove kernel text addresses from stack dump x86/dumpstack: remove raw stack dump mm: remove kernel address exposure in free_reserved_area() Documentation/kernel-parameters.txt | 3 -- Documentation/sysctl/kernel.txt | 8 ----- Documentation/x86/x86_64/boot-options.txt | 4 --- arch/x86/include/asm/kdebug.h | 1 - arch/x86/include/asm/stacktrace.h | 5 --- arch/x86/kernel/dumpstack.c | 39 ++++------------------- arch/x86/kernel/dumpstack_32.c | 33 +------------------ arch/x86/kernel/dumpstack_64.c | 53 +------------------------------ arch/x86/kernel/process_32.c | 7 ++-- arch/x86/kernel/process_64.c | 6 ++-- arch/x86/mm/fault.c | 3 +- arch/x86/platform/uv/uv_nmi.c | 4 +-- kernel/sysctl.c | 7 ---- mm/page_alloc.c | 4 +-- scripts/faddr2line | 33 ++++++++++++------- 15 files changed, 40 insertions(+), 170 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] scripts/faddr2line: fix "size mismatch" error 2016-10-25 14:51 [PATCH 0/4] x86: fix kernel address printk exposures Josh Poimboeuf @ 2016-10-25 14:51 ` Josh Poimboeuf 2016-10-26 5:39 ` [tip:x86/asm] scripts/faddr2line: Fix " tip-bot for Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump Josh Poimboeuf ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Josh Poimboeuf @ 2016-10-25 14:51 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Linus Torvalds I'm not sure how we missed this problem before. When I take a function address and size from an oops and give it to faddr2line, it usually complains about a size mismatch: $ scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 skipping write_sysrq_trigger address at 0xffffffff815731a1 due to size mismatch (0x60 != 83) no match for write_sysrq_trigger+0x51/0x60 The problem is caused by differences in how kallsyms and faddr2line determine a function's size. kallsyms calculates a function's size by parsing the output of 'nm -n' and subtracting the next function's address from the current function's address. This means that nop instructions after the end of the function are included in the size. In contrast, faddr2line reads the size from the symbol table, which does *not* include the ending nops in the function's size. Change faddr2line to calculate the size from the output of 'nm -n' to be consistent with kallsyms and oops outputs. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- scripts/faddr2line | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 450b332..29df825 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -105,9 +105,18 @@ __faddr2line() { # In rare cases there might be duplicates. while read symbol; do local fields=($symbol) - local sym_base=0x${fields[1]} - local sym_size=${fields[2]} - local sym_type=${fields[3]} + local sym_base=0x${fields[0]} + local sym_type=${fields[1]} + local sym_end=0x${fields[3]} + + # calculate the size + local sym_size=$(($sym_end - $sym_base)) + if [[ -z $sym_size ]] || [[ $sym_size -le 0 ]]; then + warn "bad symbol size: base: $sym_base end: $sym_end" + DONE=1 + return + fi + sym_size=0x$(printf %x $sym_size) # calculate the address local addr=$(($sym_base + $offset)) @@ -116,26 +125,26 @@ __faddr2line() { DONE=1 return fi - local hexaddr=0x$(printf %x $addr) + addr=0x$(printf %x $addr) # weed out non-function symbols - if [[ $sym_type != "FUNC" ]]; then + if [[ $sym_type != t ]] && [[ $sym_type != T ]]; then [[ $print_warnings = 1 ]] && - echo "skipping $func address at $hexaddr due to non-function symbol" + echo "skipping $func address at $addr due to non-function symbol of type '$sym_type'" continue fi # if the user provided a size, make sure it matches the symbol's size if [[ -n $size ]] && [[ $size -ne $sym_size ]]; then [[ $print_warnings = 1 ]] && - echo "skipping $func address at $hexaddr due to size mismatch ($size != $sym_size)" + echo "skipping $func address at $addr due to size mismatch ($size != $sym_size)" continue; fi # make sure the provided offset is within the symbol's range if [[ $offset -gt $sym_size ]]; then [[ $print_warnings = 1 ]] && - echo "skipping $func address at $hexaddr due to size mismatch ($offset > $sym_size)" + echo "skipping $func address at $addr due to size mismatch ($offset > $sym_size)" continue fi @@ -143,12 +152,12 @@ __faddr2line() { [[ $FIRST = 0 ]] && echo FIRST=0 - local hexsize=0x$(printf %x $sym_size) - echo "$func+$offset/$hexsize:" - addr2line -fpie $objfile $hexaddr | sed "s; $dir_prefix\(\./\)*; ;" + # pass real address to addr2line + echo "$func+$offset/$sym_size:" + addr2line -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;" DONE=1 - done < <(readelf -sW $objfile | awk -v f=$func '$8 == f {print}') + done < <(nm -n $objfile | awk -v fn=$func '$3 == fn { found=1; line=$0; start=$1; next } found == 1 { found=0; print line, $1 }') } [[ $# -lt 2 ]] && usage -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:x86/asm] scripts/faddr2line: Fix "size mismatch" error 2016-10-25 14:51 ` [PATCH 1/4] scripts/faddr2line: fix "size mismatch" error Josh Poimboeuf @ 2016-10-26 5:39 ` tip-bot for Josh Poimboeuf 0 siblings, 0 replies; 17+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-10-26 5:39 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, luto, jpoimboe, hpa, tglx, peterz, torvalds, dvlasenk, bp, brgerst, linux-kernel Commit-ID: efdb4167e676aaba7505bec739785b76e206cb45 Gitweb: http://git.kernel.org/tip/efdb4167e676aaba7505bec739785b76e206cb45 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Tue, 25 Oct 2016 09:51:11 -0500 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 25 Oct 2016 18:40:37 +0200 scripts/faddr2line: Fix "size mismatch" error I'm not sure how we missed this problem before. When I take a function address and size from an oops and give it to faddr2line, it usually complains about a size mismatch: $ scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 skipping write_sysrq_trigger address at 0xffffffff815731a1 due to size mismatch (0x60 != 83) no match for write_sysrq_trigger+0x51/0x60 The problem is caused by differences in how kallsyms and faddr2line determine a function's size. kallsyms calculates a function's size by parsing the output of 'nm -n' and subtracting the next function's address from the current function's address. This means that nop instructions after the end of the function are included in the size. In contrast, faddr2line reads the size from the symbol table, which does *not* include the ending nops in the function's size. Change faddr2line to calculate the size from the output of 'nm -n' to be consistent with kallsyms and oops outputs. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/bd313ed7c4003f6b1fda63e825325c44a9d837de.1477405374.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- scripts/faddr2line | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 450b332..29df825 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -105,9 +105,18 @@ __faddr2line() { # In rare cases there might be duplicates. while read symbol; do local fields=($symbol) - local sym_base=0x${fields[1]} - local sym_size=${fields[2]} - local sym_type=${fields[3]} + local sym_base=0x${fields[0]} + local sym_type=${fields[1]} + local sym_end=0x${fields[3]} + + # calculate the size + local sym_size=$(($sym_end - $sym_base)) + if [[ -z $sym_size ]] || [[ $sym_size -le 0 ]]; then + warn "bad symbol size: base: $sym_base end: $sym_end" + DONE=1 + return + fi + sym_size=0x$(printf %x $sym_size) # calculate the address local addr=$(($sym_base + $offset)) @@ -116,26 +125,26 @@ __faddr2line() { DONE=1 return fi - local hexaddr=0x$(printf %x $addr) + addr=0x$(printf %x $addr) # weed out non-function symbols - if [[ $sym_type != "FUNC" ]]; then + if [[ $sym_type != t ]] && [[ $sym_type != T ]]; then [[ $print_warnings = 1 ]] && - echo "skipping $func address at $hexaddr due to non-function symbol" + echo "skipping $func address at $addr due to non-function symbol of type '$sym_type'" continue fi # if the user provided a size, make sure it matches the symbol's size if [[ -n $size ]] && [[ $size -ne $sym_size ]]; then [[ $print_warnings = 1 ]] && - echo "skipping $func address at $hexaddr due to size mismatch ($size != $sym_size)" + echo "skipping $func address at $addr due to size mismatch ($size != $sym_size)" continue; fi # make sure the provided offset is within the symbol's range if [[ $offset -gt $sym_size ]]; then [[ $print_warnings = 1 ]] && - echo "skipping $func address at $hexaddr due to size mismatch ($offset > $sym_size)" + echo "skipping $func address at $addr due to size mismatch ($offset > $sym_size)" continue fi @@ -143,12 +152,12 @@ __faddr2line() { [[ $FIRST = 0 ]] && echo FIRST=0 - local hexsize=0x$(printf %x $sym_size) - echo "$func+$offset/$hexsize:" - addr2line -fpie $objfile $hexaddr | sed "s; $dir_prefix\(\./\)*; ;" + # pass real address to addr2line + echo "$func+$offset/$sym_size:" + addr2line -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;" DONE=1 - done < <(readelf -sW $objfile | awk -v f=$func '$8 == f {print}') + done < <(nm -n $objfile | awk -v fn=$func '$3 == fn { found=1; line=$0; start=$1; next } found == 1 { found=0; print line, $1 }') } [[ $# -lt 2 ]] && usage ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump 2016-10-25 14:51 [PATCH 0/4] x86: fix kernel address printk exposures Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 1/4] scripts/faddr2line: fix "size mismatch" error Josh Poimboeuf @ 2016-10-25 14:51 ` Josh Poimboeuf 2016-10-26 5:40 ` [tip:x86/asm] x86/dumpstack: Remove " tip-bot for Josh Poimboeuf 2016-11-25 12:26 ` [PATCH 2/4] x86/dumpstack: remove " Kirill A. Shutemov 2016-10-25 14:51 ` [PATCH 3/4] x86/dumpstack: remove raw stack dump Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 4/4] mm: remove kernel address exposure in free_reserved_area() Josh Poimboeuf 3 siblings, 2 replies; 17+ messages in thread From: Josh Poimboeuf @ 2016-10-25 14:51 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Linus Torvalds Printing kernel text addresses in stack dumps is of questionable value, especially now that address randomization is becoming common. It can be a security issue because it leaks kernel addresses. It also affects the usefulness of the stack dump. Linus says: "I actually spend time cleaning up commit messages in logs, because useless data that isn't actually information (random hex numbers) is actively detrimental. It makes commit logs less legible. It also makes it harder to parse dumps. It's not useful. That makes it actively bad. I probably look at more oops reports than most people. I have not found the hex numbers useful for the last five years, because they are just randomized crap. The stack content thing just makes code scroll off the screen etc, for example." The only real downside to removing these addresses is that they can be used to disambiguate duplicate symbol names. However such cases are rare, and the context of the stack dump should be enough to be able to figure it out. There's now a 'faddr2line' script which can be used to convert a function address to a file name and line: $ ./scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 write_sysrq_trigger+0x51/0x60: write_sysrq_trigger at drivers/tty/sysrq.c:1098 Or gdb can be used: $ echo "list *write_sysrq_trigger+0x51" |gdb ~/k/vmlinux |grep "is in" (gdb) 0xffffffff815b5d83 is in driver_probe_device (/home/jpoimboe/git/linux/drivers/base/dd.c:378). (But note that when there are duplicate symbol names, gdb will only show the first symbol it finds. faddr2line is recommended over gdb because it handles duplicates and it also does function size checking.) Here's an example of what a stack dump looks like after this change: BUG: unable to handle kernel NULL pointer dereference at (null) IP: sysrq_handle_crash+0x45/0x80 PGD 36bfa067 [ 29.650644] PUD 7aca3067 50970] Oops: 0002 [#1] PREEMPT SMP Modules linked in: ... CPU: 1 PID: 786 Comm: bash Tainted: G E 4.9.0-rc1+ #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24 04/01/2014 task: ffff880078582a40 task.stack: ffffc90000ba8000 RIP: 0010:sysrq_handle_crash+0x45/0x80 RSP: 0018:ffffc90000babdc8 EFLAGS: 00010296 RAX: ffff880078582a40 RBX: 0000000000000063 RCX: 0000000000000001 RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000292 RBP: ffffc90000babdc8 R08: 0000000b31866061 R09: 0000000000000000 R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000007 R14: ffffffff81ee8680 R15: 0000000000000000 FS: 00007ffb43869700(0000) GS:ffff88007d400000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 000000007a3e9000 CR4: 00000000001406e0 Stack: ffffc90000babe00 ffffffff81572d08 ffffffff81572bd5 0000000000000002 0000000000000000 ffff880079606600 00007ffb4386e000 ffffc90000babe20 ffffffff81573201 ffff880036a3fd00 fffffffffffffffb ffffc90000babe40 Call Trace: __handle_sysrq+0x138/0x220 ? __handle_sysrq+0x5/0x220 write_sysrq_trigger+0x51/0x60 proc_reg_write+0x42/0x70 __vfs_write+0x37/0x140 ? preempt_count_sub+0xa1/0x100 ? __sb_start_write+0xf5/0x210 ? vfs_write+0x183/0x1a0 vfs_write+0xb8/0x1a0 SyS_write+0x58/0xc0 entry_SYSCALL_64_fastpath+0x1f/0xc2 RIP: 0033:0x7ffb42f55940 RSP: 002b:00007ffd33bb6b18 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000046 RCX: 00007ffb42f55940 RDX: 0000000000000002 RSI: 00007ffb4386e000 RDI: 0000000000000001 RBP: 0000000000000011 R08: 00007ffb4321ea40 R09: 00007ffb43869700 R10: 00007ffb43869700 R11: 0000000000000246 R12: 0000000000778a10 R13: 00007ffd33bb5c00 R14: 0000000000000007 R15: 0000000000000010 Code: 34 e8 d0 34 bc ff 48 c7 c2 3b 2b 57 81 be 01 00 00 00 48 c7 c7 e0 dd e5 81 e8 a8 55 ba ff c7 05 0e 3f de 00 01 00 00 00 0f ae f8 <c6> 04 25 00 00 00 00 01 5d c3 e8 4c 49 bc ff 84 c0 75 c3 48 c7 RIP: sysrq_handle_crash+0x45/0x80 RSP: ffffc90000babdc8 CR2: 0000000000000000 Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- arch/x86/include/asm/kdebug.h | 1 - arch/x86/kernel/dumpstack.c | 18 ++++-------------- arch/x86/kernel/process_32.c | 7 +++---- arch/x86/kernel/process_64.c | 6 +++--- arch/x86/mm/fault.c | 3 +-- arch/x86/platform/uv/uv_nmi.c | 4 ++-- 6 files changed, 13 insertions(+), 26 deletions(-) diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h index d318811..29a594a 100644 --- a/arch/x86/include/asm/kdebug.h +++ b/arch/x86/include/asm/kdebug.h @@ -21,7 +21,6 @@ enum die_val { DIE_NMIUNKNOWN, }; -extern void printk_address(unsigned long address); extern void die(const char *, struct pt_regs *,long); extern int __must_check __die(const char *, struct pt_regs *, long); extern void show_stack_regs(struct pt_regs *regs); diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 64281a1..f967652 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -46,14 +46,7 @@ static void printk_stack_address(unsigned long address, int reliable, char *log_lvl) { touch_nmi_watchdog(); - printk("%s [<%p>] %s%pB\n", - log_lvl, (void *)address, reliable ? "" : "? ", - (void *)address); -} - -void printk_address(unsigned long address) -{ - pr_cont(" [<%p>] %pS\n", (void *)address, (void *)address); + printk("%s %s%pB\n", log_lvl, reliable ? "" : "? ", (void *)address); } void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, @@ -275,14 +268,11 @@ int __die(const char *str, struct pt_regs *regs, long err) sp = kernel_stack_pointer(regs); savesegment(ss, ss); } - printk(KERN_EMERG "EIP: [<%08lx>] ", regs->ip); - print_symbol("%s", regs->ip); - printk(" SS:ESP %04x:%08lx\n", ss, sp); + printk(KERN_EMERG "EIP: %pS SS:ESP: %04x:%08lx\n", + (void *)regs->ip, ss, sp); #else /* Executive summary in case the oops scrolled away */ - printk(KERN_ALERT "RIP "); - printk_address(regs->ip); - printk(" RSP <%016lx>\n", regs->sp); + printk(KERN_ALERT "RIP: %pS RSP: %016lx\n", (void *)regs->ip, regs->sp); #endif return 0; } diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 7dc8c9c..f854404 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -72,10 +72,9 @@ void __show_regs(struct pt_regs *regs, int all) savesegment(gs, gs); } - printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n", - (u16)regs->cs, regs->ip, regs->flags, - smp_processor_id()); - print_symbol("EIP is at %s\n", regs->ip); + printk(KERN_DEFAULT "EIP: %pS\n", (void *)regs->ip); + printk(KERN_DEFAULT "EFLAGS: %08lx CPU: %d\n", regs->flags, + smp_processor_id()); printk(KERN_DEFAULT "EAX: %08lx EBX: %08lx ECX: %08lx EDX: %08lx\n", regs->ax, regs->bx, regs->cx, regs->dx); diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 4481e72..6c1b43e 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -61,10 +61,10 @@ void __show_regs(struct pt_regs *regs, int all) unsigned int fsindex, gsindex; unsigned int ds, cs, es; - printk(KERN_DEFAULT "RIP: %04lx:[<%016lx>] %pS\n", regs->cs & 0xffff, - regs->ip, (void *)regs->ip); + printk(KERN_DEFAULT "RIP: %04lx:%pS\n", regs->cs & 0xffff, + (void *)regs->ip); printk(KERN_DEFAULT "RSP: %04lx:%016lx EFLAGS: %08lx", regs->ss, - regs->sp, regs->flags); + regs->sp, regs->flags); if (regs->orig_ax != -1) pr_cont(" ORIG_RAX: %016lx\n", regs->orig_ax); else diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 9f72ca3..17c55a5 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -679,8 +679,7 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, printk(KERN_CONT "paging request"); printk(KERN_CONT " at %p\n", (void *) address); - printk(KERN_ALERT "IP:"); - printk_address(regs->ip); + printk(KERN_ALERT "IP: %pS\n", (void *)regs->ip); dump_pagetable(address); } diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c index cd5173a..8410e7d 100644 --- a/arch/x86/platform/uv/uv_nmi.c +++ b/arch/x86/platform/uv/uv_nmi.c @@ -387,8 +387,8 @@ static void uv_nmi_dump_cpu_ip_hdr(void) /* Dump Instruction Pointer info */ static void uv_nmi_dump_cpu_ip(int cpu, struct pt_regs *regs) { - pr_info("UV: %4d %6d %-32.32s ", cpu, current->pid, current->comm); - printk_address(regs->ip); + pr_info("UV: %4d %6d %-32.32s %pS", + cpu, current->pid, current->comm, (void *)regs->ip); } /* -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:x86/asm] x86/dumpstack: Remove kernel text addresses from stack dump 2016-10-25 14:51 ` [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump Josh Poimboeuf @ 2016-10-26 5:40 ` tip-bot for Josh Poimboeuf 2016-11-25 12:26 ` [PATCH 2/4] x86/dumpstack: remove " Kirill A. Shutemov 1 sibling, 0 replies; 17+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-10-26 5:40 UTC (permalink / raw) To: linux-tip-commits Cc: hpa, mingo, luto, tglx, peterz, jpoimboe, torvalds, bp, linux-kernel, brgerst, dvlasenk Commit-ID: bb5e5ce545f2031c96f7901cd8d1698ea3ca4c9c Gitweb: http://git.kernel.org/tip/bb5e5ce545f2031c96f7901cd8d1698ea3ca4c9c Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Tue, 25 Oct 2016 09:51:12 -0500 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 25 Oct 2016 18:40:37 +0200 x86/dumpstack: Remove kernel text addresses from stack dump Printing kernel text addresses in stack dumps is of questionable value, especially now that address randomization is becoming common. It can be a security issue because it leaks kernel addresses. It also affects the usefulness of the stack dump. Linus says: "I actually spend time cleaning up commit messages in logs, because useless data that isn't actually information (random hex numbers) is actively detrimental. It makes commit logs less legible. It also makes it harder to parse dumps. It's not useful. That makes it actively bad. I probably look at more oops reports than most people. I have not found the hex numbers useful for the last five years, because they are just randomized crap. The stack content thing just makes code scroll off the screen etc, for example." The only real downside to removing these addresses is that they can be used to disambiguate duplicate symbol names. However such cases are rare, and the context of the stack dump should be enough to be able to figure it out. There's now a 'faddr2line' script which can be used to convert a function address to a file name and line: $ ./scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 write_sysrq_trigger+0x51/0x60: write_sysrq_trigger at drivers/tty/sysrq.c:1098 Or gdb can be used: $ echo "list *write_sysrq_trigger+0x51" |gdb ~/k/vmlinux |grep "is in" (gdb) 0xffffffff815b5d83 is in driver_probe_device (/home/jpoimboe/git/linux/drivers/base/dd.c:378). (But note that when there are duplicate symbol names, gdb will only show the first symbol it finds. faddr2line is recommended over gdb because it handles duplicates and it also does function size checking.) Here's an example of what a stack dump looks like after this change: BUG: unable to handle kernel NULL pointer dereference at (null) IP: sysrq_handle_crash+0x45/0x80 PGD 36bfa067 [ 29.650644] PUD 7aca3067 Oops: 0002 [#1] PREEMPT SMP Modules linked in: ... CPU: 1 PID: 786 Comm: bash Tainted: G E 4.9.0-rc1+ #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24 04/01/2014 task: ffff880078582a40 task.stack: ffffc90000ba8000 RIP: 0010:sysrq_handle_crash+0x45/0x80 RSP: 0018:ffffc90000babdc8 EFLAGS: 00010296 RAX: ffff880078582a40 RBX: 0000000000000063 RCX: 0000000000000001 RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000292 RBP: ffffc90000babdc8 R08: 0000000b31866061 R09: 0000000000000000 R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000007 R14: ffffffff81ee8680 R15: 0000000000000000 FS: 00007ffb43869700(0000) GS:ffff88007d400000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 000000007a3e9000 CR4: 00000000001406e0 Stack: ffffc90000babe00 ffffffff81572d08 ffffffff81572bd5 0000000000000002 0000000000000000 ffff880079606600 00007ffb4386e000 ffffc90000babe20 ffffffff81573201 ffff880036a3fd00 fffffffffffffffb ffffc90000babe40 Call Trace: __handle_sysrq+0x138/0x220 ? __handle_sysrq+0x5/0x220 write_sysrq_trigger+0x51/0x60 proc_reg_write+0x42/0x70 __vfs_write+0x37/0x140 ? preempt_count_sub+0xa1/0x100 ? __sb_start_write+0xf5/0x210 ? vfs_write+0x183/0x1a0 vfs_write+0xb8/0x1a0 SyS_write+0x58/0xc0 entry_SYSCALL_64_fastpath+0x1f/0xc2 RIP: 0033:0x7ffb42f55940 RSP: 002b:00007ffd33bb6b18 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000046 RCX: 00007ffb42f55940 RDX: 0000000000000002 RSI: 00007ffb4386e000 RDI: 0000000000000001 RBP: 0000000000000011 R08: 00007ffb4321ea40 R09: 00007ffb43869700 R10: 00007ffb43869700 R11: 0000000000000246 R12: 0000000000778a10 R13: 00007ffd33bb5c00 R14: 0000000000000007 R15: 0000000000000010 Code: 34 e8 d0 34 bc ff 48 c7 c2 3b 2b 57 81 be 01 00 00 00 48 c7 c7 e0 dd e5 81 e8 a8 55 ba ff c7 05 0e 3f de 00 01 00 00 00 0f ae f8 <c6> 04 25 00 00 00 00 01 5d c3 e8 4c 49 bc ff 84 c0 75 c3 48 c7 RIP: sysrq_handle_crash+0x45/0x80 RSP: ffffc90000babdc8 CR2: 0000000000000000 Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/69329cb29b8f324bb5fcea14d61d224807fb6488.1477405374.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/kdebug.h | 1 - arch/x86/kernel/dumpstack.c | 18 ++++-------------- arch/x86/kernel/process_32.c | 7 +++---- arch/x86/kernel/process_64.c | 6 +++--- arch/x86/mm/fault.c | 3 +-- arch/x86/platform/uv/uv_nmi.c | 4 ++-- 6 files changed, 13 insertions(+), 26 deletions(-) diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h index d318811..29a594a 100644 --- a/arch/x86/include/asm/kdebug.h +++ b/arch/x86/include/asm/kdebug.h @@ -21,7 +21,6 @@ enum die_val { DIE_NMIUNKNOWN, }; -extern void printk_address(unsigned long address); extern void die(const char *, struct pt_regs *,long); extern int __must_check __die(const char *, struct pt_regs *, long); extern void show_stack_regs(struct pt_regs *regs); diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 64281a1..f967652 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -46,14 +46,7 @@ static void printk_stack_address(unsigned long address, int reliable, char *log_lvl) { touch_nmi_watchdog(); - printk("%s [<%p>] %s%pB\n", - log_lvl, (void *)address, reliable ? "" : "? ", - (void *)address); -} - -void printk_address(unsigned long address) -{ - pr_cont(" [<%p>] %pS\n", (void *)address, (void *)address); + printk("%s %s%pB\n", log_lvl, reliable ? "" : "? ", (void *)address); } void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, @@ -275,14 +268,11 @@ int __die(const char *str, struct pt_regs *regs, long err) sp = kernel_stack_pointer(regs); savesegment(ss, ss); } - printk(KERN_EMERG "EIP: [<%08lx>] ", regs->ip); - print_symbol("%s", regs->ip); - printk(" SS:ESP %04x:%08lx\n", ss, sp); + printk(KERN_EMERG "EIP: %pS SS:ESP: %04x:%08lx\n", + (void *)regs->ip, ss, sp); #else /* Executive summary in case the oops scrolled away */ - printk(KERN_ALERT "RIP "); - printk_address(regs->ip); - printk(" RSP <%016lx>\n", regs->sp); + printk(KERN_ALERT "RIP: %pS RSP: %016lx\n", (void *)regs->ip, regs->sp); #endif return 0; } diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index bd7be8e..e3223bc 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -72,10 +72,9 @@ void __show_regs(struct pt_regs *regs, int all) savesegment(gs, gs); } - printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n", - (u16)regs->cs, regs->ip, regs->flags, - smp_processor_id()); - print_symbol("EIP is at %s\n", regs->ip); + printk(KERN_DEFAULT "EIP: %pS\n", (void *)regs->ip); + printk(KERN_DEFAULT "EFLAGS: %08lx CPU: %d\n", regs->flags, + smp_processor_id()); printk(KERN_DEFAULT "EAX: %08lx EBX: %08lx ECX: %08lx EDX: %08lx\n", regs->ax, regs->bx, regs->cx, regs->dx); diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index f1c36da..c99f1ca 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -61,10 +61,10 @@ void __show_regs(struct pt_regs *regs, int all) unsigned int fsindex, gsindex; unsigned int ds, cs, es; - printk(KERN_DEFAULT "RIP: %04lx:[<%016lx>] %pS\n", regs->cs & 0xffff, - regs->ip, (void *)regs->ip); + printk(KERN_DEFAULT "RIP: %04lx:%pS\n", regs->cs & 0xffff, + (void *)regs->ip); printk(KERN_DEFAULT "RSP: %04lx:%016lx EFLAGS: %08lx", regs->ss, - regs->sp, regs->flags); + regs->sp, regs->flags); if (regs->orig_ax != -1) pr_cont(" ORIG_RAX: %016lx\n", regs->orig_ax); else diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 9f72ca3..17c55a5 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -679,8 +679,7 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, printk(KERN_CONT "paging request"); printk(KERN_CONT " at %p\n", (void *) address); - printk(KERN_ALERT "IP:"); - printk_address(regs->ip); + printk(KERN_ALERT "IP: %pS\n", (void *)regs->ip); dump_pagetable(address); } diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c index cd5173a..8410e7d 100644 --- a/arch/x86/platform/uv/uv_nmi.c +++ b/arch/x86/platform/uv/uv_nmi.c @@ -387,8 +387,8 @@ static void uv_nmi_dump_cpu_ip_hdr(void) /* Dump Instruction Pointer info */ static void uv_nmi_dump_cpu_ip(int cpu, struct pt_regs *regs) { - pr_info("UV: %4d %6d %-32.32s ", cpu, current->pid, current->comm); - printk_address(regs->ip); + pr_info("UV: %4d %6d %-32.32s %pS", + cpu, current->pid, current->comm, (void *)regs->ip); } /* ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump 2016-10-25 14:51 ` [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump Josh Poimboeuf 2016-10-26 5:40 ` [tip:x86/asm] x86/dumpstack: Remove " tip-bot for Josh Poimboeuf @ 2016-11-25 12:26 ` Kirill A. Shutemov 2016-11-28 20:49 ` Josh Poimboeuf 1 sibling, 1 reply; 17+ messages in thread From: Kirill A. Shutemov @ 2016-11-25 12:26 UTC (permalink / raw) To: Josh Poimboeuf Cc: x86, linux-kernel, Linus Torvalds, Sasha Levin, Konstantin Khlebnikov On Tue, Oct 25, 2016 at 09:51:12AM -0500, Josh Poimboeuf wrote: > Printing kernel text addresses in stack dumps is of questionable value, > especially now that address randomization is becoming common. > > It can be a security issue because it leaks kernel addresses. It also > affects the usefulness of the stack dump. Linus says: > > "I actually spend time cleaning up commit messages in logs, because > useless data that isn't actually information (random hex numbers) is > actively detrimental. > > It makes commit logs less legible. > > It also makes it harder to parse dumps. > > It's not useful. That makes it actively bad. > > I probably look at more oops reports than most people. I have not > found the hex numbers useful for the last five years, because they are > just randomized crap. > > The stack content thing just makes code scroll off the screen etc, for > example." > > The only real downside to removing these addresses is that they can be > used to disambiguate duplicate symbol names. However such cases are > rare, and the context of the stack dump should be enough to be able to > figure it out. > > There's now a 'faddr2line' script which can be used to convert a > function address to a file name and line: > > $ ./scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 > write_sysrq_trigger+0x51/0x60: > write_sysrq_trigger at drivers/tty/sysrq.c:1098 > > Or gdb can be used: > > $ echo "list *write_sysrq_trigger+0x51" |gdb ~/k/vmlinux |grep "is in" > (gdb) 0xffffffff815b5d83 is in driver_probe_device (/home/jpoimboe/git/linux/drivers/base/dd.c:378). > > (But note that when there are duplicate symbol names, gdb will only show > the first symbol it finds. faddr2line is recommended over gdb because > it handles duplicates and it also does function size checking.) The commit breaks scripts/decode_stacktrace.sh. Not sure if it's possible to fix it only on decode_stacktrace.sh side: we seems don't have a way to clearly distinguish stack trace line of any other. May be we should mark stack lines with some prefix to simplify decoding? -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump 2016-11-25 12:26 ` [PATCH 2/4] x86/dumpstack: remove " Kirill A. Shutemov @ 2016-11-28 20:49 ` Josh Poimboeuf 2016-11-28 22:27 ` Kirill A. Shutemov 0 siblings, 1 reply; 17+ messages in thread From: Josh Poimboeuf @ 2016-11-28 20:49 UTC (permalink / raw) To: Kirill A. Shutemov Cc: x86, linux-kernel, Linus Torvalds, Sasha Levin, Konstantin Khlebnikov On Fri, Nov 25, 2016 at 03:26:04PM +0300, Kirill A. Shutemov wrote: > On Tue, Oct 25, 2016 at 09:51:12AM -0500, Josh Poimboeuf wrote: > > Printing kernel text addresses in stack dumps is of questionable value, > > especially now that address randomization is becoming common. > > > > It can be a security issue because it leaks kernel addresses. It also > > affects the usefulness of the stack dump. Linus says: > > > > "I actually spend time cleaning up commit messages in logs, because > > useless data that isn't actually information (random hex numbers) is > > actively detrimental. > > > > It makes commit logs less legible. > > > > It also makes it harder to parse dumps. > > > > It's not useful. That makes it actively bad. > > > > I probably look at more oops reports than most people. I have not > > found the hex numbers useful for the last five years, because they are > > just randomized crap. > > > > The stack content thing just makes code scroll off the screen etc, for > > example." > > > > The only real downside to removing these addresses is that they can be > > used to disambiguate duplicate symbol names. However such cases are > > rare, and the context of the stack dump should be enough to be able to > > figure it out. > > > > There's now a 'faddr2line' script which can be used to convert a > > function address to a file name and line: > > > > $ ./scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 > > write_sysrq_trigger+0x51/0x60: > > write_sysrq_trigger at drivers/tty/sysrq.c:1098 > > > > Or gdb can be used: > > > > $ echo "list *write_sysrq_trigger+0x51" |gdb ~/k/vmlinux |grep "is in" > > (gdb) 0xffffffff815b5d83 is in driver_probe_device (/home/jpoimboe/git/linux/drivers/base/dd.c:378). > > > > (But note that when there are duplicate symbol names, gdb will only show > > the first symbol it finds. faddr2line is recommended over gdb because > > it handles duplicates and it also does function size checking.) > > The commit breaks scripts/decode_stacktrace.sh. > > Not sure if it's possible to fix it only on decode_stacktrace.sh side: we > seems don't have a way to clearly distinguish stack trace line of any > other. How about this bash regex? Seems to work for me with no false positives. [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]] -- Josh ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump 2016-11-28 20:49 ` Josh Poimboeuf @ 2016-11-28 22:27 ` Kirill A. Shutemov 2016-11-28 23:06 ` [PATCH] decode_stacktrace: fix address line detection on x86 Josh Poimboeuf 0 siblings, 1 reply; 17+ messages in thread From: Kirill A. Shutemov @ 2016-11-28 22:27 UTC (permalink / raw) To: Josh Poimboeuf Cc: x86, linux-kernel, Linus Torvalds, Sasha Levin, Konstantin Khlebnikov On Mon, Nov 28, 2016 at 02:49:58PM -0600, Josh Poimboeuf wrote: > On Fri, Nov 25, 2016 at 03:26:04PM +0300, Kirill A. Shutemov wrote: > > On Tue, Oct 25, 2016 at 09:51:12AM -0500, Josh Poimboeuf wrote: > > > Printing kernel text addresses in stack dumps is of questionable value, > > > especially now that address randomization is becoming common. > > > > > > It can be a security issue because it leaks kernel addresses. It also > > > affects the usefulness of the stack dump. Linus says: > > > > > > "I actually spend time cleaning up commit messages in logs, because > > > useless data that isn't actually information (random hex numbers) is > > > actively detrimental. > > > > > > It makes commit logs less legible. > > > > > > It also makes it harder to parse dumps. > > > > > > It's not useful. That makes it actively bad. > > > > > > I probably look at more oops reports than most people. I have not > > > found the hex numbers useful for the last five years, because they are > > > just randomized crap. > > > > > > The stack content thing just makes code scroll off the screen etc, for > > > example." > > > > > > The only real downside to removing these addresses is that they can be > > > used to disambiguate duplicate symbol names. However such cases are > > > rare, and the context of the stack dump should be enough to be able to > > > figure it out. > > > > > > There's now a 'faddr2line' script which can be used to convert a > > > function address to a file name and line: > > > > > > $ ./scripts/faddr2line ~/k/vmlinux write_sysrq_trigger+0x51/0x60 > > > write_sysrq_trigger+0x51/0x60: > > > write_sysrq_trigger at drivers/tty/sysrq.c:1098 > > > > > > Or gdb can be used: > > > > > > $ echo "list *write_sysrq_trigger+0x51" |gdb ~/k/vmlinux |grep "is in" > > > (gdb) 0xffffffff815b5d83 is in driver_probe_device (/home/jpoimboe/git/linux/drivers/base/dd.c:378). > > > > > > (But note that when there are duplicate symbol names, gdb will only show > > > the first symbol it finds. faddr2line is recommended over gdb because > > > it handles duplicates and it also does function size checking.) > > > > The commit breaks scripts/decode_stacktrace.sh. > > > > Not sure if it's possible to fix it only on decode_stacktrace.sh side: we > > seems don't have a way to clearly distinguish stack trace line of any > > other. > > How about this bash regex? Seems to work for me with no false > positives. > > [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]] Seems works fine to me. Thanks. Feel free to use my tested-by. -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] decode_stacktrace: fix address line detection on x86 2016-11-28 22:27 ` Kirill A. Shutemov @ 2016-11-28 23:06 ` Josh Poimboeuf 2016-11-29 7:13 ` [tip:x86/urgent] tools/decode_stacktrace.sh: Fix " tip-bot for Josh Poimboeuf 2016-11-29 13:24 ` [tip:x86/asm] scripts/decode_stacktrace.sh: " tip-bot for Josh Poimboeuf 0 siblings, 2 replies; 17+ messages in thread From: Josh Poimboeuf @ 2016-11-28 23:06 UTC (permalink / raw) To: Kirill A. Shutemov Cc: x86, linux-kernel, Linus Torvalds, Sasha Levin, Konstantin Khlebnikov Kirill reported that the decode_stacktrace.sh script was broken by the following commit: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump") Fix it by updating the per-line absolute address check to also check for function-based address lines like the following: write_sysrq_trigger+0x51/0x60 I didn't remove the check for absolute addresses because it's still needed for ARM. Fixes: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump") Reported-by: "Kirill A. Shutemov" <kirill@shutemov.name> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- scripts/decode_stacktrace.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh index c332684..5206d99 100755 --- a/scripts/decode_stacktrace.sh +++ b/scripts/decode_stacktrace.sh @@ -139,7 +139,8 @@ handle_line() { while read line; do # Let's see if we have an address in the line - if [[ $line =~ \[\<([^]]+)\>\] ]]; then + if [[ $line =~ \[\<([^]]+)\>\] ]] || + [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]]; then # Translate address to line numbers handle_line "$line" # Is it a code line? -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86 2016-11-28 23:06 ` [PATCH] decode_stacktrace: fix address line detection on x86 Josh Poimboeuf @ 2016-11-29 7:13 ` tip-bot for Josh Poimboeuf 2016-11-29 13:06 ` Josh Poimboeuf 2016-11-29 13:24 ` [tip:x86/asm] scripts/decode_stacktrace.sh: " tip-bot for Josh Poimboeuf 1 sibling, 1 reply; 17+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-11-29 7:13 UTC (permalink / raw) To: linux-tip-commits Cc: peterz, mingo, koct9i, torvalds, alexander.levin, linux-kernel, jpoimboe, kirill, hpa, tglx Commit-ID: 8e8d8725d46d93ceffd3e708d905bc101a1905b5 Gitweb: http://git.kernel.org/tip/8e8d8725d46d93ceffd3e708d905bc101a1905b5 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Mon, 28 Nov 2016 17:06:35 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 29 Nov 2016 08:10:05 +0100 tools/decode_stacktrace.sh: Fix address line detection on x86 Kirill reported that the decode_stacktrace.sh script was broken by the following commit: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump") Fix it by updating the per-line absolute address check to also check for function-based address lines like the following: write_sysrq_trigger+0x51/0x60 I didn't remove the check for absolute addresses because it's still needed for ARM. Reported-by: Kirill A. Shutemov <kirill@shutemov.name> Tested-by: Kirill A. Shutemov <kirill@shutemov.name> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Konstantin Khlebnikov <koct9i@gmail.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Sasha Levin <alexander.levin@verizon.com> Cc: Thomas Gleixner <tglx@linutronix.de> Fixes: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump") Link: http://lkml.kernel.org/r/20161128230635.4n2ofgawltgexgcg@treble Signed-off-by: Ingo Molnar <mingo@kernel.org> --- scripts/decode_stacktrace.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh index c332684..5206d99 100755 --- a/scripts/decode_stacktrace.sh +++ b/scripts/decode_stacktrace.sh @@ -139,7 +139,8 @@ handle_line() { while read line; do # Let's see if we have an address in the line - if [[ $line =~ \[\<([^]]+)\>\] ]]; then + if [[ $line =~ \[\<([^]]+)\>\] ]] || + [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]]; then # Translate address to line numbers handle_line "$line" # Is it a code line? ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86 2016-11-29 7:13 ` [tip:x86/urgent] tools/decode_stacktrace.sh: Fix " tip-bot for Josh Poimboeuf @ 2016-11-29 13:06 ` Josh Poimboeuf 2016-11-29 13:20 ` Ingo Molnar 0 siblings, 1 reply; 17+ messages in thread From: Josh Poimboeuf @ 2016-11-29 13:06 UTC (permalink / raw) To: tip-bot for Josh Poimboeuf Cc: linux-tip-commits, peterz, mingo, koct9i, torvalds, alexander.levin, linux-kernel, kirill, hpa, tglx Hi tip-bot, On Mon, Nov 28, 2016 at 11:13:15PM -0800, tip-bot for Josh Poimboeuf wrote: > Commit-ID: 8e8d8725d46d93ceffd3e708d905bc101a1905b5 > Gitweb: http://git.kernel.org/tip/8e8d8725d46d93ceffd3e708d905bc101a1905b5 > Author: Josh Poimboeuf <jpoimboe@redhat.com> > AuthorDate: Mon, 28 Nov 2016 17:06:35 -0600 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Tue, 29 Nov 2016 08:10:05 +0100 > > tools/decode_stacktrace.sh: Fix address line detection on x86 It's actually in the scripts subdir, so: s/tools/scripts/ Also, while it should be harmless for this to be in 'urgent', I think it isn't necessary because it fixes a commit which is currently only in tip. -- Josh ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86 2016-11-29 13:06 ` Josh Poimboeuf @ 2016-11-29 13:20 ` Ingo Molnar 0 siblings, 0 replies; 17+ messages in thread From: Ingo Molnar @ 2016-11-29 13:20 UTC (permalink / raw) To: Josh Poimboeuf Cc: tip-bot for Josh Poimboeuf, linux-tip-commits, peterz, koct9i, torvalds, alexander.levin, linux-kernel, kirill, hpa, tglx * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > Hi tip-bot, > > On Mon, Nov 28, 2016 at 11:13:15PM -0800, tip-bot for Josh Poimboeuf wrote: > > Commit-ID: 8e8d8725d46d93ceffd3e708d905bc101a1905b5 > > Gitweb: http://git.kernel.org/tip/8e8d8725d46d93ceffd3e708d905bc101a1905b5 > > Author: Josh Poimboeuf <jpoimboe@redhat.com> > > AuthorDate: Mon, 28 Nov 2016 17:06:35 -0600 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitDate: Tue, 29 Nov 2016 08:10:05 +0100 > > > > tools/decode_stacktrace.sh: Fix address line detection on x86 > > It's actually in the scripts subdir, so: s/tools/scripts/ So that was conscious - it might be in 'scripts' but it is really a tool! No strong feelings either way. > Also, while it should be harmless for this to be in 'urgent', I think it > isn't necessary because it fixes a commit which is currently only in > tip. I missed that. Ok, I rebased it over into tip:x86/asm and fixed the title as well while at it. Thanks, Ingo ^ permalink raw reply [flat|nested] 17+ messages in thread
* [tip:x86/asm] scripts/decode_stacktrace.sh: Fix address line detection on x86 2016-11-28 23:06 ` [PATCH] decode_stacktrace: fix address line detection on x86 Josh Poimboeuf 2016-11-29 7:13 ` [tip:x86/urgent] tools/decode_stacktrace.sh: Fix " tip-bot for Josh Poimboeuf @ 2016-11-29 13:24 ` tip-bot for Josh Poimboeuf 1 sibling, 0 replies; 17+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-11-29 13:24 UTC (permalink / raw) To: linux-tip-commits Cc: jpoimboe, alexander.levin, linux-kernel, kirill, koct9i, tglx, peterz, hpa, torvalds, mingo Commit-ID: 53938ee427bf27525a63721b7e25d86b8f31f161 Gitweb: http://git.kernel.org/tip/53938ee427bf27525a63721b7e25d86b8f31f161 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Mon, 28 Nov 2016 17:06:35 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 29 Nov 2016 14:19:50 +0100 scripts/decode_stacktrace.sh: Fix address line detection on x86 Kirill reported that the decode_stacktrace.sh script was broken by the following commit: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump") Fix it by updating the per-line absolute address check to also check for function-based address lines like the following: write_sysrq_trigger+0x51/0x60 I didn't remove the check for absolute addresses because it's still needed for ARM. Reported-by: Kirill A. Shutemov <kirill@shutemov.name> Tested-by: Kirill A. Shutemov <kirill@shutemov.name> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Konstantin Khlebnikov <koct9i@gmail.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Sasha Levin <alexander.levin@verizon.com> Cc: Thomas Gleixner <tglx@linutronix.de> Fixes: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump") Link: http://lkml.kernel.org/r/20161128230635.4n2ofgawltgexgcg@treble Signed-off-by: Ingo Molnar <mingo@kernel.org> --- scripts/decode_stacktrace.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh index c332684..5206d99 100755 --- a/scripts/decode_stacktrace.sh +++ b/scripts/decode_stacktrace.sh @@ -139,7 +139,8 @@ handle_line() { while read line; do # Let's see if we have an address in the line - if [[ $line =~ \[\<([^]]+)\>\] ]]; then + if [[ $line =~ \[\<([^]]+)\>\] ]] || + [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]]; then # Translate address to line numbers handle_line "$line" # Is it a code line? ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] x86/dumpstack: remove raw stack dump 2016-10-25 14:51 [PATCH 0/4] x86: fix kernel address printk exposures Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 1/4] scripts/faddr2line: fix "size mismatch" error Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump Josh Poimboeuf @ 2016-10-25 14:51 ` Josh Poimboeuf 2016-10-26 5:40 ` [tip:x86/asm] x86/dumpstack: Remove " tip-bot for Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 4/4] mm: remove kernel address exposure in free_reserved_area() Josh Poimboeuf 3 siblings, 1 reply; 17+ messages in thread From: Josh Poimboeuf @ 2016-10-25 14:51 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Linus Torvalds For mostly historical reasons, the x86 oops dump shows the raw stack values: ... [registers] Stack: ffff880079af7350 ffff880079905400 0000000000000000 ffffc900008f3ae0 ffffffffa0196610 0000000000000001 00010000ffffffff 0000000087654321 0000000000000002 0000000000000000 0000000000000000 0000000000000000 Call Trace: ... This seems to be an artifact from long ago, and probably isn't needed anymore. It generally just adds noise to the dump, and it can be actively harmful because it leaks kernel addresses. Linus says: "The stack dump actually goes back to forever, and it used to be useful back in 1992 or so. But it used to be useful mainly because stacks were simpler and we didn't have very good call traces anyway. I definitely remember having used them - I just do not remember having used them in the last ten+ years. Of course, it's still true that if you can trigger an oops, you've likely already lost the security game, but since the stack dump is so useless, let's aim to just remove it and make games like the above harder." This also removes the related 'kstack=' cmdline option and the 'kstack_depth_to_print' sysctl. Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- Documentation/kernel-parameters.txt | 3 -- Documentation/sysctl/kernel.txt | 8 ----- Documentation/x86/x86_64/boot-options.txt | 4 --- arch/x86/include/asm/stacktrace.h | 5 --- arch/x86/kernel/dumpstack.c | 21 ++---------- arch/x86/kernel/dumpstack_32.c | 33 +------------------ arch/x86/kernel/dumpstack_64.c | 53 +------------------------------ kernel/sysctl.c | 7 ---- 8 files changed, 4 insertions(+), 130 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 73de01e..def1874 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1952,9 +1952,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted. kmemcheck=2 (one-shot mode) Default: 2 (one-shot mode) - kstack=N [X86] Print N words from the kernel stack - in oops dumps. - kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs. Default is 0 (don't ignore, but inject #GP) diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt index ffab8b5..065f184 100644 --- a/Documentation/sysctl/kernel.txt +++ b/Documentation/sysctl/kernel.txt @@ -40,7 +40,6 @@ show up in /proc/sys/kernel: - hung_task_warnings - kexec_load_disabled - kptr_restrict -- kstack_depth_to_print [ X86 only ] - l2cr [ PPC only ] - modprobe ==> Documentation/debugging-modules.txt - modules_disabled @@ -395,13 +394,6 @@ When kptr_restrict is set to (2), kernel pointers printed using ============================================================== -kstack_depth_to_print: (X86 only) - -Controls the number of words to print when dumping the raw -kernel stack. - -============================================================== - l2cr: (PPC only) This flag controls the L2 cache of G3 processor boards. If diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt index 0965a71..61b611e 100644 --- a/Documentation/x86/x86_64/boot-options.txt +++ b/Documentation/x86/x86_64/boot-options.txt @@ -277,10 +277,6 @@ IOMMU (input/output memory management unit) space might stop working. Use this option if you have devices that are accessed from userspace directly on some PCI host bridge. -Debugging - - kstack=N Print N words from the kernel stack in oops dumps. - Miscellaneous nogbpages diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index 37f2e0b..1e375b0 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -43,8 +43,6 @@ static inline bool on_stack(struct stack_info *info, void *addr, size_t len) addr + len > begin && addr + len <= end); } -extern int kstack_depth_to_print; - #ifdef CONFIG_X86_32 #define STACKSLOTS_PER_LINE 8 #else @@ -86,9 +84,6 @@ get_stack_pointer(struct task_struct *task, struct pt_regs *regs) void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, unsigned long *stack, char *log_lvl); -void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, char *log_lvl); - extern unsigned int code_bytes; /* The form of the top of the frame on the stack */ diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index f967652..499aa6f 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -22,7 +22,6 @@ int panic_on_unrecovered_nmi; int panic_on_io_nmi; unsigned int code_bytes = 64; -int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE; static int die_counter; bool in_task_stack(unsigned long *stack, struct task_struct *task, @@ -171,12 +170,12 @@ void show_stack(struct task_struct *task, unsigned long *sp) if (!sp && task == current) sp = get_stack_pointer(current, NULL); - show_stack_log_lvl(task, NULL, sp, KERN_DEFAULT); + show_trace_log_lvl(task, NULL, sp, KERN_DEFAULT); } void show_stack_regs(struct pt_regs *regs) { - show_stack_log_lvl(current, regs, NULL, KERN_DEFAULT); + show_trace_log_lvl(current, regs, NULL, KERN_DEFAULT); } static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED; @@ -295,22 +294,6 @@ void die(const char *str, struct pt_regs *regs, long err) oops_end(flags, regs, sig); } -static int __init kstack_setup(char *s) -{ - ssize_t ret; - unsigned long val; - - if (!s) - return -EINVAL; - - ret = kstrtoul(s, 0, &val); - if (ret) - return ret; - kstack_depth_to_print = val; - return 0; -} -early_param("kstack", kstack_setup); - static int __init code_bytes_setup(char *s) { ssize_t ret; diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index 06eb322..90cf460 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -121,36 +121,6 @@ int get_stack_info(unsigned long *stack, struct task_struct *task, return -EINVAL; } -void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, char *log_lvl) -{ - unsigned long *stack; - int i; - - if (!try_get_task_stack(task)) - return; - - sp = sp ? : get_stack_pointer(task, regs); - - stack = sp; - for (i = 0; i < kstack_depth_to_print; i++) { - if (kstack_end(stack)) - break; - if ((i % STACKSLOTS_PER_LINE) == 0) { - if (i != 0) - pr_cont("\n"); - printk("%s %08lx", log_lvl, *stack++); - } else - pr_cont(" %08lx", *stack++); - touch_nmi_watchdog(); - } - pr_cont("\n"); - show_trace_log_lvl(task, regs, sp, log_lvl); - - put_task_stack(task); -} - - void show_regs(struct pt_regs *regs) { int i; @@ -168,8 +138,7 @@ void show_regs(struct pt_regs *regs) unsigned char c; u8 *ip; - pr_emerg("Stack:\n"); - show_stack_log_lvl(current, regs, NULL, KERN_EMERG); + show_trace_log_lvl(current, regs, NULL, KERN_EMERG); pr_emerg("Code:"); diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 36cf1a4..310abf4 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -140,56 +140,6 @@ int get_stack_info(unsigned long *stack, struct task_struct *task, return -EINVAL; } -void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, char *log_lvl) -{ - unsigned long *irq_stack_end; - unsigned long *irq_stack; - unsigned long *stack; - int i; - - if (!try_get_task_stack(task)) - return; - - irq_stack_end = (unsigned long *)this_cpu_read(irq_stack_ptr); - irq_stack = irq_stack_end - (IRQ_STACK_SIZE / sizeof(long)); - - sp = sp ? : get_stack_pointer(task, regs); - - stack = sp; - for (i = 0; i < kstack_depth_to_print; i++) { - unsigned long word; - - if (stack >= irq_stack && stack <= irq_stack_end) { - if (stack == irq_stack_end) { - stack = (unsigned long *) (irq_stack_end[-1]); - pr_cont(" <EOI> "); - } - } else { - if (kstack_end(stack)) - break; - } - - if (probe_kernel_address(stack, word)) - break; - - if ((i % STACKSLOTS_PER_LINE) == 0) { - if (i != 0) - pr_cont("\n"); - printk("%s %016lx", log_lvl, word); - } else - pr_cont(" %016lx", word); - - stack++; - touch_nmi_watchdog(); - } - - pr_cont("\n"); - show_trace_log_lvl(task, regs, sp, log_lvl); - - put_task_stack(task); -} - void show_regs(struct pt_regs *regs) { int i; @@ -207,8 +157,7 @@ void show_regs(struct pt_regs *regs) unsigned char c; u8 *ip; - printk(KERN_DEFAULT "Stack:\n"); - show_stack_log_lvl(current, regs, NULL, KERN_DEFAULT); + show_trace_log_lvl(current, regs, NULL, KERN_DEFAULT); printk(KERN_DEFAULT "Code: "); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 739fb17..39b3368 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -983,13 +983,6 @@ static struct ctl_table kern_table[] = { .proc_handler = proc_dointvec, }, { - .procname = "kstack_depth_to_print", - .data = &kstack_depth_to_print, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, - { .procname = "io_delay_type", .data = &io_delay_type, .maxlen = sizeof(int), -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:x86/asm] x86/dumpstack: Remove raw stack dump 2016-10-25 14:51 ` [PATCH 3/4] x86/dumpstack: remove raw stack dump Josh Poimboeuf @ 2016-10-26 5:40 ` tip-bot for Josh Poimboeuf 0 siblings, 0 replies; 17+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-10-26 5:40 UTC (permalink / raw) To: linux-tip-commits Cc: jpoimboe, peterz, luto, linux-kernel, bp, mingo, tglx, brgerst, hpa, dvlasenk, torvalds Commit-ID: 0ee1dd9f5e7eae4e55f95935b72d4beecb03de9c Gitweb: http://git.kernel.org/tip/0ee1dd9f5e7eae4e55f95935b72d4beecb03de9c Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Tue, 25 Oct 2016 09:51:13 -0500 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 25 Oct 2016 18:40:37 +0200 x86/dumpstack: Remove raw stack dump For mostly historical reasons, the x86 oops dump shows the raw stack values: ... [registers] Stack: ffff880079af7350 ffff880079905400 0000000000000000 ffffc900008f3ae0 ffffffffa0196610 0000000000000001 00010000ffffffff 0000000087654321 0000000000000002 0000000000000000 0000000000000000 0000000000000000 Call Trace: ... This seems to be an artifact from long ago, and probably isn't needed anymore. It generally just adds noise to the dump, and it can be actively harmful because it leaks kernel addresses. Linus says: "The stack dump actually goes back to forever, and it used to be useful back in 1992 or so. But it used to be useful mainly because stacks were simpler and we didn't have very good call traces anyway. I definitely remember having used them - I just do not remember having used them in the last ten+ years. Of course, it's still true that if you can trigger an oops, you've likely already lost the security game, but since the stack dump is so useless, let's aim to just remove it and make games like the above harder." This also removes the related 'kstack=' cmdline option and the 'kstack_depth_to_print' sysctl. Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/e83bd50df52d8fe88e94d2566426ae40d813bf8f.1477405374.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- Documentation/kernel-parameters.txt | 3 -- Documentation/sysctl/kernel.txt | 8 ----- Documentation/x86/x86_64/boot-options.txt | 4 --- arch/x86/include/asm/stacktrace.h | 5 --- arch/x86/kernel/dumpstack.c | 21 ++---------- arch/x86/kernel/dumpstack_32.c | 33 +------------------ arch/x86/kernel/dumpstack_64.c | 53 +------------------------------ kernel/sysctl.c | 7 ---- 8 files changed, 4 insertions(+), 130 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 37babf9..049a917 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1958,9 +1958,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted. kmemcheck=2 (one-shot mode) Default: 2 (one-shot mode) - kstack=N [X86] Print N words from the kernel stack - in oops dumps. - kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs. Default is 0 (don't ignore, but inject #GP) diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt index ffab8b5..065f184 100644 --- a/Documentation/sysctl/kernel.txt +++ b/Documentation/sysctl/kernel.txt @@ -40,7 +40,6 @@ show up in /proc/sys/kernel: - hung_task_warnings - kexec_load_disabled - kptr_restrict -- kstack_depth_to_print [ X86 only ] - l2cr [ PPC only ] - modprobe ==> Documentation/debugging-modules.txt - modules_disabled @@ -395,13 +394,6 @@ When kptr_restrict is set to (2), kernel pointers printed using ============================================================== -kstack_depth_to_print: (X86 only) - -Controls the number of words to print when dumping the raw -kernel stack. - -============================================================== - l2cr: (PPC only) This flag controls the L2 cache of G3 processor boards. If diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt index 0965a71..61b611e 100644 --- a/Documentation/x86/x86_64/boot-options.txt +++ b/Documentation/x86/x86_64/boot-options.txt @@ -277,10 +277,6 @@ IOMMU (input/output memory management unit) space might stop working. Use this option if you have devices that are accessed from userspace directly on some PCI host bridge. -Debugging - - kstack=N Print N words from the kernel stack in oops dumps. - Miscellaneous nogbpages diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index 37f2e0b..1e375b0 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -43,8 +43,6 @@ static inline bool on_stack(struct stack_info *info, void *addr, size_t len) addr + len > begin && addr + len <= end); } -extern int kstack_depth_to_print; - #ifdef CONFIG_X86_32 #define STACKSLOTS_PER_LINE 8 #else @@ -86,9 +84,6 @@ get_stack_pointer(struct task_struct *task, struct pt_regs *regs) void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, unsigned long *stack, char *log_lvl); -void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, char *log_lvl); - extern unsigned int code_bytes; /* The form of the top of the frame on the stack */ diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index f967652..499aa6f 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -22,7 +22,6 @@ int panic_on_unrecovered_nmi; int panic_on_io_nmi; unsigned int code_bytes = 64; -int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE; static int die_counter; bool in_task_stack(unsigned long *stack, struct task_struct *task, @@ -171,12 +170,12 @@ void show_stack(struct task_struct *task, unsigned long *sp) if (!sp && task == current) sp = get_stack_pointer(current, NULL); - show_stack_log_lvl(task, NULL, sp, KERN_DEFAULT); + show_trace_log_lvl(task, NULL, sp, KERN_DEFAULT); } void show_stack_regs(struct pt_regs *regs) { - show_stack_log_lvl(current, regs, NULL, KERN_DEFAULT); + show_trace_log_lvl(current, regs, NULL, KERN_DEFAULT); } static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED; @@ -295,22 +294,6 @@ void die(const char *str, struct pt_regs *regs, long err) oops_end(flags, regs, sig); } -static int __init kstack_setup(char *s) -{ - ssize_t ret; - unsigned long val; - - if (!s) - return -EINVAL; - - ret = kstrtoul(s, 0, &val); - if (ret) - return ret; - kstack_depth_to_print = val; - return 0; -} -early_param("kstack", kstack_setup); - static int __init code_bytes_setup(char *s) { ssize_t ret; diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index 06eb322..90cf460 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -121,36 +121,6 @@ unknown: return -EINVAL; } -void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, char *log_lvl) -{ - unsigned long *stack; - int i; - - if (!try_get_task_stack(task)) - return; - - sp = sp ? : get_stack_pointer(task, regs); - - stack = sp; - for (i = 0; i < kstack_depth_to_print; i++) { - if (kstack_end(stack)) - break; - if ((i % STACKSLOTS_PER_LINE) == 0) { - if (i != 0) - pr_cont("\n"); - printk("%s %08lx", log_lvl, *stack++); - } else - pr_cont(" %08lx", *stack++); - touch_nmi_watchdog(); - } - pr_cont("\n"); - show_trace_log_lvl(task, regs, sp, log_lvl); - - put_task_stack(task); -} - - void show_regs(struct pt_regs *regs) { int i; @@ -168,8 +138,7 @@ void show_regs(struct pt_regs *regs) unsigned char c; u8 *ip; - pr_emerg("Stack:\n"); - show_stack_log_lvl(current, regs, NULL, KERN_EMERG); + show_trace_log_lvl(current, regs, NULL, KERN_EMERG); pr_emerg("Code:"); diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 36cf1a4..310abf4 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -140,56 +140,6 @@ unknown: return -EINVAL; } -void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, char *log_lvl) -{ - unsigned long *irq_stack_end; - unsigned long *irq_stack; - unsigned long *stack; - int i; - - if (!try_get_task_stack(task)) - return; - - irq_stack_end = (unsigned long *)this_cpu_read(irq_stack_ptr); - irq_stack = irq_stack_end - (IRQ_STACK_SIZE / sizeof(long)); - - sp = sp ? : get_stack_pointer(task, regs); - - stack = sp; - for (i = 0; i < kstack_depth_to_print; i++) { - unsigned long word; - - if (stack >= irq_stack && stack <= irq_stack_end) { - if (stack == irq_stack_end) { - stack = (unsigned long *) (irq_stack_end[-1]); - pr_cont(" <EOI> "); - } - } else { - if (kstack_end(stack)) - break; - } - - if (probe_kernel_address(stack, word)) - break; - - if ((i % STACKSLOTS_PER_LINE) == 0) { - if (i != 0) - pr_cont("\n"); - printk("%s %016lx", log_lvl, word); - } else - pr_cont(" %016lx", word); - - stack++; - touch_nmi_watchdog(); - } - - pr_cont("\n"); - show_trace_log_lvl(task, regs, sp, log_lvl); - - put_task_stack(task); -} - void show_regs(struct pt_regs *regs) { int i; @@ -207,8 +157,7 @@ void show_regs(struct pt_regs *regs) unsigned char c; u8 *ip; - printk(KERN_DEFAULT "Stack:\n"); - show_stack_log_lvl(current, regs, NULL, KERN_DEFAULT); + show_trace_log_lvl(current, regs, NULL, KERN_DEFAULT); printk(KERN_DEFAULT "Code: "); diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 706309f..17a5a82 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -990,13 +990,6 @@ static struct ctl_table kern_table[] = { .proc_handler = proc_dointvec, }, { - .procname = "kstack_depth_to_print", - .data = &kstack_depth_to_print, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - }, - { .procname = "io_delay_type", .data = &io_delay_type, .maxlen = sizeof(int), ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] mm: remove kernel address exposure in free_reserved_area() 2016-10-25 14:51 [PATCH 0/4] x86: fix kernel address printk exposures Josh Poimboeuf ` (2 preceding siblings ...) 2016-10-25 14:51 ` [PATCH 3/4] x86/dumpstack: remove raw stack dump Josh Poimboeuf @ 2016-10-25 14:51 ` Josh Poimboeuf 2016-10-26 5:41 ` [tip:x86/asm] mm/page_alloc: Remove " tip-bot for Josh Poimboeuf 3 siblings, 1 reply; 17+ messages in thread From: Josh Poimboeuf @ 2016-10-25 14:51 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Linus Torvalds, linux-mm Linus suggested we try to remove some of the low-hanging fruit related to kernel address exposure in dmesg. The only leaks I see on my local system are: Freeing SMP alternatives memory: 32K (ffffffff9e309000 - ffffffff9e311000) Freeing initrd memory: 10588K (ffffa0b736b42000 - ffffa0b737599000) Freeing unused kernel memory: 3592K (ffffffff9df87000 - ffffffff9e309000) Freeing unused kernel memory: 1352K (ffffa0b7288ae000 - ffffa0b728a00000) Freeing unused kernel memory: 632K (ffffa0b728d62000 - ffffa0b728e00000) Linus says: "I suspect we should just remove [the addresses in the 'Freeing' messages]. I'm sure they are useful in theory, but I suspect they were more useful back when the whole "free init memory" was originally done. These days, if we have a use-after-free, I suspect the init-mem situation is the easiest situation by far. Compared to all the dynamic allocations which are much more likely to show it anyway. So having debug output for that case is likely not all that productive." With this patch the freeing messages now look like this: Freeing SMP alternatives memory: 32K Freeing initrd memory: 10588K Freeing unused kernel memory: 3592K Freeing unused kernel memory: 1352K Freeing unused kernel memory: 632K Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: linux-mm@kvack.org Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- mm/page_alloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 2b3bf67..3f63973 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -6508,8 +6508,8 @@ unsigned long free_reserved_area(void *start, void *end, int poison, char *s) } if (pages && s) - pr_info("Freeing %s memory: %ldK (%p - %p)\n", - s, pages << (PAGE_SHIFT - 10), start, end); + pr_info("Freeing %s memory: %ldK\n", + s, pages << (PAGE_SHIFT - 10)); return pages; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:x86/asm] mm/page_alloc: Remove kernel address exposure in free_reserved_area() 2016-10-25 14:51 ` [PATCH 4/4] mm: remove kernel address exposure in free_reserved_area() Josh Poimboeuf @ 2016-10-26 5:41 ` tip-bot for Josh Poimboeuf 0 siblings, 0 replies; 17+ messages in thread From: tip-bot for Josh Poimboeuf @ 2016-10-26 5:41 UTC (permalink / raw) To: linux-tip-commits Cc: hpa, dvlasenk, torvalds, tglx, linux-kernel, peterz, jpoimboe, luto, bp, mingo, brgerst Commit-ID: adb1fe9ae2ee6ef6bc10f3d5a588020e7664dfa7 Gitweb: http://git.kernel.org/tip/adb1fe9ae2ee6ef6bc10f3d5a588020e7664dfa7 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Tue, 25 Oct 2016 09:51:14 -0500 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 25 Oct 2016 18:40:37 +0200 mm/page_alloc: Remove kernel address exposure in free_reserved_area() Linus suggested we try to remove some of the low-hanging fruit related to kernel address exposure in dmesg. The only leaks I see on my local system are: Freeing SMP alternatives memory: 32K (ffffffff9e309000 - ffffffff9e311000) Freeing initrd memory: 10588K (ffffa0b736b42000 - ffffa0b737599000) Freeing unused kernel memory: 3592K (ffffffff9df87000 - ffffffff9e309000) Freeing unused kernel memory: 1352K (ffffa0b7288ae000 - ffffa0b728a00000) Freeing unused kernel memory: 632K (ffffa0b728d62000 - ffffa0b728e00000) Linus says: "I suspect we should just remove [the addresses in the 'Freeing' messages]. I'm sure they are useful in theory, but I suspect they were more useful back when the whole "free init memory" was originally done. These days, if we have a use-after-free, I suspect the init-mem situation is the easiest situation by far. Compared to all the dynamic allocations which are much more likely to show it anyway. So having debug output for that case is likely not all that productive." With this patch the freeing messages now look like this: Freeing SMP alternatives memory: 32K Freeing initrd memory: 10588K Freeing unused kernel memory: 3592K Freeing unused kernel memory: 1352K Freeing unused kernel memory: 632K Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: linux-mm@kvack.org Link: http://lkml.kernel.org/r/6836ff90c45b71d38e5d4405aec56fa9e5d1d4b2.1477405374.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- mm/page_alloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 2b3bf67..3f63973 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -6508,8 +6508,8 @@ unsigned long free_reserved_area(void *start, void *end, int poison, char *s) } if (pages && s) - pr_info("Freeing %s memory: %ldK (%p - %p)\n", - s, pages << (PAGE_SHIFT - 10), start, end); + pr_info("Freeing %s memory: %ldK\n", + s, pages << (PAGE_SHIFT - 10)); return pages; } ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-11-29 14:14 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-25 14:51 [PATCH 0/4] x86: fix kernel address printk exposures Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 1/4] scripts/faddr2line: fix "size mismatch" error Josh Poimboeuf 2016-10-26 5:39 ` [tip:x86/asm] scripts/faddr2line: Fix " tip-bot for Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump Josh Poimboeuf 2016-10-26 5:40 ` [tip:x86/asm] x86/dumpstack: Remove " tip-bot for Josh Poimboeuf 2016-11-25 12:26 ` [PATCH 2/4] x86/dumpstack: remove " Kirill A. Shutemov 2016-11-28 20:49 ` Josh Poimboeuf 2016-11-28 22:27 ` Kirill A. Shutemov 2016-11-28 23:06 ` [PATCH] decode_stacktrace: fix address line detection on x86 Josh Poimboeuf 2016-11-29 7:13 ` [tip:x86/urgent] tools/decode_stacktrace.sh: Fix " tip-bot for Josh Poimboeuf 2016-11-29 13:06 ` Josh Poimboeuf 2016-11-29 13:20 ` Ingo Molnar 2016-11-29 13:24 ` [tip:x86/asm] scripts/decode_stacktrace.sh: " tip-bot for Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 3/4] x86/dumpstack: remove raw stack dump Josh Poimboeuf 2016-10-26 5:40 ` [tip:x86/asm] x86/dumpstack: Remove " tip-bot for Josh Poimboeuf 2016-10-25 14:51 ` [PATCH 4/4] mm: remove kernel address exposure in free_reserved_area() Josh Poimboeuf 2016-10-26 5:41 ` [tip:x86/asm] mm/page_alloc: Remove " tip-bot for Josh Poimboeuf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).