All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Peter Zijlstra <peterz@infradead.org>
Cc: keescook@chromium.org, jannh@google.com,
	linux-kernel@vger.kernel.org, vcaputo@pengaru.com,
	mingo@redhat.com, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	bristot@redhat.com, akpm@linux-foundation.org,
	christian.brauner@ubuntu.com, amistry@google.com,
	Kenta.Tada@sony.com, legion@kernel.org,
	michael.weiss@aisec.fraunhofer.de, mhocko@suse.com,
	deller@gmx.de, zhengqi.arch@bytedance.com, me@tobin.cc,
	tycho@tycho.pizza, tglx@linutronix.de, bp@alien8.de,
	hpa@zytor.com, mark.rutland@arm.com, axboe@kernel.dk,
	metze@samba.org, laijs@linux.alibaba.com, luto@kernel.org,
	dave.hansen@linux.intel.com, ebiederm@xmission.com,
	ohoono.kwon@samsung.com, kaleshsingh@google.com,
	yifeifz2@illinois.edu, jpoimboe@redhat.com,
	linux-hardening@vger.kernel.org, linux-arch@vger.kernel.org,
	vgupta@kernel.org, will@kernel.org, guoren@kernel.org,
	bcain@codeaurora.org, monstr@monstr.eu,
	tsbogend@alpha.franken.de, nickhu@andestech.com,
	jonas@southpole.se, mpe@ellerman.id.au, paul.walmsley@sifive.com,
	hca@linux.ibm.com, ysato@users.sourceforge.jp,
	davem@davemloft.net, chris@zankel.net
Subject: Re: [PATCH 0/7] wchan: Fix wchan support
Date: Thu, 14 Oct 2021 14:38:19 +0100	[thread overview]
Message-ID: <YWgyy+KvNLQ7eMIV@shell.armlinux.org.uk> (raw)
In-Reply-To: <YWgcWnbsvI1rbvEj@shell.armlinux.org.uk>

On Thu, Oct 14, 2021 at 01:02:34PM +0100, Russell King (Oracle) wrote:
> On Fri, Oct 08, 2021 at 01:15:27PM +0200, Peter Zijlstra wrote:
> > Hi,
> > 
> > This fixes up wchan which is various degrees of broken across the
> > architectures.
> > 
> > Patch 4 fixes wchan for x86, which has been returning 0 for the past many
> > releases.
> > 
> > Patch 5 fixes the fundamental race against scheduling.
> > 
> > Patch 6 deletes a lot and makes STACKTRACE unconditional
> > 
> > patch 7 fixes up a few STACKTRACE arch oddities
> > 
> > 0day says all builds are good, so it must be perfect :-) I'm planning on
> > queueing up at least the first 5 patches, but I'm hoping the last two patches
> > can be too.
> > 
> > Also available here:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git sched/wchan
> 
> These patches introduce a regression on ARM. Whereas before, I have
> /proc/*/wchan populated with non-zero values, with these patches they
> _all_ contain "0":
> 
> root@clearfog21:~# cat /proc/*/wchan
> 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000root@clearfog21:~#
> 
> I'll try to investigate what is going on later today.

What is going on here is that the ARM stacktrace code refuses to trace
non-current tasks in a SMP environment due to the racy nature of doing
so if the non-current tasks are running.

When walking the stack with frame pointers, we:

- validate that the frame pointer is between the stack pointer and the
  top of stack defined by that stack pointer.
- we then load the next stack pointer and next frame pointer from the
  stack.

The reason this is unsafe when the task is not blocked is the stack can
change at any moment, which can cause the value read as a stack pointer
to be wildly different. If the read frame pointer value is roughly in
agreement, we can end up reading any part of memory, which would be an
information leak.

