[PATCH] arm64: fpsimd: Prevent registers leaking from dead tasks

[Dave Martin]

On Wed, Dec 06, 2017 at 09:57:15AM +0000, Ard Biesheuvel wrote:
> On 5 December 2017 at 14:56, Dave Martin <Dave.Martin at arm.com> wrote:

[...]

> > This patch fixes the problem by ensuring that the sched-in check
> > fails in (5): fpsimd_flush_task_state(T) is called when T is
> > forked, so that T->thread.fpsimd_state.cpu == C cannot be true.
> > This relies on the fact that T is not schedulable until after
> > copy_thread() completes.
> >
> > Once T's fpsimd state has been loaded on some CPU C there may still
> > be other cpus D for which per_cpu(fpsimd_last_state, D) ==
> > &X->thread.fpsimd_state.  But D is necessarily != C in this case,
> > and the check in (5) must fail.
> >
> > An alternative fix would be to do refcounting on task_struct.  This
> > would result in each CPU holding a reference to the last task whose
> > fpsimd state was loaded there.  It's not clear whether this is
> > preferable, and it involves higher overhead than the fix proposed
> > in this patch.  It would also moves all the task_struct freeing
> 
> move
> 
> > work into the context switch critical section, or otherwise some
> > deferred cleanup mechanism would need to be introduced, neither of
> > which seems obviously justified.
> >
> > Fixes: 005f78cd8849 ("arm64: defer reloading a task's FPSIMD state to userland resume")
> > Signed-off-by: Dave Martin <Dave.Martin at arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > ---
> >  arch/arm64/kernel/process.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > index b2adcce..5387d15 100644
> > --- a/arch/arm64/kernel/process.c
> > +++ b/arch/arm64/kernel/process.c
> > @@ -313,6 +313,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
> >          */
> >         clear_tsk_thread_flag(p, TIF_SVE);
> >         p->thread.sve_state = NULL;
> > +       fpsimd_flush_task_state(p);
> >
> >         if (likely(!(p->flags & PF_KTHREAD))) {
> >                 *childregs = *current_pt_regs();
> > --
> > 2.1.4
> >
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>

Thanks
---Dave