Message-ID: <20240221183055.GT4163@brightrain.aerifal.cx>
Date: Wed, 21 Feb 2024 13:30:55 -0500
From: "dalias@...c.org" <dalias@...c.org>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Cc: "broonie@...nel.org" <broonie@...nel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"suzuki.poulose@....com" <suzuki.poulose@....com>,
	"Szabolcs.Nagy@....com" <Szabolcs.Nagy@....com>,
	"musl@...ts.openwall.com" <musl@...ts.openwall.com>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"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>,
	"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>,
	"fweimer@...hat.com" <fweimer@...hat.com>,
	"keescook@...omium.org" <keescook@...omium.org>,
	"james.morse@....com" <james.morse@....com>,
	"ebiederm@...ssion.com" <ebiederm@...ssion.com>,
	"will@...nel.org" <will@...nel.org>,
	"brauner@...nel.org" <brauner@...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>,
	"thiago.bauermann@...aro.org" <thiago.bauermann@...aro.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>,
	"sorear@...tmail.com" <sorear@...tmail.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS
 in userspace

On Wed, Feb 21, 2024 at 06:12:30PM +0000, Edgecombe, Rick P wrote:
> On Wed, 2024-02-21 at 12:57 -0500, dalias@...c.org wrote:
> > > This feels like it's getting complicated and I fear it may be an
> > > uphill
> > > struggle to get such code merged, at least for arm64.  My instinct
> > > is
> > > that it's going to be much more robust and generally tractable to
> > > let
> > > things run to some suitable synchronisation point and then disable
> > > there, but if we're going to do that then userspace can hopefully
> > > arrange to do the disabling itself through the standard disable
> > > interface anyway.  Presumably it'll want to notice things being
> > > disabled
> > > at some point anyway?  TBH that's been how all the prior proposals
> > > for
> > > process wide disable I've seen were done.
> > 
> > If it's possible to disable per-thread rather than per-process, some
> > things are easier. Disabling on account of using alt stacks only
> > needs
> > to be done on the threads using those stacks. However, for dlopen
> > purposes you need a way to disable shadow stack for the whole
> > process.
> > Initially this is only needed for the thread that called dlopen, but
> > it needs to have propagated to any thread that synchronizes with
> > completion of the call to dlopen by the time that synchronization
> > occurs, and since that synchronization can happen in lots of
> > different
> > ways that are purely userspace (thanks to futexes being userspace in
> > the uncontended case), I don't see any way to make it work without
> > extremely invasive, high-cost checks.
> 
> For glibc's use, we talked about a couple of options.
> 1. A mode to start suppressing the #UD's from the shadow stack
> instructions
> 2. A mode to start suppressing #CPs (the exception that happens when
> the shadow stack doesn't match). So the shadow stack instructions
> continue to operate normally, but if the shadow stack gets mismatched
> due to lack of support, the ret is emulated. It probably is safer (but
> still not perfect), but the performance penalty of emulating every RET
> after things get screwed up would be a significant down side. There
> also needs to be clean handling of shadow stack #PFs.
> 3. Per-thread locking that is used around all shadow stack operations
> that could be sensitive to disabling. This could be maybe exposed to
> apps in case they want to use shadow stack instructions manually. Then
> during dlopen() it waits until it can cleanly disable shadow stack for
> each thread. In each critical sections there are checks for whether
> shadow stack is still enabled.
> 
> 3 is the cleanest and safest I think, and it was thought it might not
> need kernel help, due to a scheme Florian had to direct signals to
> specific threads. It's my preference at this point.

The operations where the shadow stack has to be processed need to be
executable from async-signal context, so this imposes a requirement to
block all signals around the lock. This makes all longjmps a heavy,
multi-syscall operation rather than O(1) userspace operation. I do not
think this is an acceptable implementation, especially when there are
clearly superior alternatives without that cost or invasiveness.

> 1 and 2 are POCed here, if you are interested:
> https://github.com/rpedgeco/linux/commits/shstk_suppress_rfc/

I'm not clear why 2 (suppression of #CP) is desirable at all. If
shadow stack is being disabled, it should just be disabled, with
minimal fault handling to paper over any racing operations at the
moment it's disabled. Leaving it on with extreme slowness to make it
not actually do anything does not seem useful.

Is there some way folks have in mind to use option 2 to lazily disable
shadow stack once the first SS-incompatible code is executed, when
execution is then known not to be in the middle of a SS-critical
section, instead of doing it right away? I don't see how this could
work, since the SS-incompatible code could be running from a signal
handler that interrupted an SS-critical section.

> > If folks on the kernel side are not going to be amenable to doing the
> > things that are easy for the kernel to make it work without breaking
> > compatibility with existing interfaces, but that are impossible or
> > near-impossible for userspace to do, this seems like a dead-end. And
> > I
> > suspect an operation to "disable shadow stack, but without making
> > threads still in SS-critical sections crash" is going to be
> > necessary..
> 
> I think we have to work through all the alternative before we can
> accuse the kernel of not being amenable. Is there something that you
> would like to see out of this conversation that is not happening?

No, I was just interpreting "uphill battle". I really do not want to
engage in an uphill battle for the sake of making it practical to
support something that was never my goal to begin with. If I'm
misreading this, or if others are willing to put the effort into that
"battle", I'd be happy to be mistaken about "not amenable".

Rich

Powered by blists - more mailing lists