rela_insn() only cares about the base section. Convert it to take a
non-rela section as its argument instead of a relasec.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Add support for R_X86_64_NONE. With an upstream kernel, it's quite
rare, only used for a few jump labels. With older kernels it was used
for fentry hooks. Either way, it should be treated like a PC-relative
relocation.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
It doesn't make sense to disassemble a data section. That just happened
to work by accident. PC-relative offsets only need adjusting when
associated with an instruction.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Several functions have a boolean semantic, but don't actually return
bool, which is confusing. Fix that.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Several functions expect to take a ".rela" section as an argument. Make
such cases more clear by renaming "sec" -> "relasec".
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Debug sections are intended to refer to the patch module only. And in
fact, any debug section references to non-included symbols are stripped
in kpatch_include_debug_sections(). So there's no need for
need_dynrela() to even think about it.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
A seg fault was reported:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7f18c8e in __strcmp_avx2 () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install elfutils-libelf-0.186-1.fc34.x86_64 zlib-1.2.11-26.fc34.x86_64
(gdb) bt
#0 0x00007ffff7f18c8e in __strcmp_avx2 () from /lib64/libc.so.6
#1 0x000000000040a0f7 in kpatch_is_core_module_symbol (name=0x0) at create-diff-object.c:3060
#2 0x000000000040a267 in need_dynrela (kelf=0x4669a0, table=0x92af30, sec=0x6d6b20, rela=0x8c7fd0) at create-diff-object.c:3117
#3 0x000000000040a4cc in kpatch_create_intermediate_sections (kelf=0x4669a0, table=0x92af30, objname=0x7fffffffcfc6 "vmlinux", pmod_name=0x7fffffffd020 "livepatch_a") at create-diff-object.c:3281
#4 0x000000000040c7c5 in main (argc=8, argv=0x7fffffffca48) at create-diff-object.c:3931
It happened because 'rela->sym->name' was NULL, and
kpatch_is_core_module_symbol() tried to dereference it.
Here's the corresponding relocation:
Relocation section [455] '.rela.debug_loclists' for section [454] '.debug_loclists' at offset 0xd0478 contains 2432 entries:
Offset Type Value Addend Name
...
0x000000000000aad6 X86_64_64 000000000000000000 +32 .LC55
...
That '.LC55' symbol lives in the following section:
[104] .rodata.btf_show_end_aggr_type.str1.8 PROGBITS 0000000000000000 00003ef0 00000021 1 AMS 0 0 8
The problem is that the symbol wasn't included in the output file
(though its corresponding section was). So it got zeroed by
kpatch_elf_teardown(), which was designed to trigger seg faults to help
find bugs like this.
The string literal sections which hold the '.LCx' string symbols are
already being included. Include their symbols as well.
Fixes#1257.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
The __LINE__ detection code looks for "printk", which on newer kernels
has been renamed to "_printk". Fix the check for newer kernels.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
For x86, we already check arg2 and arg3 for `__LINE__` immediate load
detection. For parity, do the same thing for the other two arches.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Technically we don't support s390 yet, but it's coming soon and there's
no harm in merging this one early. In fact this came in handy for
testing my endian fixes with #1203.
Note it doesn't actually do anything since 'kelf->arch' can't actually
get set to 'S390' yet. But it should work nicely with #1203 as it
evolves.
This is based on the patch from C. Erastus Toe in #1243, though there
may still be a few outstanding issues to look at in that PR, based on
some of the code review comments.
Originally-by: C. Erastus Toe <ctoe@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
With s390 support coming, we'll soon have to worry about endianness
issues when doing cross-compiles and cross-arch unit tests. Make
insn_is_load_immediate() endian-agnostic.
Suggested-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
For ppc64le, if a rela goes through the .toc, it requires an extra level
of indirection. Use toc_rela() here to ensure it gets the rela we care
about. This will be needed for the upcoming patch which checks for
`__func__`.
For non-ppc64le arches, and for ppc64le relas which don't go through the
.toc, toc_rela() is a no-op which just returns the rela. So this is
harmless for non-.toc cases.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
The arch-specific versions of kpatch_line_macro_change_only() are mostly
duplicate code. Unify them into a single implementation.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
If the only reference to the `__dyndbg` section is through a jump table
entry, the section doesn't get included and the jump table relocations
end up with a dangling reference to an UNDEF section symbol.
Make sure jump table referenced dynamic debug symbols get their sections
included.
Fixes#1253.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
libelf can read and write various architecture ELF files that may
differ from the host system. Instead of using preprocessor directives
to build architecture-specific code as per the current host, detect the
intended target architecture from the input ELF files.
Based-on: https://github.com/dynup/kpatch/pull/1179
Signed-off-by: Bill Wendling <morbo@google.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> [small tweaks]
The last part of kpatch_elf_open() calls kpatch_find_func_profiling_calls() to
find and set sym->has_func_profiling. However, only create-diff-object.c
requires sym->has_func_profiling, so remove the call from
kpatch_elf_open() and let the lone user, create-diff-object, provide and
call it as needed.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Commit 134ab5bd1883 ("objtool,x86: Replace alternatives with .retpoline_sites")
in the kernel starts keeping track of retpoline thunk call sites in a
dedicated section rather than via the alternatives mechanism.
The .retpoline_sites section needs to have its entries and relocations
for changed symbols included in the patch ELF when building for kernel
5.16+ with CONFIG_RETPOLINE=y.
Signed-off-by: Markus Boehme <markubo@amazon.com>
In kpatch-build, there are a number of places where a dynamic allocation
is performed, but the allocation is not checked for a failure. The
common pattern in kpatch-build is to check whether the returned pointer
is NULL, and if so, invoke the ERROR() macro to print a message and
abort the program.
kpatch_create_mcount_sections(), CORRELATE_ELEMENT(), and
create_klp_arch_sections() all had dynamic allocations without failure
checks. This diff adjusts those callsites to properly check for a failed
allocation, and ERROR() accordingly.
Signed-off-by: David Vernet <void@manifault.com>
Upstream v5.14+ kernel change a358f40600b3 ("once: implement
DO_ONCE_LITE for non-fast-path "do once" functionality") consolidated a
bunch of do-once macros into a common macro:
#define DO_ONCE_LITE_IF(condition, func, ...) \
({ \
static bool __section(".data.once") __already_done; \
...
which replaced static local variable __warned with __already_done.
Update any __warned static local checks to also look for the new
__already_done variable as well.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
RHEL-9 integration tests revealed that the kernel now makes use of
R_PPC64_REL64 relocations in the jump table, but need_dynrela() contains
code to specifically skip any R_PPC64_REL64 type when determining if a
relocation should be turned into dynrela.
Kamalesh Babulal explains:
I tried digging a little deeper and the upstream Kernel commit
b0b3b2c78ec (powerpc: Switch to relative jump labels) in v5.13,
introduced the change of generating relocation entries of type
R_PPC64_REL64, instead of absolute relocation type R_PPC64_ADDR64:
Relocation section '.rela__jump_table' at offset 0x1a87d8 contains 303 entries:
Offset Info Type Symbol's Value Symbol's Name + Addend
...
00000000000003c8 000007910000002c R_PPC64_REL64 0000000000000000 __tracepoint_netif_receive_skb + 8
...
Relax the existing check in need_dynrela() for .rela__jump_table
R_PPC64_REL64 relocations in case we need dynrelas for them.
Fixes: #1212
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Building data-read-mostly.patch on rhel-9.0-beta for ppc64le leads to a
segmentation fault:
Program received signal SIGSEGV, Segmentation fault.
kpatch_check_relocations (kelf=0x10040490) at create-diff-object.c:2571
2571 sdata = rela->sym->sec->data;
(gdb) bt
(gdb) p rela->sym->sec->data
Cannot access memory at address 0x160000007e
Valgrind narrows the problem down to invalid reads through rela->sym in
kpatch-check-relocations().
The culprits are kpatch_create_intermediate_sections(), which marks
symbols referenced by rela sections that are now dynrelas to be
stripped, and kpatch_strip_unneeded_syms(), which removes and frees
them.
The problem with the symbol stripping is that multiple relas may
reference the same ELF symbol. If any remaining relocation references a
shared symbol, we must keep it.
Replace the symbol->strip boolean with an enumeration:
SYMBOL_DEFAULT - initial value, symbol usage unknown
SYMBOL_USED - symbol is definitely used by a rela
SYMBOL_STRIP - symbol was only referenced by dynrela(s)
Allow transitions from SYMBOL_DEFAULT to SYMBOL_* and SYMBOL_STRIP to
SYMBOL_USED, but _not_ SYMBOL_USED to SYMBOL_*.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
While theoretically most of these functions can work both ways right now
all calls follow the same pattern: first argument is orig element and
second is patched element. Rename the arguments so that these functions
are used in the same fashion going forward. This allows us to cut some
corners such as removing the elseif statement in kpatch_correlate_symbol().
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Rename "base" to "orig" when referencing object files and their contents
to be consistent with temporary directory structure.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
So far create-diff-object worked only with objectfiles built from a
single source file. To support object-files built from multiple sources
such as linked vmlinux.o, we call locals_match() for each of STT_FILE
symbols and store the pointer to the beginning of appropriate symbol
block in lookup table in each symbol.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
At the moment lookup_symbol() takes symbol name as an argument which
might not be enough in some cases (e.g. for objectfiles built from
multiple source files). Make it accept full symbol structure.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
For each printk() call site, CONFIG_PRINTK_INDEX makes a static local
struct named `_entry`, and then adds a pointer to it in the
`.printk_index` section.
When regenerating the `.printk_index` section for the patch module, we
only need to include those entries which are referenced by included
functions. Luckily this is a common pattern already used by several
other "special" sections. Add `.printk_index` to the special section
handling logic.
Fixes: #1206
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
CONFIG_PRINTK_INDEX creates a static local struct variable named
`_entry` for every call site to printk(). The initializer for that
struct assigns the `__LINE__` macro to one of its fields.
Similarly to the WARN macro's usage [1] of `__LINE__`, it causes
problems because it results in the line number getting directly embedded
in the struct. If a line is added or removed higher up in the source
file, the `_entry` struct changes accordingly due to a change in the
printk() call site line number.
`_entry` is similar to other "special" static locals, in that we don't
need to correlate the patched version with the original version. We can
instead just ignore any changes to it.
Any substantial (non-line-number) change to the `_entry` struct would be
a second-order (dependent) effect of a first-order code change, which
would be detected using other means. In that case the patched version
of `_entry` will be included, due to being referenced by the changed
function.
Fixes: #1206
[1] See kpatch_line_macro_change_only()
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Binutils recently became much more aggressive about removing unused
section symbols. Since we can not rely on those being available anymore
add additional checks before using them.
Fixes: #1193
Signed-off-by: Artem Savkov <asavkov@redhat.com>
A STT_SECTION symbol is not needed if if it is not used as a relocation
target. Therefore, a section, in this case a debug section, may not have
a secsym associated with it.
Signed-off-by: Bill Wendling <morbo@google.com>
Improve the relocation check for the case where the referenced symbol
isn't at the beginning of the section.
Note that the check still isn't perfect, as many relocations have a
negative addend. But it's still a lot better than nothing.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
create-build-diff expects .cold functions to be suffixed by an id, which
is not always the case. Drop the trailing '.' when searching for cold
functions.
Fixes: #1160
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make kpatch_mangled_strcmp treat two strings as the same in case when
one has a digit tail and the other one doesn't.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
gcc-generated static variables always have a numbered suffix, while
clang-generated static variables are always prepended with a function
name. Change is_special_static() so that it detects both cases.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
__verbose has renamed to __dyndbg since Linux 5.9, commit e5ebffe18e5a
("dyndbg: rename __verbose section to __dyndbg")
Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
Currently all the callers of kpatch_write_output_elf() are creating
.o object files or .ko kernel modules. Neither of these filetypes are
executable on their own, so enhance kpatch_write_output_elf() to accept
file creation mode and update its callers to pass 0664 to match
the expected permissions.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
User disaster123 reports the following build errors:
create-diff-object.c: In function 'kpatch_process_special_sections':
create-diff-object.c:2215:41: error: 'key' may be used uninitialized in this function [-Werror=maybe-uninitialized]
code->sym->name, code->addend, key->sym->name);
^~
create-diff-object.c:2138:22: note: 'key' was declared here
struct rela *code, *key, *rela;
^~~
In file included from kpatch-elf.h:26,
from create-diff-object.c:53:
log.h:20:3: error: 'code' may be used uninitialized in this function [-Werror=maybe-uninitialized]
printf(format, ##__VA_ARGS__); \
^~~~~~
create-diff-object.c:2138:15: note: 'code' was declared here
struct rela *code, *key, *rela;
^~~~
cc1: all warnings being treated as errors
These are reproducible when building with 9.3.1 and 8.3.1 when building
with optimization level > 2 ( CFLAGS=-O2 make ). Fix them by
initializing the reported variables to NULL and verifying that they are
infact non-NULL after processing the __jump_table.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
On x86, .altinstr_aux is used to store temporary code which allows
static_cpu_has() to work before apply_alternatives() has run. This code
is completely inert for modules, because apply_alternatives() runs
during module init, before the module is fully formed. Any changed
references to it (i.e. changed addend) can be ignored. As long as
they're both references to .altinstr_aux, they can be considered equal,
even if the addends differ.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Add the __mcount_loc section on ppc64le. It has pointers to all the
mcount calls. This will enable the ftrace hook to be used for patched
functions.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> (rebased)
Some theoretically unchanged functions can have undesired changes if the
compiler decides to perform inlining in a different way (e.g. because of
newly added references). In such a case, it can be useful to discard
changes to functions that don't actually need modification.
Sadly, this currently doesn't work for functions missing the ftrace hook
(e.g. notrace code) as presence of the hook is checked before
identifying elements to ignore.
Look for functions/sections to ignore earlier.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
