NACK: [SRU][Artful][PATCH 1/1] powerpc/tm: Fix endianness flip on trap

[Stefan Bader]

On 27.04.2018 19:20, Joseph Salisbury wrote:
> From: Gustavo Romero <gromero at 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 at linux.vnet.ibm.com>
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> (back ported from commit 1c200e63d055ec0125e44a5e386b9b78aada7eb3)
> Signed-off-by: Joseph Salisbury <joseph.salisbury at 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
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180523/6630351e/attachment.sig>