Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AArch64 support for -fpatchable-function-entry #788

Closed
willdeacon opened this issue Dec 9, 2019 · 8 comments
Closed

Add AArch64 support for -fpatchable-function-entry #788

willdeacon opened this issue Dec 9, 2019 · 8 comments
Assignees
Labels
[BUG] llvm A bug that should be fixed in upstream LLVM feature-request Not a bug per-se [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0

Comments

@willdeacon
Copy link

AArch64 support for -fpatchable-function-entry=N was implemented in GCC in 2017 [1] and provides a mechanism to insert NOPs at the very beginning of each function, which in turn can be patched at runtime to allow functions to be 'hooked' prior to the prologue, along with their argument values. This is not something that can be done with the more traditional -pg/'mcount' approach, since the call to _mcount() is only defined to occur as "as one of its [the function's] first operations" [2]. Some other architectures (notably x86), implement -mfentry to solve this issue, but it's considerably less flexible.

The arm64 Linux kernel requires toolchain support for -fpatchable-function-entry in order to support kernel live-patching via CONFIG_FTRACE_WITH_REGS, however it will also be needed in order to support the function graph tracer in the presence of pointer authentication for kernel functions. In this situation, it is necessary to rewrite the return address of the function from the entry hook, meaning that the return address must be authenticated/descrambled from the hook function which requires the unmodified stack pointer on entry to the function being hooked. For the same reasons as before, this is not something that is guaranteed by -pg.

[1] https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00391.html
[2] https://sourceware.org/binutils/docs/gprof/Implementation.html#Implementation

@willdeacon
Copy link
Author

It's probably worth mentioning that the GCC implementation of this seems to be broken when used in combination with -mbranch-protection=bti. With this combination, the landing pad instruction needs to be emitted before the additional nop instructions, otherwise functions would no longer be callable!

@nickdesaulniers nickdesaulniers added [BUG] llvm A bug that should be fixed in upstream LLVM feature-request Not a bug per-se labels Dec 9, 2019
@nickdesaulniers
Copy link
Member

nickdesaulniers commented Dec 9, 2019

https://llvm.org/pr43912

@willdeacon
Copy link
Author

@nickdesaulniers nickdesaulniers self-assigned this Dec 20, 2019
@nickdesaulniers
Copy link
Member

@MaskRay has implemented this in:

  1. https://reviews.llvm.org/D72222
  2. https://reviews.llvm.org/D72221
  3. https://reviews.llvm.org/D72220
  4. https://reviews.llvm.org/D72215

The question about interplay between BTI and PFE still remains.

@nickdesaulniers nickdesaulniers added the [PATCH] Submitted A patch has been submitted for review label Jan 6, 2020
@nickdesaulniers nickdesaulniers removed their assignment Jan 6, 2020
@nickdesaulniers nickdesaulniers added [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 and removed [PATCH] Submitted A patch has been submitted for review labels Jan 10, 2020
@nickdesaulniers
Copy link
Member

The above patches have landed. It sounds like there may be more work involved for BTI interplay, and supporting a non-zero offset.

@nathanchance
Copy link
Member

nathanchance commented Jan 12, 2020

Looks like Linaro's CI shows this regresses all{mod,yes}config for arm64:

https://groups.google.com/d/msg/clang-built-linux/d8z5F3bkfR8/ZgU3gkKvBwAJ

$ make -j$(nproc) -s ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- O=out.aarch64 distclean allyesconfig init/do_mounts.o
/tmp/do_mounts-026544.s: Assembler messages:
/tmp/do_mounts-026544.s:46: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:88: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:130: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:802: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:853: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:883: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:923: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:950: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:977: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:1009: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:1362: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:1502: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:1775: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:1934: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2044: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2181: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2318: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2565: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2603: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2661: Error: junk at end of line, first unrecognized character is `,'
/tmp/do_mounts-026544.s:2720: Error: junk at end of line, first unrecognized character is `,'
clang-10: error: assembler command failed with exit code 1 (use -v to see invocation)
...

creduce spits out... (lol)

a() {}

Original preprocessed file and interestingness test available here.

@nathanchance
Copy link
Member

nathanchance commented Jan 12, 2020

cc @MaskRay (I added some more info to the report above).

@MaskRay
Copy link
Member

MaskRay commented Jan 12, 2020

Should be fixed by llvm/llvm-project@7fa5290

GNU as and GNU ld lack important features that make such metadata sections work reliably. Without the features, __patchable_function_entries can only work in very few scenarios (read as: Linux kernel: no --gc-sections, no COMDAT).

I reported these a few months ago and now with Linux kernel, we see a strong need for them... https://sourceware.org/ml/binutils/2019-11/msg00266.html .tmp_vmlinux1 is broken if we pass --gc-sections...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[BUG] llvm A bug that should be fixed in upstream LLVM feature-request Not a bug per-se [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0
Projects
None yet
Development

No branches or pull requests

4 participants