[PATCH 1/5] powerpc/tm: Fix writing top half of MSR on 32 bit signals

[PATCH 1/5] powerpc/tm: Fix writing top half of MSR on 32 bit signals

[Michael Neuling]

The MSR TM controls are in the top 32 bits of the MSR hence on 32 bit signals,
we stick the top half of the MSR in the checkpointed signal context so that the
user can access it.

Unfortunately, we don't currently write anything to the checkpointed signal
context when coming in a from a non transactional process and hence the top MSR
bits can contain junk.

This updates the 32 bit signal handling code to always write something to the
top MSR bits so that users know if the process is transactional or not and the
kernel can use it on signal return.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/signal_32.c |   29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 201385c..5bc819f 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -407,7 +407,8 @@ inline unsigned long copy_transact_fpr_from_user(struct task_struct *task,
  * altivec/spe instructions at some point.
  */
 static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
-		int sigret, int ctx_has_vsx_region)
+			  struct mcontext __user *tm_frame, int sigret,
+			  int ctx_has_vsx_region)
 {
 	unsigned long msr = regs->msr;
 
@@ -475,6 +476,12 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
 
 	if (__put_user(msr, &frame->mc_gregs[PT_MSR]))
 		return 1;
+	/* We need to write 0 the MSR top 32 bits in the tm frame so that we
+	 * can check it on the restore to see if TM is active
+	 */
+	if (tm_frame && __put_user(0, &tm_frame->mc_gregs[PT_MSR]))
+		return 1;
+
 	if (sigret) {
 		/* Set up the sigreturn trampoline: li r0,sigret; sc */
 		if (__put_user(0x38000000UL + sigret, &frame->tramp[0])
@@ -952,6 +959,7 @@ int handle_rt_signal32(unsigned long sig, struct k_sigaction *ka,
 {
 	struct rt_sigframe __user *rt_sf;
 	struct mcontext __user *frame;
+	struct mcontext __user *tm_frame = NULL;
 	void __user *addr;
 	unsigned long newsp = 0;
 	int sigret;
@@ -985,23 +993,24 @@ int handle_rt_signal32(unsigned long sig, struct k_sigaction *ka,
 	}
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	tm_frame = &rt_sf->uc_transact.uc_mcontext;
 	if (MSR_TM_ACTIVE(regs->msr)) {
-		if (save_tm_user_regs(regs, &rt_sf->uc.uc_mcontext,
-				      &rt_sf->uc_transact.uc_mcontext, sigret))
+		if (save_tm_user_regs(regs, frame, tm_frame, sigret))
 			goto badframe;
 	}
 	else
 #endif
-		if (save_user_regs(regs, frame, sigret, 1))
+	{
+		if (save_user_regs(regs, frame, tm_frame, sigret, 1))
 			goto badframe;
+	}
 	regs->link = tramp;
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	if (MSR_TM_ACTIVE(regs->msr)) {
 		if (__put_user((unsigned long)&rt_sf->uc_transact,
 			       &rt_sf->uc.uc_link)
-		    || __put_user(to_user_ptr(&rt_sf->uc_transact.uc_mcontext),
-				  &rt_sf->uc_transact.uc_regs))
+		    || __put_user((unsigned long)tm_frame, &rt_sf->uc_transact.uc_regs))
 			goto badframe;
 	}
 	else
@@ -1170,7 +1179,7 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
 		mctx = (struct mcontext __user *)
 			((unsigned long) &old_ctx->uc_mcontext & ~0xfUL);
 		if (!access_ok(VERIFY_WRITE, old_ctx, ctx_size)
-		    || save_user_regs(regs, mctx, 0, ctx_has_vsx_region)
+		    || save_user_regs(regs, mctx, NULL, 0, ctx_has_vsx_region)
 		    || put_sigset_t(&old_ctx->uc_sigmask, &current->blocked)
 		    || __put_user(to_user_ptr(mctx), &old_ctx->uc_regs))
 			return -EFAULT;
@@ -1392,6 +1401,7 @@ int handle_signal32(unsigned long sig, struct k_sigaction *ka,
 {
 	struct sigcontext __user *sc;
 	struct sigframe __user *frame;
+	struct mcontext __user *tm_mctx = NULL;
 	unsigned long newsp = 0;
 	int sigret;
 	unsigned long tramp;
@@ -1425,6 +1435,7 @@ int handle_signal32(unsigned long sig, struct k_sigaction *ka,
 	}
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	tm_mctx = &frame->mctx_transact;
 	if (MSR_TM_ACTIVE(regs->msr)) {
 		if (save_tm_user_regs(regs, &frame->mctx, &frame->mctx_transact,
 				      sigret))
@@ -1432,8 +1443,10 @@ int handle_signal32(unsigned long sig, struct k_sigaction *ka,
 	}
 	else
 #endif
-		if (save_user_regs(regs, &frame->mctx, sigret, 1))
+	{
+		if (save_user_regs(regs, &frame->mctx, tm_mctx, sigret, 1))
 			goto badframe;
+	}
 
 	regs->link = tramp;
 
-- 
1.7.10.4

[PATCH 2/5] powerpc/tm: Fix 32 bit non-rt signals

[Michael Neuling]

Currently sys_sigreturn() is TM unaware.  Therefore, if we take a 32 bit signal
without SIGINFO (non RT) inside a transaction, on signal return we don't
restore the signal frame correctly.

This checks if the signal frame being restoring is an active transaction, and
if so, it copies the additional state to ptregs so it can be restored.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/signal_32.c |   28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 5bc819f..5b0fbe2 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1494,16 +1494,22 @@ badframe:
 long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
 		       struct pt_regs *regs)
 {
+	struct sigframe __user *sf;
 	struct sigcontext __user *sc;
 	struct sigcontext sigctx;
 	struct mcontext __user *sr;
 	void __user *addr;
 	sigset_t set;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	struct mcontext __user *mcp, *tm_mcp;
+	unsigned long msr_hi;
+#endif
 
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
 
-	sc = (struct sigcontext __user *)(regs->gpr[1] + __SIGNAL_FRAMESIZE);
+	sf = (struct sigframe __user *)(regs->gpr[1] + __SIGNAL_FRAMESIZE);
+	sc = &sf->sctx;
 	addr = sc;
 	if (copy_from_user(&sigctx, sc, sizeof(sigctx)))
 		goto badframe;
@@ -1520,11 +1526,23 @@ long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
 #endif
 	set_current_blocked(&set);
 
-	sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
-	addr = sr;
-	if (!access_ok(VERIFY_READ, sr, sizeof(*sr))
-	    || restore_user_regs(regs, sr, 1))
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	mcp = (struct mcontext __user *)&sf->mctx;
+	tm_mcp = (struct mcontext __user *)&sf->mctx_transact;
+	if (__get_user(msr_hi, &tm_mcp->mc_gregs[PT_MSR]))
 		goto badframe;
+	if MSR_TM_ACTIVE(msr_hi<<32) {
+		if (restore_tm_user_regs(regs, mcp, tm_mcp))
+			goto badframe;
+	} else
+#endif
+	{
+		sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
+		addr = sr;
+		if (!access_ok(VERIFY_READ, sr, sizeof(*sr))
+		    || restore_user_regs(regs, sr, 1))
+			goto badframe;
+	}
 
 	set_thread_flag(TIF_RESTOREALL);
 	return 0;
-- 
1.7.10.4

[PATCH 3/5] powerpc/tm: Fix restoration of MSR on 32bit signal return

[Michael Neuling]

Currently we clear out the MSR TM bits on signal return assuming that the
signal should never return to an active transaction.

This is bogus as the user may do this.  It's most likely the transaction will
be doomed due to a treclaim but that's a problem for the HW not the kernel.

This removes the stripping of these MSR TM bits.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/signal_32.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 5b0fbe2..b8279b3 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -755,6 +755,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 {
 	long err;
 	unsigned long msr;
+	__u32 msr_hi;
 #ifdef CONFIG_VSX
 	int i;
 #endif
@@ -859,8 +860,10 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 	tm_enable();
 	/* This loads the checkpointed FP/VEC state, if used */
 	tm_recheckpoint(&current->thread, msr);
-	/* The task has moved into TM state S, so ensure MSR reflects this */
-	regs->msr = (regs->msr & ~MSR_TS_MASK) | MSR_TS_S;
+	/* Retore the top half of the MSR */
+	if (__get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR]))
+		return 1;
+	regs->msr = (regs->msr | (((unsigned long)msr_hi) << 32));
 
 	/* This loads the speculative FP/VEC state, if used */
 	if (msr & MSR_FP) {
-- 
1.7.10.4

[PATCH 4/5] powerpc/tm: Fix return of 32bit rt signals to active transactions

[Michael Neuling]

Currently we only restore signals which are transactionally suspended but it's
possible that the transaction can be restored even when it's active.  Most
likely this will result in a transactional rollback by the hardware as the
transaction will have been doomed by an earlier treclaim.

The current code is a legacy of earlier kernel implementations which did
software rollback of active transactions in the kernel.  That code has now gone
but we didn't correctly fix up this part of the signals code which still makes
assumptions based on having software rollback.

This changes the signal return code to always restore both contexts on 32 bit
rt signal return.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/signal_32.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index b8279b3..9e331eb 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1245,7 +1245,7 @@ long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
 		if (__get_user(msr_hi, &mcp->mc_gregs[PT_MSR]))
 			goto bad;
 
-		if (MSR_TM_SUSPENDED(msr_hi<<32)) {
+		if (MSR_TM_ACTIVE(msr_hi<<32)) {
 			/* We only recheckpoint on return if we're
 			 * transaction.
 			 */
-- 
1.7.10.4

[PATCH 5/5] powerpc/tm: Fix return of active 64bit signals

[Michael Neuling]

Currently we only restore signals which are transactionally suspended but it's
possible that the transaction can be restored even when it's active.  Most
likely this will result in a transactional rollback by the hardware as the
transaction will have been doomed by an earlier treclaim.

The current code is a legacy of earlier kernel implementations which did
software rollback of active transactions in the kernel.  That code has now gone
but we didn't correctly fix up this part of the signals code which still makes
assumptions based on having software rollback.

This changes the signal return code to always restore both contexts on 64 bit
signal return.  It also ensures that the MSR TM bits are properly restored from
the signal context which they are not currently.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/signal_64.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 3459473..887e99d 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -410,6 +410,10 @@ static long restore_tm_sigcontexts(struct pt_regs *regs,
 
 	/* get MSR separately, transfer the LE bit if doing signal return */
 	err |= __get_user(msr, &sc->gp_regs[PT_MSR]);
+	/* pull in MSR TM from user context */
+	regs->msr = (regs->msr & ~MSR_TS_MASK) | (msr & MSR_TS_MASK);
+
+	/* pull in MSR LE from user context */
 	regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
 
 	/* The following non-GPR non-FPR non-VR state is also checkpointed: */
@@ -505,8 +509,6 @@ static long restore_tm_sigcontexts(struct pt_regs *regs,
 	tm_enable();
 	/* This loads the checkpointed FP/VEC state, if used */
 	tm_recheckpoint(&current->thread, msr);
-	/* The task has moved into TM state S, so ensure MSR reflects this: */
-	regs->msr = (regs->msr & ~MSR_TS_MASK) | __MASK(33);
 
 	/* This loads the speculative FP/VEC state, if used */
 	if (msr & MSR_FP) {
@@ -654,7 +656,7 @@ int sys_rt_sigreturn(unsigned long r3, unsigned long r4, unsigned long r5,
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
 		goto badframe;
-	if (MSR_TM_SUSPENDED(msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		/* We recheckpoint on return. */
 		struct ucontext __user *uc_transact;
 		if (__get_user(uc_transact, &uc->uc_link))
-- 
1.7.10.4

Re: [PATCH 2/5] powerpc/tm: Fix 32 bit non-rt signals

[Benjamin Herrenschmidt]

On Fri, 2013-06-07 at 20:36 +1000, Michael Neuling wrote:
> Currently sys_sigreturn() is TM unaware.  Therefore, if we take a 32 bit signal
> without SIGINFO (non RT) inside a transaction, on signal return we don't
> restore the signal frame correctly.
> 
> This checks if the signal frame being restoring is an active transaction, and
> if so, it copies the additional state to ptregs so it can be restored.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---

 .../...

> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	mcp = (struct mcontext __user *)&sf->mctx;
> +	tm_mcp = (struct mcontext __user *)&sf->mctx_transact;
> +	if (__get_user(msr_hi, &tm_mcp->mc_gregs[PT_MSR]))
>  		goto badframe;
> +	if MSR_TM_ACTIVE(msr_hi<<32) {

Mising ( and ). I'll apply that fix locally.

Appart from that, I suppose it's ok. I don't see any exposure
coming from users "cooking" the tm_frame and calling sigreturn,
so as long as we are confident userspace generally only uses
sigreturn with frames it got from an actual signal, and doesn't
try to "generate" frames by hand, we should be ok.

Cheers,
Ben.

Re: [PATCH 3/5] powerpc/tm: Fix restoration of MSR on 32bit signal return

[Benjamin Herrenschmidt]

On Fri, 2013-06-07 at 20:36 +1000, Michael Neuling wrote:
> Currently we clear out the MSR TM bits on signal return assuming that the
> signal should never return to an active transaction.
> 
> This is bogus as the user may do this.  It's most likely the transaction will
> be doomed due to a treclaim but that's a problem for the HW not the kernel.
> 
> This removes the stripping of these MSR TM bits.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---

> @@ -859,8 +860,10 @@ static long restore_tm_user_regs(struct pt_regs *regs,
>  	tm_enable();
>  	/* This loads the checkpointed FP/VEC state, if used */
>  	tm_recheckpoint(&current->thread, msr);
> -	/* The task has moved into TM state S, so ensure MSR reflects this */
> -	regs->msr = (regs->msr & ~MSR_TS_MASK) | MSR_TS_S;
> +	/* Retore the top half of the MSR */
> +	if (__get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR]))
> +		return 1;
> +	regs->msr = (regs->msr | (((unsigned long)msr_hi) << 32));

What kind of damage can I do by calling sigreturn with a cooked
frame with random MSR bits set ? You should probably filter
what bits you allow to come from the frame.

Additionally, I would also make sure I only do that if the CPU
features say TM is supported in case that MSR bit means something else
on a different/older CPU...

Cheers,
Ben.

Re: [PATCH 3/5] powerpc/tm: Fix restoration of MSR on 32bit signal return

[Michael Neuling]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2013-06-07 at 20:36 +1000, Michael Neuling wrote:
> > Currently we clear out the MSR TM bits on signal return assuming that the
> > signal should never return to an active transaction.
> > 
> > This is bogus as the user may do this.  It's most likely the transaction will
> > be doomed due to a treclaim but that's a problem for the HW not the kernel.
> > 
> > This removes the stripping of these MSR TM bits.
> > 
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> 
> > @@ -859,8 +860,10 @@ static long restore_tm_user_regs(struct pt_regs *regs,
> >  	tm_enable();
> >  	/* This loads the checkpointed FP/VEC state, if used */
> >  	tm_recheckpoint(&current->thread, msr);
> > -	/* The task has moved into TM state S, so ensure MSR reflects this */
> > -	regs->msr = (regs->msr & ~MSR_TS_MASK) | MSR_TS_S;
> > +	/* Retore the top half of the MSR */
> > +	if (__get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR]))
> > +		return 1;
> > +	regs->msr = (regs->msr | (((unsigned long)msr_hi) << 32));
> 
> What kind of damage can I do by calling sigreturn with a cooked
> frame with random MSR bits set ? You should probably filter
> what bits you allow to come from the frame.

Lots of damage.. good, point.  

We do that in the 64 bit version of this.  I'll update to do the same.

> Additionally, I would also make sure I only do that if the CPU
> features say TM is supported in case that MSR bit means something else
> on a different/older CPU...

Ok, I'll add that also.

Mikey

Re: [PATCH 2/5] powerpc/tm: Fix 32 bit non-rt signals

[Michael Neuling]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2013-06-07 at 20:36 +1000, Michael Neuling wrote:
> > Currently sys_sigreturn() is TM unaware.  Therefore, if we take a 32 bit signal
> > without SIGINFO (non RT) inside a transaction, on signal return we don't
> > restore the signal frame correctly.
> > 
> > This checks if the signal frame being restoring is an active transaction, and
> > if so, it copies the additional state to ptregs so it can be restored.
> > 
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> 
>  .../...
> 
> > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > +	mcp = (struct mcontext __user *)&sf->mctx;
> > +	tm_mcp = (struct mcontext __user *)&sf->mctx_transact;
> > +	if (__get_user(msr_hi, &tm_mcp->mc_gregs[PT_MSR]))
> >  		goto badframe;
> > +	if MSR_TM_ACTIVE(msr_hi<<32) {
> 
> Mising ( and ). I'll apply that fix locally.
> 
> Appart from that, I suppose it's ok. I don't see any exposure
> coming from users "cooking" the tm_frame and calling sigreturn,
> so as long as we are confident userspace generally only uses
> sigreturn with frames it got from an actual signal, and doesn't
> try to "generate" frames by hand, we should be ok.

We should add a has_cpu_feature(TM) here also in case someone cooks up
an sig frame with MSR TM active, but on a non TM CPU.  This could possibly
result in a trecheckpoint on a non TM CPU hence an illegal in the
kernel.  

I'll repost.

Thanks,
Mikey