On Fri, 2022-03-11 at 12:47 +1000, Nicholas Piggin wrote:
> Commit cf13435b730a ("powerpc/tm: Fix userspace r13 corruption") fixes
> a problem in treclaim where a SLB miss can occur on the
> thread_struct->ckpt_regs while SCRATCH0 is live with the saved user r13
> value, clobbering it with the kernel r13 and ultimately resulting in
> kernel r13 being stored in ckpt_regs.
>
> There is an equivalent problem in trechkpt where the user r13 value is
> loaded into r13 from chkpt_regs to be recheckpointed, but a SLB miss
> could occur on ckpt_regs accesses after that, which will result in r13
> being clobbered with a kernel value and that will get recheckpointed and
> then restored to user registers.
>
> The same memory page is accessed right before this critical window where
> a SLB miss could cause corruption, so hitting the bug requires the SLB
> entry be removed within a small window of instructions, which is possible
> if a SLB related MCE hits there. PAPR also permits the hypervisor to
> discard this SLB entry (because slb_shadow->persistent is only set to
> SLB_NUM_BOLTED) although it's not known whether any implementations would
> do this (KVM does not). So this is an extremely unlikely bug, only found
> by inspection.
>
> Fix this by also storing user r13 in a temporary location on the kernel
> stack and don't chane the r13 register from kernel r13 until the RI=0
> critical section that does not fault.
s/chane/change/
>
> [ The SCRATCH0 change is not strictly part of the fix, it's only used in
> the RI=0 section so it does not have the same problem as the previous
> SCRATCH0 bug. ]
>
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
This needs to be marked for stable also. Other than that:
Acked-by: Michael Neuling <mi...@neuling.org>
Thanks!
> ---
> arch/powerpc/kernel/tm.S | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
> index 3beecc32940b..5a0f023a26e9 100644
> --- a/arch/powerpc/kernel/tm.S
> +++ b/arch/powerpc/kernel/tm.S
> @@ -443,7 +443,8 @@ restore_gprs:
>
> REST_GPR(0, r7) /* GPR0 */
> REST_GPRS(2, 4, r7) /* GPR2-4 */
> - REST_GPRS(8, 31, r7) /* GPR8-31 */
> + REST_GPRS(8, 12, r7) /* GPR8-12 */
> + REST_GPRS(14, 31, r7) /* GPR14-31 */
>
> /* Load up PPR and DSCR here so we don't run with user values for long
> */
> mtspr SPRN_DSCR, r5
> @@ -479,18 +480,24 @@ restore_gprs:
> REST_GPR(6, r7)
>
> /*
> - * Store r1 and r5 on the stack so that we can access them after we
> - * clear MSR RI.
> + * Store user r1 and r5 and r13 on the stack (in the unused save
> + * areas / compiler reserved areas), so that we can access them after
> + * we clear MSR RI.
> */
>
> REST_GPR(5, r7)
> std r5, -8(r1)
> - ld r5, GPR1(r7)
> + ld r5, GPR13(r7)
> std r5, -16(r1)
> + ld r5, GPR1(r7)
> + std r5, -24(r1)
>
> REST_GPR(7, r7)
>
> - /* Clear MSR RI since we are about to use SCRATCH0. EE is already off
> */
> + /* Stash the stack pointer away for use after recheckpoint */
> + std r1, PACAR1(r13)
> +
> + /* Clear MSR RI since we are about to clobber r13. EE is already off
> */
> li r5, 0
> mtmsrd r5, 1
>
> @@ -501,9 +508,9 @@ restore_gprs:
> * until we turn MSR RI back on.
> */
>
> - SET_SCRATCH0(r1)
> ld r5, -8(r1)
> - ld r1, -16(r1)
> + ld r13, -16(r1)
> + ld r1, -24(r1)
>
> /* Commit register state as checkpointed state: */
> TRECHKPT
> @@ -519,9 +526,9 @@ restore_gprs:
> */
>
> GET_PACA(r13)
> - GET_SCRATCH0(r1)
> + ld r1, PACAR1(r13)
>
> - /* R1 is restored, so we are recoverable again. EE is still off */
> + /* R13, R1 is restored, so we are recoverable again. EE is still off
> */
> li r4, MSR_RI
> mtmsrd r4, 1
>
