[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191021164904.GD56589@lakrids.cambridge.arm.com>
Date: Mon, 21 Oct 2019 17:49:04 +0100
From: Mark Rutland <mark.rutland@....com>
To: Sami Tolvanen <samitolvanen@...gle.com>
Cc: Jann Horn <jannh@...gle.com>, Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Steven Rostedt <rostedt@...dmis.org>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
Dave Martin <Dave.Martin@....com>,
Kees Cook <keescook@...omium.org>,
Laura Abbott <labbott@...hat.com>,
Nick Desaulniers <ndesaulniers@...gle.com>,
clang-built-linux <clang-built-linux@...glegroups.com>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 18/18] arm64: implement Shadow Call Stack
On Fri, Oct 18, 2019 at 10:35:49AM -0700, Sami Tolvanen wrote:
> On Fri, Oct 18, 2019 at 10:23 AM Mark Rutland <mark.rutland@....com> wrote:
> > I think scs_save() would better live in assembly in cpu_switch_to(),
> > where we switch the stack and current. It shouldn't matter whether
> > scs_load() is inlined or not, since the x18 value _should_ be invariant
> > from the PoV of the task.
>
> Note that there's also a call to scs_save in cpu_die, because the
> current task's shadow stack pointer is only stored in x18 and we don't
> want to lose it.
>
> > We just need to add a TSK_TI_SCS to asm-offsets.c, and then insert a
> > single LDR at the end:
> >
> > mov sp, x9
> > msr sp_el0, x1
> > #ifdef CONFIG_SHADOW_CALL_STACK
> > ldr x18, [x1, TSK_TI_SCS]
> > #endif
> > ret
>
> TSK_TI_SCS is already defined, so yes, we could move this to
> cpu_switch_to. I would still prefer to have the overflow check that's
> in scs_thread_switch though.
The only bit that I think needs to be in cpu_switch_to() is the install
of the next task's shadow addr into x18.
Having a separate scs_check_overflow() sounds fine to me, as that only
has to read from the shadow stack. IIUC that's also for the prev task,
not next, in the current patches.
Thanks,
Mark.
Powered by blists - more mailing lists