1. 12 Mar, 2022 1 commit
    • Arnd Bergmann's avatar
      ARM: fix building NOMMU ARMv4/v5 kernels · 234a0f20
      Arnd Bergmann authored
      The removal of the old-style irq entry broke obscure NOMMU
      configurations on machines that have an MMU:
      
      ld.lld: error: undefined symbol: generic_handle_arch_irq
       referenced by kernel/entry-armv.o:(__irq_svc) in archive arch/arm/built-in.a
      
      A follow-up patch to convert nvic to the generic_handle_arch_irq()
      could have fixed this by removing the Kconfig conditional, but did
      it differently.
      
      Change the Kconfig logic so ARM machines now unconditionally
      enable the feature.
      
      I have also submitted a patch to remove support for the configurations
      that broke, but fixing the regression first is a trivial and correct
      change.
      Reported-by: default avatarkernel test robot <lkp@intel.com>
      Fixes: 54f481a2 ("ARM: remove old-style irq entry")
      Fixes: 52d24087 ("irqchip: nvic: Use GENERIC_IRQ_MULTI_HANDLER")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      234a0f20
  2. 11 Mar, 2022 4 commits
    • 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
    • Ard Biesheuvel's avatar
      ARM: Revert "unwind: dump exception stack from calling frame" · bee4e1fd
      Ard Biesheuvel authored
      After simplifying the stack switch code in the IRQ exception handler by
      deferring the actual stack switch to call_with_stack(), we no longer
      need to special case the way we dump the exception stack, since it will
      always be at the top of whichever stack was active when the exception
      was taken.
      
      So revert this special handling for the ARM unwinder.
      
      This reverts commit 4ab68270.
      Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      bee4e1fd
    • Ard Biesheuvel's avatar
      ARM: entry: fix unwinder problems caused by IRQ stacks · 7a8ca84a
      Ard Biesheuvel authored
      The IRQ stacks series made some changes to the unwinder, to permit
      unwinding across different stacks. This is needed because otherwise, the
      call stack would terminate at the point where the stack switch between
      the task stack and the IRQ stack occurs, which would defeat any
      diagnostics that rely on timer interrupts, such as RCU stall detection.
      
      Unfortunately, getting the unwind annotations correct turns out to be
      difficult, given that this now involves a frame pointer which needs to
      point into the right location in the task stack when unwinding from the
      IRQ stack. Getting this wrong for an exception handling routine results
      in the stack pointer to be unwound from the wrong location, causing any
      subsequent unwind attempts to cause all kinds of issues, as reported by
      Naresh here [0].
      
      So let's simplify this, by deferring the stack switch to
      call_with_stack(), which already has the correct unwind annotations, and
      removing all the complicated handling of the stack frame from the IRQ
      exception entrypoint itself.
      
      [0] https://lore.kernel.org/all/CA+G9fYtpy8VgK+ag6OsA9TDrwi5YGU4hu7GM8xwpO7v6LrCD4Q@mail.gmail.com/Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      7a8ca84a
    • Russell King (Oracle)'s avatar
      ARM: unwind: set frame.pc correctly for current-thread unwinding · c46c2c9b
      Russell King (Oracle) authored
      When e.g. a WARN_ON() is encountered, we attempt to unwind the current
      thread. To do this, we set frame.pc to unwind_backtrace, which means it
      points at the beginning of the function. However, the rest of the state
      is initialised from within the function, which means the function
      prologue has already been run.
      
      This can be confusing, and with a recent patch from Ard, can result in
      the unwinder misbehaving if we want to be strict about the PC value.
      
      If we correctly initialise the state so it is self-consistent (in other
      words, set frame.pc to the location we are initialising it) then we
      eliminate this confusion, and avoid possible future issues.
      Reviewed-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      c46c2c9b
  3. 07 Mar, 2022 2 commits
    • Ard Biesheuvel's avatar
      ARM: 9184/1: return_address: disable again for CONFIG_ARM_UNWIND=y · 6845d64d
      Ard Biesheuvel authored
      Commit 41918ec8 ("ARM: ftrace: enable the graph tracer with the EABI
      unwinder") removed the dummy version of return_address() that was
      provided for the CONFIG_ARM_UNWIND=y case, on the assumption that the
      removal of the kernel_text_address() call from unwind_frame() in the
      preceding patch made it safe to do so.
      
      However, this turns out not to be the case: Corentin reports warnings
      about suspicious RCU usage and other strange behavior that seems to
      originate in the stack unwinding that occurs in return_address().
      
      Given that the function graph tracer (which is what these changes were
      enabling for CONFIG_ARM_UNWIND=y builds) does not appear to care about
      this distinction, let's revert return_address() to the old state.
      
      Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
      Fixes: 41918ec8 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
      Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Reported-by: default avatarCorentin Labbe <clabbe.montjoie@gmail.com>
      Tested-by: default avatarCorentin Labbe <clabbe.montjoie@gmail.com>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      6845d64d
    • Ard Biesheuvel's avatar
      ARM: 9183/1: unwind: avoid spurious warnings on bogus code addresses · 81679376
      Ard Biesheuvel authored
      Corentin reports that since commit 538b9265 ("ARM: unwind: track
      location of LR value in stack frame"), numerous spurious warnings are
      emitted into the kernel log:
      
        [    0.000000] unwind: Index not found c0f0c440
        [    0.000000] unwind: Index not found 00000000
        [    0.000000] unwind: Index not found c0f0c440
        [    0.000000] unwind: Index not found 00000000
      
      This is due to the fact that the commit in question removes a check
      whether the PC value in the unwound frame is actually a kernel text
      address, on the assumption that such an address will not be associated
      with valid unwind data to begin with, which is checked right after.
      
      The reason for removing this check was that unwind_frame() will be
      called by the ftrace graph tracer code, which means that it can no
      longer be safely instrumented itself, or any code that it calls, as it
      could cause infinite recursion.
      
      In order to prevent the spurious diagnostics, let's add back the call to
      kernel_text_address(), but this time, only call it if no unwind data
      could be found for the address in question. This is more efficient for
      the common successful case, and should avoid any unintended recursion,
      considering that kernel_text_address() will only be called if no unwind
      data was found.
      
      Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
      Fixes: 538b9265 ("ARM: unwind: track location of LR value in stack frame")
      Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      81679376
  4. 28 Feb, 2022 1 commit
  5. 10 Feb, 2022 2 commits
  6. 09 Feb, 2022 10 commits
  7. 31 Jan, 2022 2 commits
  8. 25 Jan, 2022 5 commits
  9. 24 Jan, 2022 2 commits
  10. 06 Jan, 2022 1 commit
    • Ard Biesheuvel's avatar
      ARM: 9176/1: avoid literal references in inline assembly · 5fe41793
      Ard Biesheuvel authored
      Nathan reports that the new get_current() and per-CPU offset accessors
      may cause problems at build time due to the use of a literal to hold the
      address of the respective variables. This is due to the fact that LLD
      before v14 does not support the PC-relative group relocations that are
      normally used for this, and the fallback relies on literals but does not
      emit the literal pools explictly using the .ltorg directive.
      
      ./arch/arm/include/asm/current.h:53:6: error: out of range pc-relative fixup value
              asm(LOAD_SYM_ARMV6(%0, __current) : "=r"(cur));
                  ^
      ./arch/arm/include/asm/insn.h:25:2: note: expanded from macro 'LOAD_SYM_ARMV6'
              "       ldr     " #reg ", =" #sym "                     nt"
              ^
      <inline asm>:1:3: note: instantiated into assembly here
                      ldr     r0, =__current
                      ^
      
      Since emitting a literal pool in this particular case is not possible,
      let's avoid the LOAD_SYM_ARMV6() entirely, and use the ordinary C
      assigment instead.
      
      As it turns out, there are other such cases, and here, using .ltorg to
      emit the literal pool within range of the LDR instruction would be
      possible due to the presence of an unconditional branch right after it.
      Unfortunately, putting .ltorg directives in subsections appears to
      confuse the Clang inline assembler, resulting in similar errors even
      though the .ltorg is most definitely within range.
      
      So let's fix this by emitting the literal explicitly, and not rely on
      the assembler to figure this out. This means we have move the fallback
      out of the LOAD_SYM_ARMV6() macro and into the callers.
      
      Link: https://github.com/ClangBuiltLinux/linux/issues/1551
      
      Fixes: 9c46929e ("ARM: implement THREAD_INFO_IN_TASK for uniprocessor systems")
      Reported-by: default avatarNathan Chancellor <natechancellor@gmail.com>
      Tested-by: default avatarNathan Chancellor <nathan@kernel.org>
      Signed-off-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      5fe41793
  11. 05 Jan, 2022 1 commit
  12. 17 Dec, 2021 1 commit
  13. 06 Dec, 2021 8 commits