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: 8bd7f91c03d8 ("arm64/sme: Implement traps and syscall handling for SME")
Fixes: baa8515281b3 ("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>

diff --git a/Documentation/arch/arm64/sme.rst b/Documentation/arch/arm64/sme.rst
index 1c1e48d8bd1a58f9cdc72df3d5b41aa75802f513..4cb38330e7046b881118de69b4231c109874f286 100644 (file)
--- a/Documentation/arch/arm64/sme.rst
+++ b/Documentation/arch/arm64/sme.rst
@@ -241,7 +241,7 @@ prctl(PR_SME_SET_VL, unsigned long arg)
       length, or calling PR_SME_SET_VL with the PR_SME_SET_VL_ONEXEC flag,
       does not constitute a change to the vector length for this purpose.
 
-    * Changing the vector length causes PSTATE.ZA and PSTATE.SM to be cleared.
+    * Changing the vector length causes PSTATE.ZA to be cleared.
       Calling PR_SME_SET_VL with vl equal to the thread's current vector
       length, or calling PR_SME_SET_VL with the PR_SME_SET_VL_ONEXEC flag,
       does not constitute a change to the vector length for this purpose.

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index faeedaab0558e16c154244a263b26cc5513cadf7..9c0d1068e7f2860950b940e19cf125512cff31df 100644 (file)
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -724,23 +724,12 @@ void cpu_enable_fpmr(const struct arm64_cpu_capabilities *__always_unused p)
 }
 
 #ifdef CONFIG_ARM64_SVE
-/*
- * Call __sve_free() directly only if you know task can't be scheduled
- * or preempted.
- */
-static void __sve_free(struct task_struct *task)
+static void sve_free(struct task_struct *task)
 {
        kfree(task->thread.sve_state);
        task->thread.sve_state = NULL;
 }
 
-static void sve_free(struct task_struct *task)
-{
-       WARN_ON(test_tsk_thread_flag(task, TIF_SVE));
-
-       __sve_free(task);
-}
-
 /*
  * Ensure that task->thread.sve_state is allocated and sufficiently large.
  *
@@ -801,10 +790,73 @@ void fpsimd_sync_to_effective_state_zeropad(struct task_struct *task)
        __fpsimd_to_sve(sst, fst, vq);
 }
 
+static int change_live_vector_length(struct task_struct *task,
+                                    enum vec_type type,
+                                    unsigned long vl)
+{
+       unsigned int sve_vl = task_get_sve_vl(task);
+       unsigned int sme_vl = task_get_sme_vl(task);
+       void *sve_state = NULL, *sme_state = NULL;
+
+       if (type == ARM64_VEC_SME)
+               sme_vl = vl;
+       else
+               sve_vl = vl;
+
+       /*
+        * Allocate the new sve_state and sme_state before freeing the old
+        * copies so that allocation failure can be handled without needing to
+        * mutate the task's state in any way.
+        *
+        * Changes to the SVE vector length must not discard live ZA state or
+        * clear PSTATE.ZA, as userspace code which is unaware of the AAPCS64
+        * ZA lazy saving scheme may attempt to change the SVE vector length
+        * while unsaved/dormant ZA state exists.
+        */
+       sve_state = kzalloc(__sve_state_size(sve_vl, sme_vl), GFP_KERNEL);
+       if (!sve_state)
+               goto out_mem;
+
+       if (type == ARM64_VEC_SME) {
+               sme_state = kzalloc(__sme_state_size(sme_vl), GFP_KERNEL);
+               if (!sme_state)
+                       goto out_mem;
+       }
+
+       if (task == current)
+               fpsimd_save_and_flush_current_state();
+       else
+               fpsimd_flush_task_state(task);
+
+       /*
+        * Always preserve PSTATE.SM and the effective FPSIMD state, zeroing
+        * other SVE state.
+        */
+       fpsimd_sync_from_effective_state(task);
+       task_set_vl(task, type, vl);
+       kfree(task->thread.sve_state);
+       task->thread.sve_state = sve_state;
+       fpsimd_sync_to_effective_state_zeropad(task);
+
+       if (type == ARM64_VEC_SME) {
+               task->thread.svcr &= ~SVCR_ZA_MASK;
+               kfree(task->thread.sme_state);
+               task->thread.sme_state = sme_state;
+       }
+
+       return 0;
+
+out_mem:
+       kfree(sve_state);
+       kfree(sme_state);
+       return -ENOMEM;
+}
+
 int vec_set_vector_length(struct task_struct *task, enum vec_type type,
                          unsigned long vl, unsigned long flags)
 {
-       bool free_sme = false;
+       bool onexec = flags & PR_SVE_SET_VL_ONEXEC;
+       bool inherit = flags & PR_SVE_VL_INHERIT;
 
        if (flags & ~(unsigned long)(PR_SVE_VL_INHERIT |
                                     PR_SVE_SET_VL_ONEXEC))
@@ -824,62 +876,17 @@ int vec_set_vector_length(struct task_struct *task, enum vec_type type,
 
        vl = find_supported_vector_length(type, vl);
 
-       if (flags & (PR_SVE_VL_INHERIT |
-                    PR_SVE_SET_VL_ONEXEC))
+       if (!onexec && vl != task_get_vl(task, type)) {
+               if (change_live_vector_length(task, type, vl))
+                       return -ENOMEM;
+       }
+
+       if (onexec || inherit)
                task_set_vl_onexec(task, type, vl);
        else
                /* Reset VL to system default on next exec: */
                task_set_vl_onexec(task, type, 0);
 
-       /* Only actually set the VL if not deferred: */
-       if (flags & PR_SVE_SET_VL_ONEXEC)
-               goto out;
-
-       if (vl == task_get_vl(task, type))
-               goto out;
-
-       /*
-        * To ensure the FPSIMD bits of the SVE vector registers are preserved,
-        * write any live register state back to task_struct, and convert to a
-        * regular FPSIMD thread.
-        */
-       if (task == current) {
-               get_cpu_fpsimd_context();
-
-               fpsimd_save_user_state();
-       }
-
-       fpsimd_flush_task_state(task);
-       if (test_and_clear_tsk_thread_flag(task, TIF_SVE) ||
-           thread_sm_enabled(&task->thread)) {
-               fpsimd_sync_from_effective_state(task);
-               task->thread.fp_type = FP_STATE_FPSIMD;
-       }
-
-       if (system_supports_sme() && type == ARM64_VEC_SME) {
-               task->thread.svcr &= ~(SVCR_SM_MASK | SVCR_ZA_MASK);
-               clear_tsk_thread_flag(task, TIF_SME);
-               free_sme = true;
-       }
-
-       if (task == current)
-               put_cpu_fpsimd_context();
-
-       task_set_vl(task, type, vl);
-
-       /*
-        * Free the changed states if they are not in use, SME will be
-        * reallocated to the correct size on next use and we just
-        * allocate SVE now in case it is needed for use in streaming
-        * mode.
-        */
-       sve_free(task);
-       sve_alloc(task, true);
-
-       if (free_sme)
-               sme_free(task);
-
-out:
        update_tsk_thread_flag(task, vec_vl_inherit_flag(type),
                               flags & PR_SVE_VL_INHERIT);
 
@@ -1175,7 +1182,7 @@ void __init sve_setup(void)
  */
 void fpsimd_release_task(struct task_struct *dead_task)
 {
-       __sve_free(dead_task);
+       sve_free(dead_task);
        sme_free(dead_task);
 }