linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "dalias@libc.org" <dalias@libc.org>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "broonie@kernel.org" <broonie@kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"suzuki.poulose@arm.com" <suzuki.poulose@arm.com>,
	"Szabolcs.Nagy@arm.com" <Szabolcs.Nagy@arm.com>,
	"musl@lists.openwall.com" <musl@lists.openwall.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"kvmarm@lists.linux.dev" <kvmarm@lists.linux.dev>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"oliver.upton@linux.dev" <oliver.upton@linux.dev>,
	"palmer@dabbelt.com" <palmer@dabbelt.com>,
	"debug@rivosinc.com" <debug@rivosinc.com>,
	"aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"maz@kernel.org" <maz@kernel.org>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"fweimer@redhat.com" <fweimer@redhat.com>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"james.morse@arm.com" <james.morse@arm.com>,
	"ebiederm@xmission.com" <ebiederm@xmission.com>,
	"will@kernel.org" <will@kernel.org>,
	"brauner@kernel.org" <brauner@kernel.org>,
	"hjl.tools@gmail.com" <hjl.tools@gmail.com>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"paul.walmsley@sifive.com" <paul.walmsley@sifive.com>,
	"ardb@kernel.org" <ardb@kernel.org>,
	"thiago.bauermann@linaro.org" <thiago.bauermann@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"sorear@fastmail.com" <sorear@fastmail.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace
Date: Wed, 21 Feb 2024 13:30:55 -0500	[thread overview]
Message-ID: <20240221183055.GT4163@brightrain.aerifal.cx> (raw)
In-Reply-To: <f4a54297767eb098d903404cbe8860d655d79bed.camel@intel.com>

