Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On Mon, Oct 12, 2020 at 12:03:18PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-10-11 22:32:38 [+0100], Russell King - ARM Linux admin wrote: > > I don't have a problem getting rid of the hex numbers in [< >] > > although then I will need to convert the symbol back to an address > > using the vmlinux to then calculate its address to then find the > > appropriate place in the objdump output - because objdump does > > _not_ use the symbol+offset annotation. Yes, I really do look up > > the numeric addresses in the objdump output to then read the > > disassembly. > > > > $ objdump -d vmlinux | less > > > > and then search for the address is the fastest and most convenient > > way for me rather than having to deal with some random script. > > > > Maybe I'm just antequated about how I do my debugging, but this > > seems to me to be the most efficient and fastest way. > > besides what Josh mentioned, there is also > scripts/decode_stacktrace.sh path-vmlinux > > where you can copy/paste your complete stack trace / dmesg and it will > decode it line by line. So if you invoke > scripts/decode_stacktrace.sh vmlinux.o > > and paste this: > > |[7.568155] 001: PC is at do_work_pending+0x190/0x5c4 > |[7.568641] 001: LR is at slow_work_pending+0xc/0x20 > |[7.569232] 001: Backtrace: > |[7.569367] 001: [] (do_work_pending) from [] > (slow_work_pending+0xc/0x20) > > you get this in return: > |[7.568155] 001: PC is at do_work_pending (arch/arm/kernel/signal.c:616 > arch/arm/kernel/signal.c:670) > |[7.568641] 001: LR is at slow_work_pending > (arch/arm/kernel/entry-common.S:112) > |[7.569232] 001: Backtrace: > |[7.569367] 001: (do_work_pending) from slow_work_pending > (arch/arm/kernel/entry-common.S:112) That's assuming: 1) you have built the kernel with debug information enabled (I never do, it uses way too much disk space) 2) you want to look at the C code rather than the assembly. I think you've assumed that I debug oopses by looking primerily at C code. I don't. I understand the assembly and then look at the C code. I've stated in the past in detail how I debug the kernel when someone has posted a hard-to-debug oops, going through the validation of the dumped state, sometimes to find the bug in a function several parents up from the one where the oops actually occurred. However, as I've said, I have little problem removing the hex values inside [< >] as I can work around that. Removing the other information is what I'm objecting to. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On 2020-10-11 22:32:38 [+0100], Russell King - ARM Linux admin wrote: > I don't have a problem getting rid of the hex numbers in [< >] > although then I will need to convert the symbol back to an address > using the vmlinux to then calculate its address to then find the > appropriate place in the objdump output - because objdump does > _not_ use the symbol+offset annotation. Yes, I really do look up > the numeric addresses in the objdump output to then read the > disassembly. > > $ objdump -d vmlinux | less > > and then search for the address is the fastest and most convenient > way for me rather than having to deal with some random script. > > Maybe I'm just antequated about how I do my debugging, but this > seems to me to be the most efficient and fastest way. besides what Josh mentioned, there is also scripts/decode_stacktrace.sh path-vmlinux where you can copy/paste your complete stack trace / dmesg and it will decode it line by line. So if you invoke scripts/decode_stacktrace.sh vmlinux.o and paste this: |[7.568155] 001: PC is at do_work_pending+0x190/0x5c4 |[7.568641] 001: LR is at slow_work_pending+0xc/0x20 |[7.569232] 001: Backtrace: |[7.569367] 001: [] (do_work_pending) from [] (slow_work_pending+0xc/0x20) you get this in return: |[7.568155] 001: PC is at do_work_pending (arch/arm/kernel/signal.c:616 arch/arm/kernel/signal.c:670) |[7.568641] 001: LR is at slow_work_pending (arch/arm/kernel/entry-common.S:112) |[7.569232] 001: Backtrace: |[7.569367] 001: (do_work_pending) from slow_work_pending (arch/arm/kernel/entry-common.S:112) Sebastian
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On 2020/10/12 5:32, Russell King - ARM Linux admin wrote: On Fri, Oct 09, 2020 at 10:18:20AM +0200, Sebastian Andrzej Siewior wrote: On 2020-10-09 09:08:50 [+0100], Russell King - ARM Linux admin wrote: I am really not happy about this - it hurts at least my ability to debug the kernel when people post oopses to the mailing list. If people wish to make the kernel harder to debug, and are prepared to be told "your kernel is undebuggable" then this patch is fine. I haven't look at the patch but don't they keep/add the representation: PC: symbol+offset/size LR: symbol+offset/size ? This is needed at very least as a replacement for the missing address. I don't have a problem getting rid of the hex numbers in [< >] although then I will need to convert the symbol back to an address using the vmlinux to then calculate its address to then find the appropriate place in the objdump output - because objdump does _not_ use the symbol+offset annotation. Yes, I really do look up the numeric addresses in the objdump output to then read the disassembly. $ objdump -d vmlinux | less and then search for the address is the fastest and most convenient way for me rather than having to deal with some random script. Maybe I'm just antequated about how I do my debugging, but this seems to me to be the most efficient and fastest way. The loading address of the kernel module is not fixed, so symbol+offset is more useful than a hexadecimal address when the call stack contains kernel module symbols. Delete the PC/LR address and retain the sysbol+offset. The kernel can still be debugged. Thanks Xiaoming Ni
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On Fri, Oct 09, 2020 at 10:18:20AM +0200, Sebastian Andrzej Siewior wrote: > On 2020-10-09 09:08:50 [+0100], Russell King - ARM Linux admin wrote: > > I am really not happy about this - it hurts at least my ability to > > debug the kernel when people post oopses to the mailing list. If > > people wish to make the kernel harder to debug, and are prepared > > to be told "your kernel is undebuggable" then this patch is fine. > > I haven't look at the patch but don't they keep/add the representation: > PC: symbol+offset/size > LR: symbol+offset/size > > ? This is needed at very least as a replacement for the missing address. I don't have a problem getting rid of the hex numbers in [< >] although then I will need to convert the symbol back to an address using the vmlinux to then calculate its address to then find the appropriate place in the objdump output - because objdump does _not_ use the symbol+offset annotation. Yes, I really do look up the numeric addresses in the objdump output to then read the disassembly. $ objdump -d vmlinux | less and then search for the address is the fastest and most convenient way for me rather than having to deal with some random script. Maybe I'm just antequated about how I do my debugging, but this seems to me to be the most efficient and fastest way. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On Fri, Oct 09, 2020 at 09:08:50AM +0100, Russell King - ARM Linux admin wrote:
> On Fri, Oct 09, 2020 at 03:59:57PM +0800, Xiaoming Ni wrote:
> > Printing raw pointer values in backtraces has potential security
> > implications and are of questionable value anyway.
> >
> > This patch follows x86 and arm64's lead and removes the "Exception stack:"
> > dump from kernel backtraces:
> > commit a25ffd3a6302a6 ("arm64: traps: Don't print stack or raw
> > PC/LR values in backtraces")
> > commit 0ee1dd9f5e7eae ("x86/dumpstack: Remove raw stack dump")
> > commit bb5e5ce545f203 ("x86/dumpstack: Remove kernel text
> > addresses from stack dump")
> >
> > Signed-off-by: Xiaoming Ni
>
> I am really not happy about this - it hurts at least my ability to
> debug the kernel when people post oopses to the mailing list. If
> people wish to make the kernel harder to debug, and are prepared
> to be told "your kernel is undebuggable" then this patch is fine.
At least on x86 we've had this for four years now, without any apparent
harm to debugability. scripts/faddr2line helps.
--
Josh
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On 2020/10/9 16:18, Sebastian Andrzej Siewior wrote:
On 2020-10-09 09:08:50 [+0100], Russell King - ARM Linux admin wrote:
I am really not happy about this - it hurts at least my ability to
debug the kernel when people post oopses to the mailing list. If
In the reset scenario, dump_mem is retained:
@@ -125,6 +118,9 @@ static void dump_mem(const char *lvl, const char
*str, unsigned long bottom,
mm_segment_t fs;
int i;
+ /* Do not print virtual addresses in non-reset scenarios */
+ if (!panic_on_oops)
+ return;
people wish to make the kernel harder to debug, and are prepared
to be told "your kernel is undebuggable" then this patch is fine.
I haven't look at the patch but don't they keep/add the representation:
PC: symbol+offset/size
LR: symbol+offset/size
? This is needed at very least as a replacement for the missing address.
Yes, only %08lx was deleted, but %ps is still retained.
- printk("%s[<%08lx>] (%ps) from [<%08lx>] (%pS)\n",
- loglvl, where, (void *)where, from, (void *)from);
+ printk("%s (%ps) from (%pS)\n",
+ loglvl, (void *)where, (void *)from);
Thanks
Xiaoming Ni
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On 2020-10-09 09:08:50 [+0100], Russell King - ARM Linux admin wrote: > I am really not happy about this - it hurts at least my ability to > debug the kernel when people post oopses to the mailing list. If > people wish to make the kernel harder to debug, and are prepared > to be told "your kernel is undebuggable" then this patch is fine. I haven't look at the patch but don't they keep/add the representation: PC: symbol+offset/size LR: symbol+offset/size ? This is needed at very least as a replacement for the missing address. Sebastian
Re: [PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
On Fri, Oct 09, 2020 at 03:59:57PM +0800, Xiaoming Ni wrote:
> Printing raw pointer values in backtraces has potential security
> implications and are of questionable value anyway.
>
> This patch follows x86 and arm64's lead and removes the "Exception stack:"
> dump from kernel backtraces:
> commit a25ffd3a6302a6 ("arm64: traps: Don't print stack or raw
>PC/LR values in backtraces")
> commit 0ee1dd9f5e7eae ("x86/dumpstack: Remove raw stack dump")
> commit bb5e5ce545f203 ("x86/dumpstack: Remove kernel text
>addresses from stack dump")
>
> Signed-off-by: Xiaoming Ni
I am really not happy about this - it hurts at least my ability to
debug the kernel when people post oopses to the mailing list. If
people wish to make the kernel harder to debug, and are prepared
to be told "your kernel is undebuggable" then this patch is fine.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
[PATCH] arm:traps: Don't print stack or raw PC/LR values in backtraces
Printing raw pointer values in backtraces has potential security
implications and are of questionable value anyway.
This patch follows x86 and arm64's lead and removes the "Exception stack:"
dump from kernel backtraces:
commit a25ffd3a6302a6 ("arm64: traps: Don't print stack or raw
PC/LR values in backtraces")
commit 0ee1dd9f5e7eae ("x86/dumpstack: Remove raw stack dump")
commit bb5e5ce545f203 ("x86/dumpstack: Remove kernel text
addresses from stack dump")
Signed-off-by: Xiaoming Ni
---
arch/arm/kernel/process.c | 3 +--
arch/arm/kernel/traps.c | 12 +---
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 8e6ace03e960..71c9e5597d39 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -121,8 +121,7 @@ void __show_regs(struct pt_regs *regs)
printk("PC is at %pS\n", (void *)instruction_pointer(regs));
printk("LR is at %pS\n", (void *)regs->ARM_lr);
- printk("pc : [<%08lx>]lr : [<%08lx>]psr: %08lx\n",
- regs->ARM_pc, regs->ARM_lr, regs->ARM_cpsr);
+ printk("psr: %08lx\n", regs->ARM_cpsr);
printk("sp : %08lx ip : %08lx fp : %08lx\n",
regs->ARM_sp, regs->ARM_ip, regs->ARM_fp);
printk("r10: %08lx r9 : %08lx r8 : %08lx\n",
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 17d5a785df28..b0b188e01070 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -60,23 +60,18 @@ static int __init user_debug_setup(char *str)
__setup("user_debug=", user_debug_setup);
#endif
-static void dump_mem(const char *, const char *, unsigned long, unsigned long);
-
void dump_backtrace_entry(unsigned long where, unsigned long from,
unsigned long frame, const char *loglvl)
{
unsigned long end = frame + 4 + sizeof(struct pt_regs);
#ifdef CONFIG_KALLSYMS
- printk("%s[<%08lx>] (%ps) from [<%08lx>] (%pS)\n",
- loglvl, where, (void *)where, from, (void *)from);
+ printk("%s (%ps) from (%pS)\n",
+ loglvl, (void *)where, (void *)from);
#else
printk("%sFunction entered at [<%08lx>] from [<%08lx>]\n",
loglvl, where, from);
#endif
-
- if (in_entry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
- dump_mem(loglvl, "Exception stack", frame + 4, end);
}
void dump_backtrace_stm(u32 *stack, u32 instruction, const char *loglvl)
@@ -125,6 +120,9 @@ static void dump_mem(const char *lvl, const char *str,
unsigned long bottom,
mm_segment_t fs;
int i;
+ /* Do not print virtual addresses in non-reset scenarios */
+ if (!panic_on_oops)
+ return;
/*
* We need to switch to kernel mode so that we can use __get_user
* to safely read from kernel space. Note that we now dump the
--
2.27.0
