• Ard Biesheuvel's avatar
    ARM: unwind: only permit stack switch when unwinding call_with_stack() · f6b8e352
    Ard Biesheuvel authored
    Commit b6506981 ("ARM: unwind: support unwinding across multiple
    stacks") updated the logic in the ARM unwinder to widen the bounds
    within which SP is assumed to be valid, in order to allow the unwind to
    traverse from the IRQ stack to the task stack. This is necessary, as
    otherwise, unwinds started from the IRQ stack would terminate in the IRQ
    exception handler, making stacktraces substantially less useful.
    
    This turns out to be a mistake, as it breaks asynchronous unwinding
    across exceptions, when the exception is taken before the stack frame is
    consistent with the unwind info. For instance, in the following
    backtrace:
    
      ...
       generic_handle_arch_irq from call_with_stack+0x18/0x20
       call_with_stack from __irq_svc+0x80/0x98
      Exception stack(0xc7093e20 to 0xc7093e68)
      3e20: b6a94a88 c7093ea0 00000008 00000000 c7093ea0 b7e127d0 00000051 c9220000
      3e40: b6a94a88 b6a94a88 00000004 0002b000 0036b570 c7093e70 c040ca2c c0994a90
      3e60: 20070013 ffffffff
       __irq_svc from __copy_to_user_std+0x20/0x378
      ...
    
    we need to apply the following unwind directives:
    
      0xc099720c <__copy_to_user_std+0x1c>: @0xc295d1d4
        Compact model index: 1
        0x9b      vsp = r11
        0xb1 0x0d pop {r0, r2, r3}
        0x84 0x81 pop {r4, r11, r14}
        0xb0      finish
    
    which tell us to switch to the frame pointer register R11 and proceed
    with the unwind from that. However, having been interrupted 0x20 bytes
    into the function:
    
      c09971f0 <__copy_to_user_std>:
      c09971f0:       e59f3350        ldr     r3, [pc, #848]
      c09971f4:       e243c001        sub     ip, r3, #1
      c09971f8:       e05cc000        subs    ip, ip, r0
      c09971fc:       228cc001        addcs   ip, ip, #1
      c0997200:       205cc002        subscs  ip, ip, r2
      c0997204:       33a00000        movcc   r0, #0
      c0997208:       e320f014        csdb
      c099720c:       e3a03000        mov     r3, #0
      c0997210:       e92d481d        push    {r0, r2, r3, r4, fp, lr}
      c0997214:       e1a0b00d        mov     fp, sp
      c0997218:       e2522004        subs    r2, r2, #4
    
    the value for R11 recovered from the previous frame (__irq_svc) will be
    a snapshot of its value before the exception was taken (0x0002b000),
    which occurred at address __copy_to_user_std+0x20 (0xc0997210), when R11
    had not been assigned its value yet.
    
    This means we can never assume that the SP values recovered from the
    stack or from the frame pointer are ever safe to use, given the need to
    do asynchronous unwinding, and the only robust approach is to revert to
    the previous approach, which is to derive bounds for SP based on the
    initial value, and never update them.
    
    We can make an exception, though: now that the IRQ stack switch is
    guaranteed to occur in call_with_stack(), we can implement a special
    case for this function, and use a different set of bounds based on the
    knowledge that it will always unwind from R11 rather than SP. As
    call_with_stack() is a hand-rolled assembly routine, this is guaranteed
    to remain that way.
    
    So let's do a partial revert of b6506981, and drop all manipulations
    for sp_low and sp_high based on the information collected during the
    unwind itself. To support call_with_stack(), set sp_low and sp_high
    explicitly to values derived from R11 when we unwind that function.
    
    The only downside is that, while unwinding an overflow of the vmap'ed
    stack will work fine as before, we will no longer be able to produce a
    backtrace that unwinds the overflow stack itself across the exception
    that was raised due to the faulting access to the guard region. However,
    this only affects exceptions caused by problems in the stack overflow
    handling code itself, in which case the remaining backtrace is not that
    relevant.
    
    Fixes: b6506981 ("ARM: unwind: support unwinding across multiple stacks")
    Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
    Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
    f6b8e352
unwind.c 14.3 KB