Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

arm64/fpsimd: signal: Fix restoration of SVE context

When SME is supported, Restoring SVE signal context can go wrong in a
few ways, including placing the task into an invalid state where the
kernel may read from out-of-bounds memory (and may potentially take a
fatal fault) and/or may kill the task with a SIGKILL.

(1) Restoring a context with SVE_SIG_FLAG_SM set can place the task into
an invalid state where SVCR.SM is set (and sve_state is non-NULL)
but TIF_SME is clear, consequently resuting in out-of-bounds memory
reads and/or killing the task with SIGKILL.

This can only occur in unusual (but legitimate) cases where the SVE
signal context has either been modified by userspace or was saved in
the context of another task (e.g. as with CRIU), as otherwise the
presence of an SVE signal context with SVE_SIG_FLAG_SM implies that
TIF_SME is already set.

While in this state, task_fpsimd_load() will NOT configure SMCR_ELx
(leaving some arbitrary value configured in hardware) before
restoring SVCR and attempting to restore the streaming mode SVE
registers from memory via sve_load_state(). As the value of
SMCR_ELx.LEN may be larger than the task's streaming SVE vector
length, this may read memory outside of the task's allocated
sve_state, reading unrelated data and/or triggering a fault.

While this can result in secrets being loaded into streaming SVE
registers, these values are never exposed. As TIF_SME is clear,
fpsimd_bind_task_to_cpu() will configure CPACR_ELx.SMEN to trap EL0
accesses to streaming mode SVE registers, so these cannot be
accessed directly at EL0. As fpsimd_save_user_state() verifies the
live vector length before saving (S)SVE state to memory, no secret
values can be saved back to memory (and hence cannot be observed via
ptrace, signals, etc).

When the live vector length doesn't match the expected vector length
for the task, fpsimd_save_user_state() will send a fatal SIGKILL
signal to the task. Hence the task may be killed after executing
userspace for some period of time.

(2) Restoring a context with SVE_SIG_FLAG_SM clear does not clear the
task's SVCR.SM. If SVCR.SM was set prior to restoring the context,
then the task will be left in streaming mode unexpectedly, and some
register state will be combined inconsistently, though the task will
be left in legitimate state from the kernel's PoV.

This can only occur in unusual (but legitimate) cases where ptrace
has been used to set SVCR.SM after entry to the sigreturn syscall,
as syscall entry clears SVCR.SM.

In these cases, the the provided SVE register data will be loaded
into the task's sve_state using the non-streaming SVE vector length
and the FPSIMD registers will be merged into this using the
streaming SVE vector length.

Fix (1) by setting TIF_SME when setting SVCR.SM. This also requires
ensuring that the task's sme_state has been allocated, but as this could
contain live ZA state, it should not be zeroed. Fix (2) by clearing
SVCR.SM when restoring a SVE signal context with SVE_SIG_FLAG_SM clear.

For consistency, I've pulled the manipulation of SVCR, TIF_SVE, TIF_SME,
and fp_type earlier, immediately after the allocation of
sve_state/sme_state, before the restore of the actual register state.
This makes it easier to ensure that these are always modified
consistently, even if a fault is taken while reading the register data
from the signal context. I do not expect any software to depend on the
exact state restored when a fault is taken while reading the context.

Fixes: 85ed24dad290 ("arm64/sme: Implement streaming SVE signal handling")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: <stable@vger.kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

authored by

Mark Rutland and committed by
Catalin Marinas
d2907cbe ea8ccfdd

+16 -6
+16 -6
arch/arm64/kernel/signal.c
··· 449 449 if (user->sve_size < SVE_SIG_CONTEXT_SIZE(vq)) 450 450 return -EINVAL; 451 451 452 + if (sm) { 453 + sme_alloc(current, false); 454 + if (!current->thread.sme_state) 455 + return -ENOMEM; 456 + } 457 + 452 458 sve_alloc(current, true); 453 459 if (!current->thread.sve_state) { 454 460 clear_thread_flag(TIF_SVE); 455 461 return -ENOMEM; 456 462 } 463 + 464 + if (sm) { 465 + current->thread.svcr |= SVCR_SM_MASK; 466 + set_thread_flag(TIF_SME); 467 + } else { 468 + current->thread.svcr &= ~SVCR_SM_MASK; 469 + set_thread_flag(TIF_SVE); 470 + } 471 + 472 + current->thread.fp_type = FP_STATE_SVE; 457 473 458 474 err = __copy_from_user(current->thread.sve_state, 459 475 (char __user const *)user->sve + ··· 477 461 SVE_SIG_REGS_SIZE(vq)); 478 462 if (err) 479 463 return -EFAULT; 480 - 481 - if (flags & SVE_SIG_FLAG_SM) 482 - current->thread.svcr |= SVCR_SM_MASK; 483 - else 484 - set_thread_flag(TIF_SVE); 485 - current->thread.fp_type = FP_STATE_SVE; 486 464 487 465 err = read_fpsimd_context(&fpsimd, user); 488 466 if (err)