[3/3] powerpc/tm: P9 disable transactionally suspended sigcontexts

From: Michael Neuling <mikey@neuling.org>

Unfortunately userspace can construct a sigcontext which enables
suspend. Thus userspace can force Linux into a path where trechkpt is
executed.

This patch blocks this from happening on POWER9 but sanity checking
sigcontexts passed in.

ptrace doesn't have this problem as only MSR SE and BE can be changed
via ptrace.

This patch also adds a number of WARN_ON() in case we every enter
suspend when we shouldn't. This should catch systems that don't have
the firmware change and are running TM.

A future firmware change will allow suspend mode on POWER9 but that is
going to require additional Linux changes to support. In the interim,
this allows TM to continue to (partially) work while stopping
userspace from crashing Linux.

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/powerpc/kernel/process.c   | 2 ++
 arch/powerpc/kernel/signal_32.c | 4 ++++
 arch/powerpc/kernel/signal_64.c | 5 +++++
 3 files changed, 11 insertions(+)

On Fri, 2017-10-06 at 18:46 +1100, Cyril Bur wrote:
> From: Michael Neuling <mikey@neuling.org>
> 
> Unfortunately userspace can construct a sigcontext which enables
> suspend. Thus userspace can force Linux into a path where trechkpt is
> executed.
> 
> This patch blocks this from happening on POWER9 but sanity checking
> sigcontexts passed in.
> 
> ptrace doesn't have this problem as only MSR SE and BE can be changed
> via ptrace.
> 
> This patch also adds a number of WARN_ON() in case we every enter
> suspend when we shouldn't. This should catch systems that don't have
> the firmware change and are running TM.
> 
> A future firmware change will allow suspend mode on POWER9 but that is
> going to require additional Linux changes to support. In the interim,
> this allows TM to continue to (partially) work while stopping
> userspace from crashing Linux.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> ---
>  arch/powerpc/kernel/process.c   | 2 ++
>  arch/powerpc/kernel/signal_32.c | 4 ++++
>  arch/powerpc/kernel/signal_64.c | 5 +++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index a0c74bbf3454..5b81673c5026 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -903,6 +903,8 @@ static inline void tm_reclaim_task(struct task_struct *tsk)
>  	if (!MSR_TM_ACTIVE(thr->regs->msr))
>  		goto out_and_saveregs;
>  
> +	WARN_ON(!tm_suspend_supported());
> +

What does this function really says ? That TM is supported or that TM
supports suspend ? Because the implementation in the previous patch
seems to indicate that what it actually indicates is that TM is
supported, period.

>  	TM_DEBUG("--- tm_reclaim on pid %d (NIP=%lx, "
>  		 "ccr=%lx, msr=%lx, trap=%lx)\n",
>  		 tsk->pid, thr->regs->nip,
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 92fb1c8dbbd8..9eac0131c080 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -519,6 +519,8 @@ static int save_tm_user_regs(struct pt_regs *regs,
>  {
>  	unsigned long msr = regs->msr;
>  
> +	WARN_ON(!tm_suspend_supported());
> +
>  	/* 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
> @@ -769,6 +771,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,
>  	int i;
>  #endif
>  
> +	if (!tm_suspend_supported())
> +		return 1;
>  	/*
>  	 * restore general registers but not including MSR or SOFTE. Also
>  	 * take care of keeping r2 (TLS) intact if not a signal.
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index c83c115858c1..6d28caf8496f 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -214,6 +214,8 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
>  
>  	BUG_ON(!MSR_TM_ACTIVE(regs->msr));
>  
> +	WARN_ON(!tm_suspend_supported());
> +
>  	/* 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
> @@ -430,6 +432,9 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
>  
>  	BUG_ON(tsk != current);
>  
> +	if (!tm_suspend_supported())
> +		return -EINVAL;
> +
>  	/* copy the GPRs */
>  	err |= __copy_from_user(regs->gpr, tm_sc->gp_regs, sizeof(regs->gpr));
>  	err |= __copy_from_user(&tsk->thread.ckpt_regs, sc->gp_regs,

Hi Cyril,

On 06-10-2017 04:46, Cyril Bur wrote:
> From: Michael Neuling <mikey@neuling.org>
> 
> Unfortunately userspace can construct a sigcontext which enables
> suspend. Thus userspace can force Linux into a path where trechkpt is
> executed.
> 
> This patch blocks this from happening on POWER9 but sanity checking
> sigcontexts passed in.

I think "but" should say "by" as pointed out by Joel and acked by Mikey
previously.

Regards,
Gustavo