Re: [PATCH] powerpc/tm: Avoid possible userspace r1 corruption on reclaim

[Michael Ellerman <mpe@ellerman.id.au>]

Michael Neuling <mikey@neuling.org> writes:
> Current we store the userspace r1 to PACATMSCRATCH before finally
> saving it to the thread struct.
>
> In theory an exception could be taken here (like a machine check or
> SLB miss) that could write PACATMSCRATCH and hence corrupt the
> userspace r1. The SLB fault currently doesn't touch PACATMSCRATCH, but
> others do.
>
> We've never actually seen this happen but it's theoretically
> possible. Either way, the code is fragile as it is.
>
> This patch saves r1 to the kernel stack (which can't fault) before we
> turn MSR[RI] back on. PACATMSCRATCH is still used but only with
> MSR[RI] off. We then copy r1 from the kernel stack to the thread
> struct once we have MSR[RI] back on.
>
> Suggested-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/kernel/tm.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
> index 701b0f5b09..8207816a1e 100644
> --- a/arch/powerpc/kernel/tm.S
> +++ b/arch/powerpc/kernel/tm.S
> @@ -178,6 +178,12 @@ _GLOBAL(tm_reclaim)
>  
>  	std	r11, GPR11(r1)			/* Temporary stash */
>  
> +	/* Move r1 to kernel stack in case PACATMSCRATCH is used once
> +	 * we turn on RI
> +	 */

I see we still need to send you to Comment Formatting Re-Education Camp.

I rewrote it a bit too, to hopefully be clearer?

	/*
	 * Move the saved user r1 to the kernel stack in case PACATMSCRATCH is
	 * clobbered by an exception once we turn on MSR_RI below.
	 */
	ld	r11, PACATMSCRATCH(r13)
	std	r11, GPR1(r1)


cheers