[Bionic][PATCH 0/1] powerpc/tm: Fix endianness flip on trap

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bionic][PATCH 0/1] powerpc/tm: Fix endianness flip on trap

Joseph Salisbury-3
BugLink: http://bugs.launchpad.net/bugs/1762928

== Bionic Justification ==
IBM is requesting this commit in Bionic because the tm_trap_test failed on
P8 compat mode. Guests on a P9 host running bionic final beta version.

Commit 1c200e63d055 is in mailine as of v4.16-rc1.

== Fix ==
1c200e63d055 ("powerpc/tm: Fix endianness flip on trap")

Gustavo Romero (1):
  powerpc/tm: Fix endianness flip on trap

 arch/powerpc/kernel/signal_64.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Bionic][PATCH 1/1] powerpc/tm: Fix endianness flip on trap

Joseph Salisbury-3
From: Gustavo Romero <[hidden email]>

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 <[hidden email]>
Signed-off-by: Michael Ellerman <[hidden email]>
(cherry picked from commit 1c200e63d055ec0125e44a5e386b9b78aada7eb3)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 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 4b9ca35..b1b9962 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -207,7 +207,7 @@ 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);
@@ -216,6 +216,12 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 
  WARN_ON(tm_suspend_disabled);
 
+ /* 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
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [Bionic][PATCH 0/1] powerpc/tm: Fix endianness flip on trap

Seth Forshee
In reply to this post by Joseph Salisbury-3
On Wed, Apr 11, 2018 at 11:58:59AM -0400, Joseph Salisbury wrote:

> BugLink: http://bugs.launchpad.net/bugs/1762928
>
> == Bionic Justification ==
> IBM is requesting this commit in Bionic because the tm_trap_test failed on
> P8 compat mode. Guests on a P9 host running bionic final beta version.
>
> Commit 1c200e63d055 is in mailine as of v4.16-rc1.
>
> == Fix ==
> 1c200e63d055 ("powerpc/tm: Fix endianness flip on trap")

Applied to bionic/master-next, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team