Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530161 > unrolled thread

Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump

Started by"Kirill A. Shutemov" <kirill@shutemov.name>
First post2016-11-25 13:30 +0100
Last post2016-11-29 15:20 +0100
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from  stack dump "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-11-25 13:30 +0100
    Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from  stack dump Josh Poimboeuf <jpoimboe@redhat.com> - 2016-11-28 22:00 +0100
      Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from  stack dump "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-11-28 23:30 +0100
        [PATCH] decode_stacktrace: fix address line detection on x86 Josh Poimboeuf <jpoimboe@redhat.com> - 2016-11-29 00:10 +0100
          [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line  detection on x86 tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2016-11-29 08:20 +0100
            Re: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line  detection on x86 Josh Poimboeuf <jpoimboe@redhat.com> - 2016-11-29 14:10 +0100
              Re: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line  detection on x86 Ingo Molnar <mingo@kernel.org> - 2016-11-29 14:30 +0100
          [tip:x86/asm] scripts/decode_stacktrace.sh: Fix address line  detection on x86 tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2016-11-29 15:20 +0100

#1530161 — Re: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2016-11-25 13:30 +0100
SubjectRe: [PATCH 2/4] x86/dumpstack: remove kernel text addresses from stack dump
Message-ID<sHnND-wd-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1531655

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-11-28 22:00 +0100
Message-ID<sIBbP-72M-9@gated-at.bofh.it>
In reply to#1530161
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

[toc] | [prev] | [next] | [standalone]

#1531741

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2016-11-28 23:30 +0100
Message-ID<sICAW-85L-15@gated-at.bofh.it>
In reply to#1531655
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

[toc] | [prev] | [next] | [standalone]

#1531786 — [PATCH] decode_stacktrace: fix address line detection on x86

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-11-29 00:10 +0100
Subject[PATCH] decode_stacktrace: fix address line detection on x86
Message-ID<sIDdE-75-45@gated-at.bofh.it>
In reply to#1531741
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

[toc] | [prev] | [next] | [standalone]

#1531962 — [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86

Fromtip-bot for Josh Poimboeuf <tipbot@zytor.com>
Date2016-11-29 08:20 +0100
Subject[tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86
Message-ID<sIKRP-5c9-1@gated-at.bofh.it>
In reply to#1531786
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?

[toc] | [prev] | [next] | [standalone]

#1532264 — Re: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2016-11-29 14:10 +0100
SubjectRe: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86
Message-ID<sIQky-rW-35@gated-at.bofh.it>
In reply to#1531962
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

[toc] | [prev] | [next] | [standalone]

#1532275 — Re: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86

FromIngo Molnar <mingo@kernel.org>
Date2016-11-29 14:30 +0100
SubjectRe: [tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86
Message-ID<sIQDT-yf-5@gated-at.bofh.it>
In reply to#1532264
* 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

[toc] | [prev] | [next] | [standalone]

#1532327 — [tip:x86/asm] scripts/decode_stacktrace.sh: Fix address line detection on x86

Fromtip-bot for Josh Poimboeuf <tipbot@zytor.com>
Date2016-11-29 15:20 +0100
Subject[tip:x86/asm] scripts/decode_stacktrace.sh: Fix address line detection on x86
Message-ID<sIRqh-17H-15@gated-at.bofh.it>
In reply to#1531786
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?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web