On Wed, 13 Oct 2021 at 13:01, Mark Rutland [off-list ref] wrote:

We recently realised that out-of-line extable fixups cause a number of problems
for backtracing (mattering both for developers and for RELIABLE_STACKTRACE and
LIVEPATCH). Dmitry spotted a confusing backtrace, which we identified was due
to problems with unwinding fixups, as summarized in:

  https://lore.kernel.org/linux-arm-kernel/20210927171812.GB9201@C02TD0UTHF1T.local/ (local)

The gist is that while backtracing through a fixup, the fixup gets symmbolized
as an offset from the nearest prior symbol (which happens to be
`__entry_tramp_text_end`), and we the backtrace misses the function that was
being fixed up (because the fixup handling adjusts the PC, then the fixup does
a direct branch back to the original function). We can't reliably map from an
arbitrary PC in the fixup text back to the original function.

The way we create fixups is a bit unfortunate: most fixups are generated from
common templates, and only differ in register to be poked and the address to
branch back to, leading to redundant copies of the same logic that must pollute
Since the fixups are all written in assembly, and duplicated for each fixup
site, we can only perform very simple fixups, and can't handle any complex
triage that we might need for some exceptions (e.g. MTE faults).

This series address these concerns by getting rid of the out-of-line anonymous
fixup logic:

* For plain assembly functions, we move the fixup into the body of
  the function, after the usual return, as we already do for our cache
  routines. This simplifies the source code, and only adds a handful of
  instructions to the main body of `.text`.

  This is handled by the first three patches, which I think are trivial and
  could be queued regardless of the rest of the series.

* For inline assembly, we add specialised handlers which run in exception
  context to update registers, then adjust the PC *within* the faulting
  function. This requires some new glue to capture the handler and metadata in
  struct exception_table_entry (costing 32 bits per fixup), but for any
  non-trivial fixup (which is all of the inline asm cases), this removes at
  least two instructions of out-of-line fixup.

As the fixups are now handled from C code in exception context, we can more
easily extend these in future with more complex triage if necessary.

Overall, this doesn't have an appreciable impact on Image size (in local
testing the size of the Image was identical before/after), but does shift the
boundary between .text and .ordata, making .text smaller and .rodata bigger.
.text somewhat while growing .rodata somewhat.

I've tested this with both GCC and clang (including with clang CFI), and
everything is working as expected.

Other than changes to backtracing, there should be no functional change as a
result of this series.

Thanks
Mark.

Mark Rutland (13):
  arm64: lib: __arch_clear_user(): fold fixups into body
  arm64: lib: __arch_copy_from_user(): fold fixups into body
  arm64: lib: __arch_copy_to_user(): fold fixups into body
  arm64: kvm: use kvm_exception_table_entry
  arm64: factor out GPR numbering helpers
  arm64: gpr-num: support W registers
  arm64: extable: consolidate definitions
  arm64: extable: make fixup_exception() return bool
  arm64: extable: use `ex` for `exception_table_entry`
  arm64: extable: add `type` and `data` fields
  arm64: extable: add a dedicated uaccess handler
  arm64: extable: add load_unaligned_zeropad() handler
  arm64: vmlinux.lds.S: remove `.fixup` section

This looks like a very worthwhile improvement to me, as it moves
complexity from the asm fault sites to the C handling code.

For the series,

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

 arch/arm64/include/asm/asm-extable.h    | 95 +++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/asm-uaccess.h    |  7 ++-
 arch/arm64/include/asm/assembler.h      | 29 +---------
 arch/arm64/include/asm/extable.h        | 23 +++++---
 arch/arm64/include/asm/futex.h          | 25 +++------
 arch/arm64/include/asm/gpr-num.h        | 26 +++++++++
 arch/arm64/include/asm/kvm_asm.h        |  7 +--
 arch/arm64/include/asm/sysreg.h         | 25 +++------
 arch/arm64/include/asm/uaccess.h        | 26 ++-------
 arch/arm64/include/asm/word-at-a-time.h | 21 ++------
 arch/arm64/kernel/armv8_deprecated.c    | 12 ++---
 arch/arm64/kernel/traps.c               |  9 +---
 arch/arm64/kernel/vmlinux.lds.S         |  1 -
 arch/arm64/kvm/hyp/include/hyp/switch.h | 10 ++--
 arch/arm64/lib/clear_user.S             |  9 ++--
 arch/arm64/lib/copy_from_user.S         |  7 +--
 arch/arm64/lib/copy_to_user.S           |  7 +--
 arch/arm64/mm/extable.c                 | 85 +++++++++++++++++++++++++----
 arch/arm64/net/bpf_jit_comp.c           |  9 ++--
 scripts/sorttable.c                     | 30 +++++++++++
 20 files changed, 306 insertions(+), 157 deletions(-)
 create mode 100644 arch/arm64/include/asm/asm-extable.h
 create mode 100644 arch/arm64/include/asm/gpr-num.h

--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel