linux-kernel - Re: [PATCH v10 25/40] arm64/ptrace: Expose GCS via ptrace and core files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <58ee01de-88a6-4d0c-845a-3d5bebc0c55c@sirena.org.uk>
Date: Wed, 21 Aug 2024 19:27:53 +0100
From: Mark Brown <broonie@...nel.org>
To: Catalin Marinas <catalin.marinas@....com>
Cc: Will Deacon <will@...nel.org>, Jonathan Corbet <corbet@....net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Marc Zyngier <maz@...nel.org>,
	Oliver Upton <oliver.upton@...ux.dev>,
	James Morse <james.morse@....com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Arnd Bergmann <arnd@...db.de>, Oleg Nesterov <oleg@...hat.com>,
	Eric Biederman <ebiederm@...ssion.com>,
	Shuah Khan <shuah@...nel.org>,
	"Rick P. Edgecombe" <rick.p.edgecombe@...el.com>,
	Deepak Gupta <debug@...osinc.com>, Ard Biesheuvel <ardb@...nel.org>,
	Szabolcs Nagy <Szabolcs.Nagy@....com>, Kees Cook <kees@...nel.org>,
	"H.J. Lu" <hjl.tools@...il.com>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>,
	Florian Weimer <fweimer@...hat.com>,
	Christian Brauner <brauner@...nel.org>,
	Thiago Jung Bauermann <thiago.bauermann@...aro.org>,
	Ross Burton <ross.burton@....com>,
	linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
	kvmarm@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
	linux-arch@...r.kernel.org, linux-mm@...ck.org,
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v10 25/40] arm64/ptrace: Expose GCS via ptrace and core
 files

On Wed, Aug 21, 2024 at 06:57:16PM +0100, Catalin Marinas wrote:
> On Thu, Aug 01, 2024 at 01:06:52PM +0100, Mark Brown wrote:

> > +	if (user_gcs.features_enabled & ~PR_SHADOW_STACK_SUPPORTED_STATUS_MASK)
> > +		return -EINVAL;

> > +	/* Do not allow enable via ptrace */
> > +	if ((user_gcs.features_enabled & PR_SHADOW_STACK_ENABLE) &&
> > +	    !(target->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE))
> > +		return -EBUSY;

> > +	target->thread.gcs_el0_mode = user_gcs.features_enabled;
> > +	target->thread.gcs_el0_locked = user_gcs.features_locked;
> > +	target->thread.gcspr_el0 = user_gcs.gcspr_el0;

> As in the previous thread, I thought we need to restore GCSPR_EL0
> unconditionally.

My thinking here is that if we return an error code then ideally we
shouldn't implement any changes, especially in cases like this one
where we're rejecting inputs that are just invalid.  That seems like the
least surprising outcome and what we should strive for, even if it's too
complicated for some cases.  It is possible to write GCSPR_EL0
regardless of if GCS is enabled, it's just not possible to write it as
part of an otherwise invalid write.  The validation is checking for
unknown features and enables.  With clone3() we could relax the enable
check, but I've just pulled that out of the series for the time being.

> I don't particularly like that this register becomes some scrap one that
> threads can use regardless of GCS. Not sure we have a simple solution.
> We could track three states: GCS never enabled, GCS enabled and GCS
> disabled after being enabled. It's probably not worth it.

Yeah, I did give consideration to that but as you say I think it's
probably not worth it - you end up with a state machine which for the
most part only gets two of the states exercised.

> On ptrace() access to the shadow stack, we rely on the barrier in the
> context switch code if stopping a thread. If other threads are running
> on other CPUs, it's racy anyway even for normal accesses, so I don't
> think we need to do anything more for ptrace.

Yes, I don't see any reason to try to do anything special there.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)