| Message ID | 1568210317-11793-2-git-send-email-frank.heimes@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix FP/VMX vulnerabilities - CVE-2019-15030 and CVE-2019-15031 (LP: 1843533) | expand |
On 2019-09-11 15:58:36, frank.heimes@canonical.com wrote: > From: Gustavo Romero <gromero@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@vger.kernel.org # 4.12+ > Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com> > Signed-off-by: Michael Neuling <mikey@neuling.org> > Signed-off-by: Michael Ellerman <mpe@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@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@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
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)));