On Wed, Feb 21, 2024 at 06:12:30PM +0000, Edgecombe, Rick P wrote:
> On Wed, 2024-02-21 at 12:57 -0500, dalias@libc.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


  reply	other threads:[~2024-02-21 18:30 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-03 12:25 [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace Mark Brown
2024-02-03 12:25 ` [PATCH v8 01/38] arm64/mm: Restructure arch_validate_flags() for extensibility Mark Brown
2024-02-03 12:25 ` [PATCH v8 02/38] prctl: arch-agnostic prctl for shadow stack Mark Brown
2024-02-03 12:25 ` [PATCH v8 03/38] mman: Add map_shadow_stack() flags Mark Brown
2024-02-03 12:25 ` [PATCH v8 04/38] arm64: Document boot requirements for Guarded Control Stacks Mark Brown
2024-02-03 12:25 ` [PATCH v8 05/38] arm64/gcs: Document the ABI " Mark Brown
2024-02-03 12:25 ` [PATCH v8 06/38] arm64/sysreg: Add definitions for architected GCS caps Mark Brown
2024-02-03 12:25 ` [PATCH v8 07/38] arm64/gcs: Add manual encodings of GCS instructions Mark Brown
2024-02-03 12:25 ` [PATCH v8 08/38] arm64/gcs: Provide put_user_gcs() Mark Brown
2024-02-03 12:25 ` [PATCH v8 09/38] arm64/cpufeature: Runtime detection of Guarded Control Stack (GCS) Mark Brown
2024-02-03 12:25 ` [PATCH v8 10/38] arm64/mm: Allocate PIE slots for EL0 guarded control stack Mark Brown
2024-02-03 12:25 ` [PATCH v8 11/38] mm: Define VM_SHADOW_STACK for arm64 when we support GCS Mark Brown
2024-02-03 12:25 ` [PATCH v8 12/38] arm64/mm: Map pages for guarded control stack Mark Brown
2024-02-03 12:25 ` [PATCH v8 13/38] KVM: arm64: Manage GCS registers for guests Mark Brown
2024-02-05  9:46   ` Marc Zyngier
2024-02-05 12:35     ` Mark Brown
2024-02-05 15:34       ` Marc Zyngier
2024-02-05 16:58         ` Mark Brown
2024-02-03 12:25 ` [PATCH v8 14/38] arm64/gcs: Allow GCS usage at EL0 and EL1 Mark Brown
2024-02-03 12:25 ` [PATCH v8 15/38] arm64/idreg: Add overrride for GCS Mark Brown
2024-02-03 12:25 ` [PATCH v8 16/38] arm64/hwcap: Add hwcap " Mark Brown
2024-02-03 12:25 ` [PATCH v8 17/38] arm64/traps: Handle GCS exceptions Mark Brown
2024-02-03 12:25 ` [PATCH v8 18/38] arm64/mm: Handle GCS data aborts Mark Brown
2024-02-03 12:25 ` [PATCH v8 19/38] arm64/gcs: Context switch GCS state for EL0 Mark Brown
2024-02-03 12:25 ` [PATCH v8 20/38] arm64/gcs: Ensure that new threads have a GCS Mark Brown
2024-02-20  2:02   ` Thiago Jung Bauermann
2024-02-21 18:16     ` Mark Brown
2024-02-03 12:25 ` [PATCH v8 21/38] arm64/gcs: Implement shadow stack prctl() interface Mark Brown
2024-02-03 12:25 ` [PATCH v8 22/38] arm64/mm: Implement map_shadow_stack() Mark Brown
2024-02-03 12:25 ` [PATCH v8 23/38] arm64/signal: Set up and restore the GCS context for signal handlers Mark Brown
2024-02-20  2:03   ` Thiago Jung Bauermann
2024-02-03 12:25 ` [PATCH v8 24/38] arm64/signal: Expose GCS state in signal frames Mark Brown
2024-02-03 12:25 ` [PATCH v8 25/38] arm64/ptrace: Expose GCS via ptrace and core files Mark Brown
2024-02-03 12:25 ` [PATCH v8 26/38] arm64: Add Kconfig for Guarded Control Stack (GCS) Mark Brown
2024-02-03 12:25 ` [PATCH v8 27/38] kselftest/arm64: Verify the GCS hwcap Mark Brown
2024-02-03 12:25 ` [PATCH v8 28/38] kselftest/arm64: Add GCS as a detected feature in the signal tests Mark Brown
2024-02-03 12:25 ` [PATCH v8 29/38] kselftest/arm64: Add framework support for GCS to signal handling tests Mark Brown
2024-02-03 12:25 ` [PATCH v8 30/38] kselftest/arm64: Allow signals tests to specify an expected si_code Mark Brown
2024-02-03 12:25 ` [PATCH v8 31/38] kselftest/arm64: Always run signals tests with GCS enabled Mark Brown
2024-02-03 12:25 ` [PATCH v8 32/38] kselftest/arm64: Add very basic GCS test program Mark Brown
2024-02-03 12:25 ` [PATCH v8 33/38] kselftest/arm64: Add a GCS test program built with the system libc Mark Brown
2024-02-20  2:15   ` Thiago Jung Bauermann
2024-02-21 19:20     ` Mark Brown
2024-02-22 19:11     ` Mark Brown
2024-02-23  2:24       ` Thiago Jung Bauermann
2024-02-27 16:14         ` Mark Brown
2024-02-27 19:08           ` Thiago Jung Bauermann
2024-02-29 21:45         ` Mark Brown
2024-02-29 22:13           ` Thiago Jung Bauermann
2024-02-03 12:26 ` [PATCH v8 34/38] kselftest/arm64: Add test coverage for GCS mode locking Mark Brown
2024-02-03 12:26 ` [PATCH v8 35/38] selftests/arm64: Add GCS signal tests Mark Brown
2024-02-20  2:17   ` Thiago Jung Bauermann
2024-02-03 12:26 ` [PATCH v8 36/38] kselftest/arm64: Add a GCS stress test Mark Brown
2024-02-03 12:26 ` [PATCH v8 37/38] kselftest/arm64: Enable GCS for the FP stress tests Mark Brown
2024-02-03 12:26 ` [PATCH v8 38/38] kselftest: Provide shadow stack enable helpers for arm64 Mark Brown
2024-02-20  2:00 ` [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace Thiago Jung Bauermann
2024-02-20 16:36 ` Stefan O'Rear
2024-02-20 18:41   ` Edgecombe, Rick P
2024-02-20 18:57     ` [musl] " Rich Felker
2024-02-20 23:30       ` Edgecombe, Rick P
2024-02-20 23:54         ` dalias
2024-02-21  0:35           ` Edgecombe, Rick P
2024-02-21  0:44             ` Mark Brown
2024-02-21  1:27             ` dalias
2024-02-21  2:11               ` Edgecombe, Rick P
2024-02-21  4:18                 ` Edgecombe, Rick P
2024-02-21 13:53               ` Mark Brown
2024-02-21 14:58                 ` dalias
2024-02-21 17:36                   ` Mark Brown
2024-02-21 17:57                     ` dalias
2024-02-21 18:12                       ` Edgecombe, Rick P
2024-02-21 18:30                         ` dalias [this message]
2024-02-21 18:53                           ` Edgecombe, Rick P
2024-02-21 19:06                             ` dalias
2024-02-21 19:22                               ` Edgecombe, Rick P
2024-02-21 20:18                                 ` H.J. Lu
2024-02-21 20:25                                   ` H.J. Lu
2024-02-21 21:12                                     ` H.J. Lu
2024-02-21 20:18                                 ` dalias
2024-02-22 13:57                                 ` Mark Brown
2024-02-21 18:32                       ` Mark Brown
2024-02-21 19:10                         ` dalias
2024-03-02 14:57                     ` Szabolcs Nagy
2024-03-02 15:05                       ` H.J. Lu
2024-03-14 14:03                       ` Mark Brown
2024-02-20 23:59         ` Stefan O'Rear
2024-02-21  0:40           ` Mark Brown
2024-02-21  4:30           ` Edgecombe, Rick P
2024-02-20 20:14     ` Mark Brown
2024-02-20 23:30       ` Edgecombe, Rick P

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=20240221183055.GT4163@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=Szabolcs.Nagy@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=brauner@kernel.org \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=debug@rivosinc.com \
    --cc=ebiederm@xmission.com \
    --cc=fweimer@redhat.com \
    --cc=hjl.tools@gmail.com \
    --cc=james.morse@arm.com \
    --cc=keescook@chromium.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=musl@lists.openwall.com \
    --cc=oleg@redhat.com \
    --cc=oliver.upton@linux.dev \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=shuah@kernel.org \
    --cc=sorear@fastmail.com \
    --cc=suzuki.poulose@arm.com \
    --cc=thiago.bauermann@linaro.org \
    --cc=will@kernel.org \
    /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).