linux-kernel - Re: [PATCH v8 23/38] arm64/signal: Set up and restore the GCS context for signal handlers

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: <87zfvv7uyl.fsf@linaro.org>
Date: Mon, 19 Feb 2024 23:03:46 -0300
From: Thiago Jung Bauermann <thiago.bauermann@...aro.org>
To: Mark Brown <broonie@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>, 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>, Kees
 Cook <keescook@...omium.org>, 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>, "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>, 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 v8 23/38] arm64/signal: Set up and restore the GCS
 context for signal handlers

Mark Brown <broonie@...nel.org> writes:

> +#ifdef CONFIG_ARM64_GCS
> +static int gcs_restore_signal(void)
> +{
> +	u64 gcspr_el0, cap;
> +	int ret;
> +
> +	if (!system_supports_gcs())
> +		return 0;
> +
> +	if (!(current->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE))
> +		return 0;
> +
> +	gcspr_el0 = read_sysreg_s(SYS_GCSPR_EL0);
> +
> +	/*
> +	 * GCSPR_EL0 should be pointing at a capped GCS, read the cap...
> +	 */
> +	gcsb_dsync();
> +	ret = copy_from_user(&cap, (__user void*)gcspr_el0, sizeof(cap));
> +	if (ret)
> +		return -EFAULT;
> +
> +	/*
> +	 * ...then check that the cap is the actual GCS before
> +	 * restoring it.
> +	 */
> +	if (!gcs_signal_cap_valid(gcspr_el0, cap))
> +		return -EINVAL;
> +
> +	/* Invalidate the token to prevent reuse */
> +	put_user_gcs(0, (__user void*)gcspr_el0, &ret);
> +	if (ret != 0)
> +		return -EFAULT;

You had mentioned that "ideally we'd be doing a compare and exchange
here to substitute in a zero". Is a compare and exchange not necessary
anymore, or is it just being left for later? In the latter case, a TODO
or FIXME comment mentioning it would be useful here.

> +
> +	current->thread.gcspr_el0 = gcspr_el0 + sizeof(cap);
> +	write_sysreg_s(current->thread.gcspr_el0, SYS_GCSPR_EL0);
> +
> +	return 0;
> +}

-- 
Thiago