On Fri, 2017-10-06 at 18:46 +1100, Cyril Bur wrote:
> From: Michael Neuling <mi...@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 <mi...@neuling.org>
> Signed-off-by: Cyril Bur <cyril...@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,
