[SRU][B][D][Patch 1/2] powerpc/tm: Fix FP/VMX unavailable exceptions inside a transaction

Tyler Hicks tyhicks at canonical.com
Wed Sep 11 15:26:56 UTC 2019


On 2019-09-11 15:58:36, frank.heimes at canonical.com wrote:
> From: Gustavo Romero <gromero at linux.ibm.com>
> 
> CVE-2019-15030
> 
> BugLink: https://bugs.launchpad.net/bugs/184353

This BugLink is wrong. It is missing a trailing '3'. Should be:

BugLink: https://bugs.launchpad.net/bugs/1843533

Tyler

> 
> When we take an FP unavailable exception in a transaction we have to
> account for the hardware FP TM checkpointed registers being
> incorrect. In this case for this process we know the current and
> checkpointed FP registers must be the same (since FP wasn't used
> inside the transaction) hence in the thread_struct we copy the current
> FP registers to the checkpointed ones.
> 
> This copy is done in tm_reclaim_thread(). We use thread->ckpt_regs.msr
> to determine if FP was on when in userspace. thread->ckpt_regs.msr
> represents the state of the MSR when exiting userspace. This is setup
> by check_if_tm_restore_required().
> 
> Unfortunatley there is an optimisation in giveup_all() which returns
> early if tsk->thread.regs->msr (via local variable `usermsr`) has
> FP=VEC=VSX=SPE=0. This optimisation means that
> check_if_tm_restore_required() is not called and hence
> thread->ckpt_regs.msr is not updated and will contain an old value.
> 
> This can happen if due to load_fp=255 we start a userspace process
> with MSR FP=1 and then we are context switched out. In this case
> thread->ckpt_regs.msr will contain FP=1. If that same process is then
> context switched in and load_fp overflows, MSR will have FP=0. If that
> process now enters a transaction and does an FP instruction, the FP
> unavailable will not update thread->ckpt_regs.msr (the bug) and MSR
> FP=1 will be retained in thread->ckpt_regs.msr.  tm_reclaim_thread()
> will then not perform the required memcpy and the checkpointed FP regs
> in the thread struct will contain the wrong values.
> 
> The code path for this happening is:
> 
>        Userspace:                      Kernel
>                    Start userspace
>                     with MSR FP/VEC/VSX/SPE=0 TM=1
>                       < -----
>        ...
>        tbegin
>        bne
>        fp instruction
>                    FP unavailable
>                        ---- >
>                                         fp_unavailable_tm()
> 					  tm_reclaim_current()
> 					    tm_reclaim_thread()
> 					      giveup_all()
> 					        return early since FP/VMX/VSX=0
> 						/* ckpt MSR not updated (Incorrect) */
> 					      tm_reclaim()
> 					        /* thread_struct ckpt FP regs contain junk (OK) */
>                                               /* Sees ckpt MSR FP=1 (Incorrect) */
> 					      no memcpy() performed
> 					        /* thread_struct ckpt FP regs not fixed (Incorrect) */
> 					  tm_recheckpoint()
> 					     /* Put junk in hardware checkpoint FP regs */
>                                          ....
>                       < -----
>                    Return to userspace
>                      with MSR TM=1 FP=1
>                      with junk in the FP TM checkpoint
>        TM rollback
>        reads FP junk
> 
> This is a data integrity problem for the current process as the FP
> registers are corrupted. It's also a security problem as the FP
> registers from one process may be leaked to another.
> 
> This patch moves up check_if_tm_restore_required() in giveup_all() to
> ensure thread->ckpt_regs.msr is updated correctly.
> 
> A simple testcase to replicate this will be posted to
> tools/testing/selftests/powerpc/tm/tm-poison.c
> 
> Similarly for VMX.
> 
> This fixes CVE-2019-15030.
> 
> Fixes: f48e91e87e67 ("powerpc/tm: Fix FP and VMX register corruption")
> Cc: stable at vger.kernel.org # 4.12+
> Signed-off-by: Gustavo Romero <gromero at linux.vnet.ibm.com>
> Signed-off-by: Michael Neuling <mikey at neuling.org>
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> Link: https://lore.kernel.org/r/20190904045529.23002-1-gromero@linux.vnet.ibm.com
> (cherry picked from commit 8205d5d98ef7f155de211f5e2eb6ca03d95a5a60)
> Signed-off-by: Frank Heimes <frank.heimes at canonical.com>
> ---
>  arch/powerpc/kernel/process.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 06a5699..4538bf2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -496,13 +496,14 @@ void giveup_all(struct task_struct *tsk)
>  	if (!tsk->thread.regs)
>  		return;
>  
> +	check_if_tm_restore_required(tsk);
> +
>  	usermsr = tsk->thread.regs->msr;
>  
>  	if ((usermsr & msr_all_available) == 0)
>  		return;
>  
>  	msr_check_and_set(msr_all_available);
> -	check_if_tm_restore_required(tsk);
>  
>  	WARN_ON((usermsr & MSR_VSX) && !((usermsr & MSR_FP) && (usermsr & MSR_VEC)));
>  
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team



More information about the kernel-team mailing list