Message-ID: <2fb80876e286b4db8f9ef36bcce04bbf02af0de2.camel@intel.com>
Date: Tue, 16 Jul 2024 18:50:12 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "fweimer@...hat.com" <fweimer@...hat.com>, "broonie@...nel.org"
	<broonie@...nel.org>, "sroettger@...gle.com" <sroettger@...gle.com>
CC: "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"ross.burton@....com" <ross.burton@....com>, "suzuki.poulose@....com"
	<suzuki.poulose@....com>, "Szabolcs.Nagy@....com" <Szabolcs.Nagy@....com>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"Schimpe, Christina" <christina.schimpe@...el.com>, "kvmarm@...ts.linux.dev"
	<kvmarm@...ts.linux.dev>, "corbet@....net" <corbet@....net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"catalin.marinas@....com" <catalin.marinas@....com>, "kees@...nel.org"
	<kees@...nel.org>, "oliver.upton@...ux.dev" <oliver.upton@...ux.dev>,
	"palmer@...belt.com" <palmer@...belt.com>, "debug@...osinc.com"
	<debug@...osinc.com>, "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
	"shuah@...nel.org" <shuah@...nel.org>, "arnd@...db.de" <arnd@...db.de>,
	"maz@...nel.org" <maz@...nel.org>, "oleg@...hat.com" <oleg@...hat.com>,
	"thiago.bauermann@...aro.org" <thiago.bauermann@...aro.org>, "Pandey, Sunil
 K" <sunil.k.pandey@...el.com>, "james.morse@....com" <james.morse@....com>,
	"ebiederm@...ssion.com" <ebiederm@...ssion.com>, "brauner@...nel.org"
	<brauner@...nel.org>, "will@...nel.org" <will@...nel.org>,
	"hjl.tools@...il.com" <hjl.tools@...il.com>,
	"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
	"paul.walmsley@...ive.com" <paul.walmsley@...ive.com>, "ardb@...nel.org"
	<ardb@...nel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-mm@...ck.org"
	<linux-mm@...ck.org>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>
Subject: Re: [PATCH v9 05/39] arm64/gcs: Document the ABI for Guarded Control
 Stacks

+Stephen, who had been asking about RIP integrity awhile back.

Thread context for Stephen:
https://lore.kernel.org/lkml/Zo7SdDT_cBp6uXgT@finisterre.sirena.org.uk/#t

On Wed, 2024-07-10 at 19:27 +0100, Mark Brown wrote:
> On Wed, Jul 10, 2024 at 12:36:21PM +0200, Florian Weimer wrote:
> > * Mark Brown:
> 
> > > +* When GCS is enabled for the interrupted thread a signal handling
> > > specific
> > > +  GCS cap token will be written to the GCS, this is an architectural GCS
> > > cap
> > > +  token with bit 63 set and the token type (bits 0..11) all clear.  The
> > > +  GCSPR_EL0 reported in the signal frame will point to this cap token.
> 
> > How does this marker interfere with Top Byte Ignore (TBI; I hope I got
> > the name right)?  The specification currently does not say that only
> > addresses pushed to the shadow stack with the top byte cleared, which
> > potentially makes the markup ambiguous.  On x86-64, the same issue may
> 
> Indeed...  Given that we use the address on the GCS as part of the token
> on first pass I think we could get away with just using the address and
> not setting the top bit, we'd have an invalid cap pointing into a GCS
> page which shouldn't otherwise be on the GCS.  I'll give that some more
> thought.
> 
> > exist with LAM.  I have not tested yet what happens there.  On AArch64
> > and RISC-V, it may be more natural to use the LSB instead of the LSB for
> > the mark bit because of its instruction alignment.
> 
> The LSB is already taken by the architecture on aarch64, the bottom bits
> of the value are used for the token type field with no values/bits
> reserved for software use.
> 
> > We also have a gap on x86-64 for backtrace generation because the
> > interrupted instruction address does not end up on the shadow stack.
> > This address is potentially quite interesting for backtrace generation.
> > I assume it's currently missing because the kernel does not resume
> > execution using a regular return instruction.  It would be really useful
> > if it could be pushed to the shadow stack, or recoverable from the
> > shadow stack in some other way (e.g., the address of the signal context
> > could be pushed instead).  That would need some form of marker as well.
> 
> Right, we'd have to manually consume any extra address we put on the
> GCS.  I'm not seeing any gagetisation issues with writing an extra value
> there that isn't a valid stack cap at the minute but I'll need to think
> it through properly - don't know if anyone else has thoughts here?

Shadow stack has one main usage (security) and another less proven, but
interesting usage for backtracing. I'm wary of adding things to the shadow stack
as they come up in an ad-hoc fashion, especially for the fuzzier usage. Do you
have a handle on everything the tracing usage would need?

But besides that I've wondered if there could be a security benefit to adding
some fields of the sigframe (RIP being the prime one) to the shadow stack, or a
cryptographic hash of the sigframe.

Powered by blists - more mailing lists