Skip to content

Commit b87c8c4

Browse files
mrutland-armwilldeacon
authored andcommitted
arm64/fpsimd: ptrace/prctl: Ensure VL changes leave task in a valid state
Currently, vec_set_vector_length() can manipulate a task into an invalid state as a result of a prctl/ptrace syscall which changes the SVE/SME vector length, resulting in several problems: (1) When changing the SVE vector length, if the task initially has PSTATE.ZA==1, and sve_alloc() fails to allocate memory, the task will be left with PSTATE.ZA==1 and sve_state==NULL. This is not a legitimate state, and could result in a subsequent null pointer dereference. (2) When changing the SVE vector length, if the task initially has PSTATE.SM==1, the task will be left with PSTATE.SM==1 and fp_type==FP_STATE_FPSIMD. Streaming mode state always needs to be saved in SVE format, so this is not a legitimate state. Attempting to restore this state may cause a task to erroneously inherit stale streaming mode predicate registers and FFR contents, behaving non-deterministically and potentially leaving information from another task. While in this state, reads of the NT_ARM_SSVE regset will indicate that the registers are not stored in SVE format. For the NT_ARM_SSVE regset specifically, debuggers interpret this as meaning that PSTATE.SM==0. (3) When changing the SME vector length, if the task initially has PSTATE.SM==1, the lower 128 bits of task's streaming mode vector state will be migrated to non-streaming mode, rather than these bits being zeroed as is usually the case for changes to PSTATE.SM. To fix the first issue, we can eagerly allocate the new sve_state and sme_state before modifying the task. This makes it possible to handle memory allocation failure without modifying the task state at all, and removes the need to clear TIF_SVE and TIF_SME. To fix the second issue, we either need to clear PSTATE.SM or not change the saved fp_type. Given we're going to eagerly allocate sve_state and sme_state, the simplest option is to preserve PSTATE.SM and the saves fp_type, and consistently truncate the SVE state. This ensures that the task always stays in a valid state, and by virtue of not exiting streaming mode, this also sidesteps the third issue. I believe these changes should not be problematic for realistic usage: * When the SVE/SME vector length is changed via prctl(), syscall entry will have cleared PSTATE.SM. Unless the task's state has been manipulated via ptrace after entry, the task will have PSTATE.SM==0. * When the SVE/SME vector length is changed via a write to the NT_ARM_SVE or NT_ARM_SSVE regsets, PSTATE.SM will be forced immediately after the length change, and new vector state will be copied from userspace. * When the SME vector length is changed via a write to the NT_ARM_ZA regset, the (S)SVE state is clobbered today, so anyone who cares about the specific state would need to install this after writing to the NT_ARM_ZA regset. As we need to free the old SVE state while TIF_SVE may still be set, we cannot use sve_free(), and using kfree() directly makes it clear that the free pairs with the subsequent assignment. As this leaves sve_free() unused, I've removed the existing sve_free() and renamed __sve_free() to mirror sme_free(). Fixes: 8bd7f91 ("arm64/sme: Implement traps and syscall handling for SME") Fixes: baa8515 ("arm64/fpsimd: Track the saved FPSIMD state type separately to TIF_SVE") Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: David Spickett <david.spickett@arm.com> Cc: Luis Machado <luis.machado@arm.com> Cc: Marc Zyngier <maz@kernel.org> Cc: Mark Brown <broonie@kernel.org> Cc: Will Deacon <will@kernel.org> Link: https://lore.kernel.org/r/20250508132644.1395904-16-mark.rutland@arm.com Signed-off-by: Will Deacon <will@kernel.org>
1 parent 49ce484 commit b87c8c4

File tree

2 files changed

+73
-66
lines changed

2 files changed

+73
-66
lines changed

Documentation/arch/arm64/sme.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ prctl(PR_SME_SET_VL, unsigned long arg)
241241
length, or calling PR_SME_SET_VL with the PR_SME_SET_VL_ONEXEC flag,
242242
does not constitute a change to the vector length for this purpose.
243243

244-
* Changing the vector length causes PSTATE.ZA and PSTATE.SM to be cleared.
244+
* Changing the vector length causes PSTATE.ZA to be cleared.
245245
Calling PR_SME_SET_VL with vl equal to the thread's current vector
246246
length, or calling PR_SME_SET_VL with the PR_SME_SET_VL_ONEXEC flag,
247247
does not constitute a change to the vector length for this purpose.

arch/arm64/kernel/fpsimd.c

Lines changed: 72 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -724,23 +724,12 @@ void cpu_enable_fpmr(const struct arm64_cpu_capabilities *__always_unused p)
724724
}
725725

726726
#ifdef CONFIG_ARM64_SVE
727-
/*
728-
* Call __sve_free() directly only if you know task can't be scheduled
729-
* or preempted.
730-
*/
731-
static void __sve_free(struct task_struct *task)
727+
static void sve_free(struct task_struct *task)
732728
{
733729
kfree(task->thread.sve_state);
734730
task->thread.sve_state = NULL;
735731
}
736732

