linux-kernel - Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100603203155.GA1163@redhat.com>
Date:	Thu, 3 Jun 2010 22:31:55 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	Arjan van de Ven <arjan@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Roland McGrath <roland@...hat.com>,
	Vegard Nossum <vegard.nossum@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] x86: make save_stack_address()
	!CONFIG_FRAME_POINTER friendly

On 06/03, Frederic Weisbecker wrote:
>
> On Thu, Jun 03, 2010 at 12:53:52PM -0700, Arjan van de Ven wrote:
> > On 6/3/2010 12:32 PM, Oleg Nesterov wrote:
> >> If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
> >> non-reliable addresses on stack, this is all we have if dump_trace(bp)
> >> is called with the wrong or zero bp.
> >>
> >> For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
> >>
> >> This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
> >> it reverts 1650743c "x86: don't save unreliable stack trace entries".
> >
> > would be nice if there was a compile time thing to detect if frame
> > pointers are on ratehr than an ifdef.
>
> I wanted to suggest that too, but since only one place got the ifdef
> after the second patch.
>
> But yeah, something like this could be reused:
>
> 	if (reliable_frame_pointer(reliable))
> 		return ...;

Do you mean it makes sense to add the helper which depends on
FRAME_POINTER ?

> > you're now also changing the rules; until now, you would ALWAYS get a
> > backtrace without noise....
> > now that's changing quite a bit. How are various tools (like perf and
> > sysprof) going to cope with that?
>
>
>
> perf and sysprof have their own stacktrace ops, so they aren't affected.
> I think the rest is /proc/pid/task, lockdep, latencytop, ftrace, kmemleak,
> etc...
>
> For the kernel parts it's in fact desired.
> And with ftrace we are changing some rules, but this is desired too, without
> frame pointers we would have nothing anyway. And it's quite easy to
> find out a stacktrace is not entirely reliable at a glance.

Frederic, Arjan. Honestly, I have no opinion if this change makes
things better or worse for, say, lockdep.

But note that this only affects the !CONFIG_FRAME_POINTER case.
Looking into Kconfig's I don't even understand how the bug reporters
managed to set CONFIG_STACKTRACE without !CONFIG_FRAME_POINTER.

So, should I redo this patch to fix /proc/pid/stack ? Say, we
can change the meaning of stack_trace-<skip, if it is < 0, then
save_stack_address() ignores reliable. Yes, this is hack.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/