| Message ID | 3279c8577c3f76c14ed66a49a613a32408f14182.1524687471.git.joseph.salisbury@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | powerpc/tm: Fix endianness flip on trap | expand |
On 04/27/18 19:20, Joseph Salisbury wrote: > From: Gustavo Romero <gromero@linux.vnet.ibm.com> > > BugLink: http://bugs.launchpad.net/bugs/1762928 > > Currently it's possible that a thread on PPC64 LE has its endianness > flipped inadvertently to Big-Endian resulting in a crash once the process > is back from the signal handler. > > If giveup_all() is called when regs->msr has the bits MSR.FP and MSR.VEC > disabled (and hence MSR.VSX disabled too) it returns without calling > check_if_tm_restore_required() which copies regs->msr to ckpt_regs->msr if > the process caught a signal whilst in transactional mode. Then once in > setup_tm_sigcontexts() MSR from ckpt_regs.msr is used, but since > check_if_tm_restore_required() was not called previuosly, gp_regs[PT_MSR] > gets a copy of invalid MSR bits as MSR in ckpt_regs was not updated from > regs->msr and so is zeroed. Later when leaving the signal handler once in > sys_rt_sigreturn() the TS bits of gp_regs[PT_MSR] are checked to determine > if restore_tm_sigcontexts() must be called to pull in the correct MSR state > into the user context. Because TS bits are zeroed > restore_tm_sigcontexts() is never called and MSR restored from the user > context on returning from the signal handler has the MSR.LE (the endianness > bit) forced to zero (Big-Endian). That leads, for instance, to 'nop' being > treated as an illegal instruction in the following sequence: > > tbegin. > beq 1f > trap > tend. > 1: nop > > on PPC64 LE machines and the process dies just after returning from the > signal handler. > > PPC64 BE is also affected but in a subtle way since forcing Big-Endian on > a BE machine does not change the endianness. > > This commit fixes the issue described above by ensuring that once in > setup_tm_sigcontexts() the MSR used is from regs->msr instead of from > ckpt_regs->msr and by ensuring that we pull in only the MSR.FP, MSR.VEC, > and MSR.VSX bits from ckpt_regs->msr. > > The fix was tested both on LE and BE machines and no regression regarding > the powerpc/tm selftests was observed. > > Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > (back ported from commit 1c200e63d055ec0125e44a5e386b9b78aada7eb3) > Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> The backport looks fine, tested by the reporter. Thanks, Kleber > --- > arch/powerpc/kernel/signal_64.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c > index b2c0029..3726e40 100644 > --- a/arch/powerpc/kernel/signal_64.c > +++ b/arch/powerpc/kernel/signal_64.c > @@ -207,13 +207,19 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc, > elf_vrreg_t __user *tm_v_regs = sigcontext_vmx_regs(tm_sc); > #endif > struct pt_regs *regs = tsk->thread.regs; > - unsigned long msr = tsk->thread.ckpt_regs.msr; > + unsigned long msr = tsk->thread.regs->msr; > long err = 0; > > BUG_ON(tsk != current); > > BUG_ON(!MSR_TM_ACTIVE(regs->msr)); > > + /* Restore checkpointed FP, VEC, and VSX bits from ckpt_regs as > + * it contains the correct FP, VEC, VSX state after we treclaimed > + * the transaction and giveup_all() was called on reclaiming. > + */ > + msr |= tsk->thread.ckpt_regs.msr & (MSR_FP | MSR_VEC | MSR_VSX); > + > /* Remove TM bits from thread's MSR. The MSR in the sigcontext > * just indicates to userland that we were doing a transaction, but we > * don't want to return in transactional state. This also ensures >
On 27.04.2018 19:20, Joseph Salisbury wrote: > From: Gustavo Romero <gromero@linux.vnet.ibm.com> > > BugLink: http://bugs.launchpad.net/bugs/1762928 > > Currently it's possible that a thread on PPC64 LE has its endianness > flipped inadvertently to Big-Endian resulting in a crash once the process > is back from the signal handler. > > If giveup_all() is called when regs->msr has the bits MSR.FP and MSR.VEC > disabled (and hence MSR.VSX disabled too) it returns without calling > check_if_tm_restore_required() which copies regs->msr to ckpt_regs->msr if > the process caught a signal whilst in transactional mode. Then once in > setup_tm_sigcontexts() MSR from ckpt_regs.msr is used, but since > check_if_tm_restore_required() was not called previuosly, gp_regs[PT_MSR] > gets a copy of invalid MSR bits as MSR in ckpt_regs was not updated from > regs->msr and so is zeroed. Later when leaving the signal handler once in > sys_rt_sigreturn() the TS bits of gp_regs[PT_MSR] are checked to determine > if restore_tm_sigcontexts() must be called to pull in the correct MSR state > into the user context. Because TS bits are zeroed > restore_tm_sigcontexts() is never called and MSR restored from the user > context on returning from the signal handler has the MSR.LE (the endianness > bit) forced to zero (Big-Endian). That leads, for instance, to 'nop' being > treated as an illegal instruction in the following sequence: > > tbegin. > beq 1f > trap > tend. > 1: nop > > on PPC64 LE machines and the process dies just after returning from the > signal handler. > > PPC64 BE is also affected but in a subtle way since forcing Big-Endian on > a BE machine does not change the endianness. > > This commit fixes the issue described above by ensuring that once in > setup_tm_sigcontexts() the MSR used is from regs->msr instead of from > ckpt_regs->msr and by ensuring that we pull in only the MSR.FP, MSR.VEC, > and MSR.VSX bits from ckpt_regs->msr. > > The fix was tested both on LE and BE machines and no regression regarding > the powerpc/tm selftests was observed. > > Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > (back ported from commit 1c200e63d055ec0125e44a5e386b9b78aada7eb3) > Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com> According to the LP bug report this has been retracted to be applied to artful. -Stefan > --- > arch/powerpc/kernel/signal_64.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c > index b2c0029..3726e40 100644 > --- a/arch/powerpc/kernel/signal_64.c > +++ b/arch/powerpc/kernel/signal_64.c > @@ -207,13 +207,19 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc, > elf_vrreg_t __user *tm_v_regs = sigcontext_vmx_regs(tm_sc); > #endif > struct pt_regs *regs = tsk->thread.regs; > - unsigned long msr = tsk->thread.ckpt_regs.msr; > + unsigned long msr = tsk->thread.regs->msr; > long err = 0; > > BUG_ON(tsk != current); > > BUG_ON(!MSR_TM_ACTIVE(regs->msr)); > > + /* Restore checkpointed FP, VEC, and VSX bits from ckpt_regs as > + * it contains the correct FP, VEC, VSX state after we treclaimed > + * the transaction and giveup_all() was called on reclaiming. > + */ > + msr |= tsk->thread.ckpt_regs.msr & (MSR_FP | MSR_VEC | MSR_VSX); > + > /* Remove TM bits from thread's MSR. The MSR in the sigcontext > * just indicates to userland that we were doing a transaction, but we > * don't want to return in transactional state. This also ensures >
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c index b2c0029..3726e40 100644 --- a/arch/powerpc/kernel/signal_64.c +++ b/arch/powerpc/kernel/signal_64.c @@ -207,13 +207,19 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc, elf_vrreg_t __user *tm_v_regs = sigcontext_vmx_regs(tm_sc); #endif struct pt_regs *regs = tsk->thread.regs; - unsigned long msr = tsk->thread.ckpt_regs.msr; + unsigned long msr = tsk->thread.regs->msr; long err = 0; BUG_ON(tsk != current); BUG_ON(!MSR_TM_ACTIVE(regs->msr)); + /* Restore checkpointed FP, VEC, and VSX bits from ckpt_regs as + * it contains the correct FP, VEC, VSX state after we treclaimed + * the transaction and giveup_all() was called on reclaiming. + */ + msr |= tsk->thread.ckpt_regs.msr & (MSR_FP | MSR_VEC | MSR_VSX); + /* Remove TM bits from thread's MSR. The MSR in the sigcontext * just indicates to userland that we were doing a transaction, but we * don't want to return in transactional state. This also ensures