kernel-hardening - Re: [PATCH v4 13/17] arm64: preserve x18 when CPU is suspended

Follow @Openwall on Twitter for new release announcements and other news

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

Message-ID: <CABCJKueN+Op8xm+L3aSFgCL9BLC8b-WHj3oBYhf1W=OcX2aqCg@mail.gmail.com>
Date: Mon, 4 Nov 2019 16:02:16 -0800
From: Sami Tolvanen <samitolvanen@...gle.com>
To: Nick Desaulniers <ndesaulniers@...gle.com>
Cc: Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>, 
	Catalin Marinas <catalin.marinas@....com>, Steven Rostedt <rostedt@...dmis.org>, 
	Masami Hiramatsu <mhiramat@...nel.org>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, 
	Dave Martin <dave.martin@....com>, Kees Cook <keescook@...omium.org>, 
	Laura Abbott <labbott@...hat.com>, Mark Rutland <mark.rutland@....com>, Jann Horn <jannh@...gle.com>, 
	Miguel Ojeda <miguel.ojeda.sandonis@...il.com>, 
	Masahiro Yamada <yamada.masahiro@...ionext.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>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 13/17] arm64: preserve x18 when CPU is suspended

On Mon, Nov 4, 2019 at 1:59 PM Nick Desaulniers <ndesaulniers@...gle.com> wrote:
>
> On Mon, Nov 4, 2019 at 1:38 PM Sami Tolvanen <samitolvanen@...gle.com> wrote:
> >
> > On Mon, Nov 4, 2019 at 5:20 AM Marc Zyngier <maz@...nel.org> wrote:
> > > >  ENTRY(cpu_do_suspend)
> > > >       mrs     x2, tpidr_el0
> > > > @@ -73,6 +75,9 @@ alternative_endif
> > > >       stp     x8, x9, [x0, #48]
> > > >       stp     x10, x11, [x0, #64]
> > > >       stp     x12, x13, [x0, #80]
> > > > +#ifdef CONFIG_SHADOW_CALL_STACK
> > > > +     str     x18, [x0, #96]
> > > > +#endif
> > >
> > > Do we need the #ifdefery here? We didn't add that to the KVM path,
> > > and I'd feel better having a single behaviour, specially when
> > > NR_CTX_REGS is unconditionally sized to hold 13 regs.
> >
> > I'm fine with dropping the ifdefs here in v5 unless someone objects to this.
>
> Oh, yeah I guess it would be good to be consistent.  Rather than drop
> the ifdefs, would you (Marc) be ok with conditionally setting
> NR_CTX_REGS based on CONFIG_SHADOW_CALL_STACK, and doing so in KVM?
> (So 3 ifdefs, rather than 0)?
>
> Without any conditionals or comments, it's not clear why x18 is being
> saved and restored (unless git blame survives, or a comment is added
> in place of the ifdefs in v6).

True. Clearing the sleep state buffer in cpu_do_resume is also
pointless without CONFIG_SHADOW_CALL_STACK, so if the ifdefs are
removed, some kind of an explanation is needed there.

Sami

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.