lkml.org 
[lkml]   [2021]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 56/95] KVM: PPC: Book3S HV: Fix stack handling in idle_kvm_start_guest()
    Date
    From: Michael Ellerman <mpe@ellerman.id.au>

    commit 9b4416c5095c20e110c82ae602c254099b83b72f upstream.

    In commit 10d91611f426 ("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: 10d91611f426 ("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 | 19 ++++++++++---------
    1 file changed, 10 insertions(+), 9 deletions(-)

    --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
    +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
    @@ -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

    \
     
     \ /
      Last update: 2021-10-25 21:43    [W:2.618 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site