From: Mark Rutland <mark.rutland@arm.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <adech.fo@gmail.com>,
Dmitriy Vyukov <dvyukov@google.com>,
Andrey Ryabinin <ryabinin.a.a@gmail.com>,
will.deacon@arm.com, catalin.marinas@arm.com,
Andrew Morton <akpm@linux-foundation.org>,
kasan-dev@googlegroups.com, LKML <linux-kernel@vger.kernel.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1] kasan, arm64: Unpoison dirty stack frames when resuming from suspend.
Date: Tue, 1 Mar 2016 19:37:16 +0000 [thread overview]
Message-ID: <20160301193529.GA335@leverpostej> (raw)
In-Reply-To: <CAG_fn=XowizL_=tt5aiHBF=_UNYKZsGWQOR7Ju-NYz2kre-7bA@mail.gmail.com>
On Fri, Feb 26, 2016 at 06:28:27PM +0100, Alexander Potapenko wrote:
> On Fri, Feb 26, 2016 at 2:53 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi,
> >
> > On Fri, Feb 26, 2016 at 01:38:37PM +0100, Alexander Potapenko wrote:
> >> Before an ARM64 CPU is suspended, the kernel saves the context which will
> >> be used to initialize the register state upon resume. After that and
> >> before the actual execution of the SMC instruction the kernel creates
> >> several stack frames which are never unpoisoned because arm_smccc_smc()
> >> does not return. This may cause false positive stack buffer overflow
> >> reports from KASAN.
> >>
> >> The solution is to record the stack pointer value just before the CPU is
> >> suspended, and unpoison the part of stack between the saved value and
> >> the stack pointer upon resume.
> >
> > Thanks for looking into this! That's much appreciated.
> >
> > I think the general approach (unposioning the stack upon cold return to
> > the kernel) is fine, but I have concerns with the implementation, which
> > I've noted below.
> >
> > The problem also applies for hotplug, as leftover poison from the
> > hot-unplug path isn't cleaned before a CPU is hotplugged back on. The
> > first few functions are likely deterministic in their stack usage, so
> > it's not seen with a defconfig, but I think it's possible to trigger,
> > and it's also a cross-architecture problem shared with x86.
> Agreed, but since I haven't yet seen problems with hotplug, it's hard
> to test the fix for them.
For testing, I used the below to deliberately hit stale poison after a
hotplug. It deliberately creates large stack frames, accessing as much
of the stack as possible to increase the chance of hitting any posion.
Mark.
---->8----
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 544a713..ef4693f 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -195,6 +195,21 @@ exit_idle:
DEFINE_PER_CPU(bool, cpu_dead_idle);
+#define NR_STACK_ELEMS 128
+static noinline void hit_stale_poison(unsigned int frames)
+{
+ volatile unsigned long magic[NR_STACK_ELEMS];
+ int i;
+
+ for (i = 0; i < NR_STACK_ELEMS; i++)
+ magic[i] = 0;
+
+ if (frames)
+ hit_stale_poison(frames - 1);
+
+ return;
+}
+
/*
* Generic idle loop implementation
*
@@ -202,6 +217,8 @@ DEFINE_PER_CPU(bool, cpu_dead_idle);
*/
static void cpu_idle_loop(void)
{
+ hit_stale_poison(4);
+
while (1) {
/*
* If the arch has a polling bit, we maintain an invariant:
next prev parent reply other threads:[~2016-03-01 19:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-26 12:38 [PATCH v1] kasan, arm64: Unpoison dirty stack frames when resuming from suspend Alexander Potapenko
2016-02-26 13:53 ` Mark Rutland
2016-02-26 17:28 ` Alexander Potapenko
2016-03-01 19:37 ` Mark Rutland [this message]
2016-03-01 19:42 ` Mark Rutland
2016-03-01 20:05 ` [PATCH] sched/kasan: clear stale stack poison kbuild test robot
2016-03-02 12:53 ` [PATCH v1] kasan, arm64: Unpoison dirty stack frames when resuming from suspend Andrey Ryabinin
2016-03-02 13:51 ` [PATCH 1/2] cpu, idle: move init_idle() out of idle_thread_get() Andrey Ryabinin
2016-03-02 13:51 ` [PATCH 2/2] kasan: unpoison stack of idle task on cpu online Andrey Ryabinin
2016-03-02 14:50 ` Mark Rutland
2016-03-02 15:27 ` Andrey Ryabinin
2016-03-02 15:43 ` Mark Rutland
2016-02-27 1:05 ` [PATCH v1] kasan, arm64: Unpoison dirty stack frames when resuming from suspend kbuild test robot
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=20160301193529.GA335@leverpostej \
--to=mark.rutland@arm.com \
--cc=adech.fo@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=catalin.marinas@arm.com \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ryabinin.a.a@gmail.com \
--cc=will.deacon@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).