737-
static void sve_free(struct task_struct *task)
738-
{
739-
WARN_ON(test_tsk_thread_flag(task, TIF_SVE));
740-
741-
__sve_free(task);
742-
}
743-
744733
/*
745734
* Ensure that task->thread.sve_state is allocated and sufficiently large.
746735
*
@@ -801,10 +790,73 @@ void fpsimd_sync_to_effective_state_zeropad(struct task_struct *task)
801790
__fpsimd_to_sve(sst, fst, vq);
802791
}
803792

793+
static int change_live_vector_length(struct task_struct *task,
794+
enum vec_type type,
795+
unsigned long vl)
796+
{
797+
unsigned int sve_vl = task_get_sve_vl(task);
798+
unsigned int sme_vl = task_get_sme_vl(task);
799+
void *sve_state = NULL, *sme_state = NULL;
800+
801+
if (type == ARM64_VEC_SME)
802+
sme_vl = vl;
803+
else
804+
sve_vl = vl;
805+
806+
/*
807+
* Allocate the new sve_state and sme_state before freeing the old
808+
* copies so that allocation failure can be handled without needing to
809+
* mutate the task's state in any way.
810+
*
811+
* Changes to the SVE vector length must not discard live ZA state or
812+
* clear PSTATE.ZA, as userspace code which is unaware of the AAPCS64
813+
* ZA lazy saving scheme may attempt to change the SVE vector length
814+
* while unsaved/dormant ZA state exists.
815+
*/
816+
sve_state = kzalloc(__sve_state_size(sve_vl, sme_vl), GFP_KERNEL);
817+
if (!sve_state)
818+
goto out_mem;
819+
820+
if (type == ARM64_VEC_SME) {
821+
sme_state = kzalloc(__sme_state_size(sme_vl), GFP_KERNEL);
822+
if (!sme_state)
823+
goto out_mem;
824+
}
825+
826+
if (task == current)
827+
fpsimd_save_and_flush_current_state();
828+
else
829+
fpsimd_flush_task_state(task);
830+
831+
/*
832+
* Always preserve PSTATE.SM and the effective FPSIMD state, zeroing
833+
* other SVE state.
834+
*/
835+
fpsimd_sync_from_effective_state(task);
836+
task_set_vl(task, type, vl);
837+
kfree(task->thread.sve_state);
838+
task->thread.sve_state = sve_state;
839+
fpsimd_sync_to_effective_state_zeropad(task);
840+
841+
if (type == ARM64_VEC_SME) {
842+
task->thread.svcr &= ~SVCR_ZA_MASK;
843+
kfree(task->thread.sme_state);
844+
task->thread.sme_state = sme_state;
845+
}
846+
847+
return 0;
848+
849+
out_mem:
850+
kfree(sve_state);
851+
kfree(sme_state);
852+
return -ENOMEM;
853+
}
854+
804855
int vec_set_vector_length(struct task_struct *task, enum vec_type type,
805856
unsigned long vl, unsigned long flags)
806857
{
807-
bool free_sme = false;
858+
bool onexec = flags & PR_SVE_SET_VL_ONEXEC;
859+
bool inherit = flags & PR_SVE_VL_INHERIT;
808860

809861
if (flags & ~(unsigned long)(PR_SVE_VL_INHERIT |
810862
PR_SVE_SET_VL_ONEXEC))
@@ -824,62 +876,17 @@ int vec_set_vector_length(struct task_struct *task, enum vec_type type,
824876

825877
vl = find_supported_vector_length(type, vl);
826878

827-
if (flags & (PR_SVE_VL_INHERIT |
828-
PR_SVE_SET_VL_ONEXEC))
879+
if (!onexec && vl != task_get_vl(task, type)) {
880+
if (change_live_vector_length(task, type, vl))
881+
return -ENOMEM;
882+
}
883+
884+
if (onexec || inherit)
829885
task_set_vl_onexec(task, type, vl);
830886
else
831887
/* Reset VL to system default on next exec: */
832888
task_set_vl_onexec(task, type, 0);
833889

834-
/* Only actually set the VL if not deferred: */
835-
if (flags & PR_SVE_SET_VL_ONEXEC)
836-
goto out;
837-
838-
if (vl == task_get_vl(task, type))
839-
goto out;
840-
841-
/*
842-
* To ensure the FPSIMD bits of the SVE vector registers are preserved,
843-
* write any live register state back to task_struct, and convert to a
844-
* regular FPSIMD thread.
845-
*/
846-
if (task == current) {
847-
get_cpu_fpsimd_context();
848-
849-
fpsimd_save_user_state();
850-
}
851-
852-
fpsimd_flush_task_state(task);
853-
if (test_and_clear_tsk_thread_flag(task, TIF_SVE) ||
854-
thread_sm_enabled(&task->thread)) {
855-
fpsimd_sync_from_effective_state(task);
856-
task->thread.fp_type = FP_STATE_FPSIMD;
857-
}
858-
859-
if (system_supports_sme() && type == ARM64_VEC_SME) {
860-
task->thread.svcr &= ~(SVCR_SM_MASK | SVCR_ZA_MASK);
861-
clear_tsk_thread_flag(task, TIF_SME);
862-
free_sme = true;
863-
}
864-
865-
if (task == current)
866-
put_cpu_fpsimd_context();
867-
868-
task_set_vl(task, type, vl);
869-
870-
/*
871-
* Free the changed states if they are not in use, SME will be
872-
* reallocated to the correct size on next use and we just
873-
* allocate SVE now in case it is needed for use in streaming
874-
* mode.
875-
*/
876-
sve_free(task);
877-
sve_alloc(task, true);
878-
879-
if (free_sme)
880-
sme_free(task);
881-
882-
out:
883890
update_tsk_thread_flag(task, vec_vl_inherit_flag(type),
884891
flags & PR_SVE_VL_INHERIT);
885892

@@ -1175,7 +1182,7 @@ void __init sve_setup(void)
11751182
*/
11761183
void fpsimd_release_task(struct task_struct *dead_task)
11771184
{
1178-
__sve_free(dead_task);
1185+
sve_free(dead_task);
11791186
sme_free(dead_task);
11801187
}
11811188

0 commit comments

Comments
 (0)