Re: [PATCH 03/20] arm64/fpsimd: signal: Clear PSTATE.SM when restoring FPSIMD frame only

[Will Deacon <will@kernel.org>]

On Tue, May 06, 2025 at 04:25:06PM +0100, Mark Rutland wrote:
> On systems with SVE and/or SME, the kernel will always create SVE and
> FPSIMD signal frames when delivering a signal, but a user can manipulate
> signal frames such that a signal return only observes an FPSIMD signal
> frame. When this happens, restore_fpsimd_context() will restore state
> such that fp_type==FP_STATE_FPSIMD, but will leave PSTATE.SM as-is.
> 
> It is possible for a user to set PSTATE.SM between syscall entry and
> execution of the sigreturn logic (e.g. via ptrace), and consequently the
> sigreturn may result in the task having PSTATE.SM==1 and
> fp_type==FP_STATE_FPSIMD.
> 
> For various reasons it is not legitimate for a task to be in a state
> where PSTATE.SM==1 and fp_type==FP_STATE_FPSIMD. Portions of the user
> ABI are written with the requirement that streaming SVE state is always
> presented in SVE format rather than FPSIMD format, and as there is no
> mechanism to permit access to only the FPSIMD subset of streaming SVE
> state, streaming SVE state must always be saved and restored in SVE
> format.
> 
> Fix restore_fpsimd_context() to clear PSTATE.SM when restoring an FPSIMD
> signal frame without an SVE signal frame. This matches the current
> behaviour when an SVE signal frame is present, but the SVE signal frame
> has no register payload (e.g. as is the case on SME-only systems which
> lack SVE).
> 
> This change should have no effect for applications which do not alter
> signal frames (i.e. almost all applications). I do not expect
> non-{malicious,buggy} applications to hide the SVE signal frame, but
> I've chosen to clear PSTATE.SM rather than mandating the presence of an
> SVE signal frame in case there is some legacy (non-SME) usage that I am
> not currently aware of.
> 
> For context, the SME handling was originally introduced in commit:
> 
>   85ed24dad290 ("arm64/sme: Implement streaming SVE signal handling")
> 
> ... and subsequently updated/fixed to handle SME-only systems in commits:
> 
>   7dde62f0687c ("arm64/signal: Always accept SVE signal frames on SME only systems")
>   f26cd7372160 ("arm64/signal: Always allocate SVE signal frames on SME only systems")
> 
> Fixes: 85ed24dad290 ("arm64/sme: Implement streaming SVE signal handling")
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/kernel/signal.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 48d3c0129dade..fdce1b856f498 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -280,6 +280,7 @@ static int restore_fpsimd_context(struct user_ctxs *user)
>  	__get_user_error(fpsimd.fpcr, &(user->fpsimd->fpcr), err);
>  
>  	clear_thread_flag(TIF_SVE);
> +	current->thread.svcr &= ~SVCR_SM_MASK;
>  	current->thread.fp_type = FP_STATE_FPSIMD;

Hmm, I think we're preemptible here so do we need some compiler barriers
to make sure that the context-switching code doesn't see these fields in
an inconsistent state?

Will