2020-10-16 04:01:18

by Xiaoming Ni

Subject: [PATCH v2] arm:traps: Don't print stack or raw PC/LR hex 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 bb5e5ce545f203 ("x86/dumpstack: Remove kernel text
addresses from stack dump")

Signed-off-by: Xiaoming Ni <[email protected]>

-------
v2:
Delete [<hex numbers>] from the stack according to the email discussion
in patch V1, Other information processing will be discussed in subsequent
patches.

v1: https://lore.kernel.org/lkml/[email protected]/
1. Don't print stack or raw PC/LR hex values in backtraces
2. Don't print stack mem in backtraces
3. if (!panic_on_oops), Don't print stack mem in __die()
---
arch/arm/kernel/process.c | 3 +--
arch/arm/kernel/traps.c | 4 ++--
2 files changed, 3 insertions(+), 4 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..911bbf164875 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -68,8 +68,8 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
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);
--
2.27.0

2020-10-26 08:35:49

by Xiaoming Ni

Subject: ping //Re: [PATCH v2] arm:traps: Don't print stack or raw PC/LR hex values in backtraces

ping

On 2020/10/16 10:31, 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 bb5e5ce545f203 ("x86/dumpstack: Remove kernel text
> addresses from stack dump")
>
> Signed-off-by: Xiaoming Ni <[email protected]>
>
> -------
> v2:
> Delete [<hex numbers>] from the stack according to the email discussion
> in patch V1, Other information processing will be discussed in subsequent
> patches.
>
> v1: https://lore.kernel.org/lkml/[email protected]/
> 1. Don't print stack or raw PC/LR hex values in backtraces
> 2. Don't print stack mem in backtraces
> 3. if (!panic_on_oops), Don't print stack mem in __die()
> ---
> arch/arm/kernel/process.c | 3 +--
> arch/arm/kernel/traps.c | 4 ++--
> 2 files changed, 3 insertions(+), 4 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..911bbf164875 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -68,8 +68,8 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
> 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);
>

2020-10-27 02:59:55

by Russell King (Oracle)

Subject: Re: ping //Re: [PATCH v2] arm:traps: Don't print stack or raw PC/LR hex values in backtraces

On Mon, Oct 26, 2020 at 02:33:10PM +0800, Xiaoming Ni wrote:
> ping

The arm tree was closed due to the merge window, and thus patches do
not get applied at that point. Plus I tend not to review development
patches during the merge window.

> On 2020/10/16 10:31, 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 bb5e5ce545f203 ("x86/dumpstack: Remove kernel text
> > addresses from stack dump")
> >
> > Signed-off-by: Xiaoming Ni <[email protected]>
> >
> > -------
> > v2:
> > Delete [<hex numbers>] from the stack according to the email discussion
> > in patch V1, Other information processing will be discussed in subsequent
> > patches.
> >
> > v1: https://lore.kernel.org/lkml/[email protected]/
> > 1. Don't print stack or raw PC/LR hex values in backtraces
> > 2. Don't print stack mem in backtraces
> > 3. if (!panic_on_oops), Don't print stack mem in __die()
> > ---
> > arch/arm/kernel/process.c | 3 +--
> > arch/arm/kernel/traps.c | 4 ++--
> > 2 files changed, 3 insertions(+), 4 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..911bbf164875 100644
> > --- a/arch/arm/kernel/traps.c
> > +++ b/arch/arm/kernel/traps.c
> > @@ -68,8 +68,8 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
> > 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);
> >
>
>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!