ARM: 7307/1: vfp: fix ptrace regset modification race

In a preemptible kernel, vfp_set() can be preempted, causing the
hardware VFP context to be switched while the thread vfp state is
being read and modified.  This leads to a race condition which can
cause the thread vfp state to become corrupted if lazy VFP context
save occurs due to preemption in between the time thread->vfpstate
is read and the time the modified state is written back.

This may occur if preemption occurs during the execution of a
ptrace() call which modifies the VFP register state of a thread.
Such instances should be very rare in most realistic scenarios --
none has been reported, so far as I am aware.  Only uniprocessor
systems should be affected, since VFP context save is not currently
lazy in SMP kernels.

The problem was introduced by my earlier patch migrating to use
regsets to implement ptrace.

This patch does a vfp_sync_hwstate() before reading
thread->vfpstate, to make sure that the thread's VFP state is not
live in the hardware registers while the registers are modified.

Thanks to Will Deacon for spotting this.

Cc: stable <stable@vger.kernel.org>
Signed-off-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index e1d5e1929fbd6a36e9266a2c4d254cba5060e438..d001be4e0ce16896539c70ccced6e0887400d2a6 100644 (file)
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -699,10 +699,13 @@ static int vfp_set(struct task_struct *target,
 {
        int ret;
        struct thread_info *thread = task_thread_info(target);
-       struct vfp_hard_struct new_vfp = thread->vfpstate.hard;
+       struct vfp_hard_struct new_vfp;
        const size_t user_fpregs_offset = offsetof(struct user_vfp, fpregs);
        const size_t user_fpscr_offset = offsetof(struct user_vfp, fpscr);
 
+       vfp_sync_hwstate(thread);
+       new_vfp = thread->vfpstate.hard;
+
        ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
                                  &new_vfp.fpregs,
                                  user_fpregs_offset,
@@ -723,7 +726,6 @@ static int vfp_set(struct task_struct *target,
        if (ret)
                return ret;
 
-       vfp_sync_hwstate(thread);
        thread->vfpstate.hard = new_vfp;
        vfp_flush_hwstate(thread);