The table based unwinding is much more complex being essentially a set
of instructions to the unwinder code about which values to read from
the stack into a set of pseudo-registers, corrections to the stack
pointer, or transfers from the pseudo-registers. I haven't analysed
this code enough to really know the implications of what could be
possible if the values on the stack change while this code is running
on another CPU (it's not my code!) There is an attempt to bounds-limit
the virtual stack pointer after each unwind instruction is processed
to catch the unwinder doing anything silly, so it may be safe in so far
as it will fail should it encounter anything "stupid".

However, get_wchan() is a different case; we know for certain that the
task is blocked, so it won't be running on another CPU, and with your
patch 4, we have this guarantee. However, that is not true of all
callers to the stacktracing code, so I don't see how we can sanely
switch to using the stacktracing code for this.

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

  reply	other threads:[~2021-10-14 13:39 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08 11:15 [PATCH 0/7] wchan: Fix wchan support Peter Zijlstra
2021-10-08 11:15 ` [PATCH 1/7] Revert "proc/wchan: use printk format instead of lookup_symbol_name()" Peter Zijlstra
2021-10-15  9:45   ` [tip: sched/core] " tip-bot2 for Kees Cook
2021-10-08 11:15 ` [PATCH 2/7] leaking_addresses: Always print a trailing newline Peter Zijlstra
2021-10-15  9:45   ` [tip: sched/core] " tip-bot2 for Kees Cook
2021-10-08 11:15 ` [PATCH 3/7] proc: Use task_is_running() for wchan in /proc/$pid/stat Peter Zijlstra
2021-10-15  9:45   ` [tip: sched/core] " tip-bot2 for Kees Cook
2021-10-08 11:15 ` [PATCH 4/7] x86: Fix get_wchan() to support the ORC unwinder Peter Zijlstra
2021-10-15  9:45   ` [tip: sched/core] " tip-bot2 for Qi Zheng
2021-10-08 11:15 ` [PATCH 5/7] sched: Add wrapper for get_wchan() to keep task blocked Peter Zijlstra
2021-10-08 11:26   ` Geert Uytterhoeven
2021-10-08 12:45   ` Mark Rutland
2021-10-14 10:46   ` Russell King (Oracle)
2021-10-15  9:45   ` [tip: sched/core] " tip-bot2 for Kees Cook
2021-10-08 11:15 ` [PATCH 6/7] arch: __get_wchan || STACKTRACE_SUPPORT Peter Zijlstra
2021-10-08 12:40   ` Mark Rutland
2021-10-08 13:45     ` Peter Zijlstra
2021-10-08 16:17       ` Josh Poimboeuf
2021-10-14 18:07         ` Mark Rutland
2021-10-14 18:41           ` Josh Poimboeuf
2021-10-14 18:03       ` Mark Rutland
2021-10-14 18:48         ` Josh Poimboeuf
2021-10-14 11:07   ` Russell King (Oracle)
2021-10-08 11:15 ` [PATCH 7/7] arch: Fix STACKTRACE_SUPPORT Peter Zijlstra
2021-10-08 12:52   ` Mark Rutland
2021-10-09  9:36   ` Guo Ren
2021-10-14 12:02 ` [PATCH 0/7] wchan: Fix wchan support Russell King (Oracle)
2021-10-14 13:38   ` Russell King (Oracle) [this message]
2021-10-14 19:51     ` Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YWgyy+KvNLQ7eMIV@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Kenta.Tada@sony.com \
    --cc=akpm@linux-foundation.org \
    --cc=amistry@google.com \
    --cc=axboe@kernel.dk \
    --cc=bcain@codeaurora.org \
    --cc=bp@alien8.de \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=chris@zankel.net \
    --cc=christian.brauner@ubuntu.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=dietmar.eggemann@arm.com \
    --cc=ebiederm@xmission.com \
    --cc=guoren@kernel.org \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jannh@google.com \
    --cc=jonas@southpole.se \
    --cc=jpoimboe@redhat.com \
    --cc=juri.lelli@redhat.com \
    --cc=kaleshsingh@google.com \
    --cc=keescook@chromium.org \
    --cc=laijs@linux.alibaba.com \
    --cc=legion@kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=me@tobin.cc \
    --cc=metze@samba.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.com \
    --cc=michael.weiss@aisec.fraunhofer.de \
    --cc=mingo@redhat.com \
    --cc=monstr@monstr.eu \
    --cc=mpe@ellerman.id.au \
    --cc=nickhu@andestech.com \
    --cc=ohoono.kwon@samsung.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tsbogend@alpha.franken.de \
    --cc=tycho@tycho.pizza \
    --cc=vcaputo@pengaru.com \
    --cc=vgupta@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=will@kernel.org \
    --cc=yifeifz2@illinois.edu \
    --cc=ysato@users.sourceforge.jp \
    --cc=zhengqi.arch@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.