[PATCH 19/20] arm64/fpsimd: ptrace: Gracefully handle errors

[Mark Rutland]

On Wed, May 07, 2025 at 05:32:43PM +0100, Will Deacon wrote:
> On Tue, May 06, 2025 at 04:25:22PM +0100, Mark Rutland wrote:
> > Within sve_set_common() we do not handle error conditions correctly:
> > 
> > * When writing to NT_ARM_SSVE, if sme_alloc() fails, the task will be
> >   left with task->thread.sme_state==NULL, but TIF_SME will be set and
> >   task->thread.fp_type==FP_STATE_SVE. This will result in a subsequent
> >   null pointer dereference when the task's state is loaded or otherwise
> >   manipulated.
> > 
> > * When writing to NT_ARM_SSVE, if sve_alloc() fails, the task will be
> >   left with task->thread.sve_state==NULL, but TIF_SME will be set,
> >   PSTATE.SM will be set, and task->thread.fp_type==FP_STATE_FPSIMD.
> >   This is not a legitimate state, and can result in various problems,
> >   including a subsequent null pointer dereference and/or the task
> >   inheriting stale streaming mode register state the next time its state
> >   is loaded into hardware.
> > 
> > * When writing to NT_ARM_SSVE, if the VL is changed but the resultign VL
> >   differs from that in the header, the task will be left with TIF_SME
> >   set, PSTATE.SM set, but task->thread.fp_type==FP_STATE_FPSIMD. This is
> >   not a legitimate state, and can result in various problems as
> >   described above.
> > 
> > Avoid these problems by allocating memory earlier, and by changing the
> > task's saved fp_type to FP_STATE_SVE before skipping register writes due
> > to a change of VL. To make this simpler I've pulled the flushing of task
> > state earlier and moved the setting of TIF_SVE earlier -- this will be
> > cleared when loading FPSIMD-only state, and so moving this has no
> > resulting functional change.
> 
> Doesn't flushing the state earlier mean that passing a count smaller than
> the header size is now potentially destructive to the fpsimd state?

Sorry, this is poorly worded, and I will go rewrite this for clarity.

There are two key things to bear in mind:

(1) In patch 5 ("arm64/fpsimd: ptrace: Consistently handle partial
    writes to NT_ARM_(S)SVE"), sve_set_common() was changed to always
    zero state first, so that's *always* destructive.

(2) Here, "flush" is referring to the poorly named which detaches a task
    from live/stale state on any CPU, and does not modify the saved
    state in any way. For the ptrace set_*() functions, we know that the
    tracee is blocked (and its state has already been saved), so calling
    that earlier doesn't result in any functional change.

> > When changlnig the
> 
> This ends mid-sentence and 'changlnig' sounds like a Doors tune.

I've deleted this (and fix the "resultign" typo above).

Mark.