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

Kirill A. Shutemov
2016-11-28T22:27:24Z

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