]> git.baikalelectronics.ru Git - kernel.git/commitdiff
KVM: PPC: Book3S HV: Fix stack handling in idle_kvm_start_guest()
authorMichael Ellerman <mpe@ellerman.id.au>
Fri, 15 Oct 2021 12:01:48 +0000 (23:01 +1100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 27 Oct 2021 07:54:28 +0000 (09:54 +0200)
commit 9b4416c5095c20e110c82ae602c254099b83b72f upstream.

In commit 7c6829b24785 ("powerpc/64s: Reimplement book3s idle code in
C") kvm_start_guest() became idle_kvm_start_guest(). The old code
allocated a stack frame on the emergency stack, but didn't use the
frame to store anything, and also didn't store anything in its caller's
frame.

idle_kvm_start_guest() on the other hand is written more like a normal C
function, it creates a frame on entry, and also stores CR/LR into its
callers frame (per the ABI). The problem is that there is no caller
frame on the emergency stack.

The emergency stack for a given CPU is allocated with:

  paca_ptrs[i]->emergency_sp = alloc_stack(limit, i) + THREAD_SIZE;

So emergency_sp actually points to the first address above the emergency
stack allocation for a given CPU, we must not store above it without
first decrementing it to create a frame. This is different to the
regular kernel stack, paca->kstack, which is initialised to point at an
initial frame that is ready to use.

idle_kvm_start_guest() stores the backchain, CR and LR all of which
write outside the allocation for the emergency stack. It then creates a
stack frame and saves the non-volatile registers. Unfortunately the
frame it creates is not large enough to fit the non-volatiles, and so
the saving of the non-volatile registers also writes outside the
emergency stack allocation.

The end result is that we corrupt whatever is at 0-24 bytes, and 112-248
bytes above the emergency stack allocation.

In practice this has gone unnoticed because the memory immediately above
the emergency stack happens to be used for other stack allocations,
either another CPUs mc_emergency_sp or an IRQ stack. See the order of
calls to irqstack_early_init() and emergency_stack_init().

The low addresses of another stack are the top of that stack, and so are
only used if that stack is under extreme pressue, which essentially
never happens in practice - and if it did there's a high likelyhood we'd
crash due to that stack overflowing.

Still, we shouldn't be corrupting someone else's stack, and it is purely
luck that we aren't corrupting something else.

To fix it we save CR/LR into the caller's frame using the existing r1 on
entry, we then create a SWITCH_FRAME_SIZE frame (which has space for
pt_regs) on the emergency stack with the backchain pointing to the
existing stack, and then finally we switch to the new frame on the
emergency stack.

Fixes: 7c6829b24785 ("powerpc/64s: Reimplement book3s idle code in C")
Cc: stable@vger.kernel.org # v5.2+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20211015133929.832061-1-mpe@ellerman.id.au
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
arch/powerpc/kvm/book3s_hv_rmhandlers.S

index feaf6ca2e76c1c2e12fe457628d8213c23cd67b7..663d219c292b6f73ce7bd9751f0b1716919bf8f6 100644 (file)
@@ -292,13 +292,15 @@ kvm_novcpu_exit:
  * r3 contains the SRR1 wakeup value, SRR1 is trashed.
  */
 _GLOBAL(idle_kvm_start_guest)
-       ld      r4,PACAEMERGSP(r13)
        mfcr    r5
        mflr    r0
-       std     r1,0(r4)
-       std     r5,8(r4)
-       std     r0,16(r4)
-       subi    r1,r4,STACK_FRAME_OVERHEAD
+       std     r5, 8(r1)       // Save CR in caller's frame
+       std     r0, 16(r1)      // Save LR in caller's frame
+       // Create frame on emergency stack
+       ld      r4, PACAEMERGSP(r13)
+       stdu    r1, -SWITCH_FRAME_SIZE(r4)
+       // Switch to new frame on emergency stack
+       mr      r1, r4
        SAVE_NVGPRS(r1)
 
        /*
@@ -444,10 +446,9 @@ kvm_no_guest:
        /* set up r3 for return */
        mfspr   r3,SPRN_SRR1
        REST_NVGPRS(r1)
-       addi    r1, r1, STACK_FRAME_OVERHEAD
-       ld      r0, 16(r1)
-       ld      r5, 8(r1)
-       ld      r1, 0(r1)
+       ld      r1, 0(r1)       // Switch back to caller stack
+       ld      r0, 16(r1)      // Reload LR
+       ld      r5, 8(r1)       // Reload CR
        mtlr    r0
        mtcr    r5
        blr