This is the mail archive of the
mailing list for the GCC project.
__patchable_function_entries is flawed
- From: Fangrui Song <i at maskray dot me>
- To: "gcc at gcc dot gnu dot org" <gcc at gcc dot gnu dot org>
- Cc: nd <nd at arm dot com>, Szabolcs Nagy <Szabolcs dot Nagy at arm dot com>, Martin Liška <mliska at suse dot cz>, Alexander Monakov <amonakov at ispras dot ru>, Torsten Duwe <duwe at suse dot de>, Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>, "nickc at redhat dot com" <nickc at redhat dot com>
- Date: Wed, 8 Jan 2020 01:16:31 -0800
- Subject: __patchable_function_entries is flawed
- References: <firstname.lastname@example.org> <email@example.com> <firstname.lastname@example.org>
On 2020-01-07, Szabolcs Nagy wrote:
On 07/01/2020 07:25, Fangrui Song wrote:
On 2020-01-06, Fangrui Song wrote:
The addresses of NOPs are collected in a section named __patchable_function_entries.
A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE);
preemptible: symbolic relocation (e.g. R_X86_64_64)).
In either case, the section contents will be modified at runtime.
Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
pie/pic should either imply writable __patchable_function_entries,
or __patchable_function_entries should be documented to be offsets
from some base address in the module: the users of it have to modify
.text and do lowlevel hacks so they should be able to handle such
i think it's worth opening a gcc bug report.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93194 patch posted to gcc-patches.
PC-relative relocation types (R_X86_64_64, R_AARCH64_PREL64,
R_PPC64_REL64) can avoid dynamic relocations, and avoid the need for
SHF_WRITE. Unfortunately, it seems we cannot re-interpret
__patchable_function_entries. I have found 2 other problems with the
current __patchable_function_entries. Perhaps we need a new design.
__patchable_function_entries will always be collected by --gc-sections
(.tmp_vmlinux1 is not linked with --gc-sections, so it appears that
the Linux kernel is not affected.)
I think the solution requires a GNU ld and gold side fix:
If you clang>=8, you can play with clang -fstack-size-section -c a.cc
Basically I think __patchable_function_entry has to behave like .stack_sizes .
__patchable_function_entries should consider comdat groups.
It can cause linker failures (GNU ld, gold and lld). It is fairly
easy to trigger with C++ inline.
(OK, the Linux kernel does not speak C++.)
(Clang's function multi-versioning implementation uses comdat, even in
C. I don't have an example with GCC. My GCC __attribute__((target(..))) is broken.)
The two issues make -fpatchable-function-entry useless for user
applications. I still hope we can make the solution more general, not
restricted to the Linux kernel. We've already implemented enough GCC
options for dynamic ftrace (with regs) and live patching: -mfentry
-mnop-mcount -mrecord-mcount -mhotpatch -fpatchable-function-entry.
FWIW I am working on a clang/llvm patch https://reviews.llvm.org/D72215
I have tried resolving these problems.
("o" (SHF_LINK_ORDER+sh_link) is not recognized by GNU as.)