__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>
If the final module has a reference in its symbol table to a kernel
symbol and the symbol version differs from the kernel symbol version,
the module will be unloadable.
Have kpatch-build emit a clear error and die in such a case instead
of providing an unusable module.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
The CRCs of exported symbols Module.symvers can differ between the
original build and the patched build.
In such a case, it is probably wise to rework the patch to avoid such
modifications.
Warn when a symbol changes version in the exported symbol list.
Fixes issue #1084
Signed-off-by: Julien Thierry <jthierry@redhat.com>
When patching an OOT module, the symbol version file is obtained by
combining the file from the module build and the Module.symvers file
provided with kernel headers. This is done for each modified .o in the
OOT build.
Create the final Module.symvers file once for the whole OOT module.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
The modpost step complains about one of our generated files, output.o
and that it can't find a corresponding .cmd file for it (full path names
stripped):
WARNING: could not find .output.o.cmd for output.o
This was turned into an error in v5.8:
.output.o.cmd: No such file or directory
Avoid this by creating an empty .cmd file so that modpost acknowledges
that the file exists, but doesn't parse anything out of it.
Fixes#1125
Reported-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> (for v5.8+)
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Fail the kpatch build, while attempted to build on unsupported
architecture. The build will eventually fail but with the error that
doesn't hint the user clearly about the support status of kpatch, on the
architecture. This patch forces the build error with the message about
the unsupported status, avoiding confusion to the user.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Before kpatch-build would only keep build.log with --debug option
specified, but it also makes sense to keep it if --skip-cleanup is
specified.
Signed-off-by: Artem Savkov <asavkov@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>
Linux kernel tristate config options allows selected feature, either to
be built-in to the kernel or to be built as a kernel module. When built
as a kernel module, it's expected that the module, will be built with
module information such as author, license, description and others.
For each of the modinfo, a corresponding __UNIQUE_ID_ symbol is
generated. Their lookup succeeds in the case of module but fails when
selected to built-in to the kernel, the reason being that the kernel
discards these __UNIQUE_ID_ symbols during linking. Add __UNIQUE_ID_
symbols to maybe_discarded_sym list, to avoid failure in case of
table->object is vmlinux.
i.e.:
# cat .config|grep IOSCHED_BFQ (can be compiled as module too)
CONFIG_IOSCHED_BFQ=y
# readelf -sW ./block/bfq-iosched.o|grep UNIQUE
219: 0000000000000000 54 OBJECT LOCAL DEFAULT 267 __UNIQUE_ID_description223
220: 0000000000000036 16 OBJECT LOCAL DEFAULT 267 __UNIQUE_ID_license222
221: 0000000000000046 19 OBJECT LOCAL DEFAULT 267 __UNIQUE_ID_file221
222: 0000000000000059 25 OBJECT LOCAL DEFAULT 267 __UNIQUE_ID_author220
223: 0000000000000072 22 OBJECT LOCAL DEFAULT 267 __UNIQUE_ID_alias219
the line below before the kpatch error, is a debug printf to find the failing lookup symbol:
Failed lookup for __UNIQUE_ID_description223
/root/kpatch/kpatch-build/create-diff-object: ERROR: bfq-iosched.o: find_local_syms: 180: couldn't find matching bfq-iosched.c local symbols in ./vmlinux symbol table
with the patch, it successfully builds with both y/m config options:
...
bfq-iosched.o: changed function: bfq_idle_slice_timer
Patched objects: vmlinux
Building patch module:
livepatch-0001-block-bfq-fix-use-after-free-in-b.ko
SUCCESS
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Kernel livepatching modules build on GCC 10, with patched functions
referring to local function would fail to load with the error:
module_64: livepatch_ext4_cond_resched: Expected nop after call, got 7fe5fb78 at ext4_setup_system_zone+0x460/0xc90 [livepatch_ext4_cond_resched]
for more details on the error, refer to discussion at:
https://lkml.kernel.org/r/1508217523-18885-1-git-send-email-kamalesh@linux.vnet.ibm.com
the reason was that the gcc-plugin would skip the pass on error, failing
to convert the local calls into global, i.e on ppc64le every global call
is followed by a nop instruction, that gets replaced by the kernel to
restore
the TOC/r2 value of the callee, while parsing the relocations and would
skip the TOC restoration for local functions, where the TOC remains the
same across sibling functions.
GCC 10 commit 07c48b61a082("[RS6000] Put call cookie back in AIX/ELFv2
call patterns") merged a couple of call codes definition, breaking the
plugin. Change the plugin codes to match the GCC 10 codes.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Abort building the klp module, if the code for local and non-local calls
are not found instead of skipping the pass and building module, which
might result in un-loadable module with the kernel error:
module_64: livepatch_ext4_cond_resched: Expected nop after call, got 7fe5fb78 at ext4_setup_system_zone+0x460/0xc90 [livepatch_ext4_cond_resched]
gcc would not allow me to use "can't" in the error message and throw
build error:
gcc-plugins/ppc64le-plugin.c:49:17: error: contraction ‘can't’ in format; use ‘cannot’ instead [-Werror=format-diag]
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Building on Fedora 32 with GCC 10.1.1, triggers build failures:
In file included from gcc-plugins/ppc64le-plugin.c:1:
gcc-plugins/gcc-common.h:37:10: fatal error: params.h: No such file or directory
37 | #include "params.h"
| ^~~~~~~~~~
compilation terminated.
In file included from gcc-plugins/ppc64le-plugin.c:1:
gcc-plugins/gcc-common.h:841:13: error: redefinition of ‘static bool is_a_helper<T>::test(U*) [with U = const gimple; T = const ggoto*]’
841 | inline bool is_a_helper<const ggoto *>::test(const_gimple gs)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from gcc-plugins/gcc-common.h:124,
from gcc-plugins/ppc64le-plugin.c:1:
/usr/lib/gcc/ppc64le-redhat-linux/10/plugin/include/gimple.h:1037:1:
note: ‘static bool is_a_helper<T>::test(U*) [with U = const gimple; T = const ggoto*]’ previously declared here
1037 | is_a_helper <const ggoto *>::test (const gimple *gs)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from gcc-plugins/ppc64le-plugin.c:1:
gcc-plugins/gcc-common.h:848:13: error: redefinition of ‘static bool is_a_helper<T>::test(U*) [with U = const gimple; T = const greturn*]’
848 | inline bool is_a_helper<const greturn *>::test(const_gimple gs)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from gcc-plugins/gcc-common.h:124,
from gcc-plugins/ppc64le-plugin.c:1:
/usr/lib/gcc/ppc64le-redhat-linux/10/plugin/include/gimple.h:1489:1:
note: ‘static bool is_a_helper<T>::test(U*) [with U = const gimple; T = const greturn*]’ previously declared here
1489 | is_a_helper <const greturn *>::test (const gimple *gs)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
as per kernel commit c7527373fe28 ("gcc-common.h: Update for GCC 10")
"params.h header file has been dropped from GCC 10 and is_a_helper()
macro is now defined in gimple.h"
this patch fix them by guarding the both param.h header file and
is_a_helper() with #ifdef checking for gcc version < 10000.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.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)
Or, to be exact, with addend values which cannot be represented by
a signed int variable.
This only applies to the old KPatch core.
Commit 15067fcd "kmod/core: apply dynrela addend for R_X86_64_64" fixed
calculation of the values for R_X86_64_64 dynrelas. This revealed
another issue, similar to https://github.com/dynup/kpatch/issues/1064.
Dynrelas are stored as 'struct kpatch_patch_dynrela' instances in the
patch module but both the patch module and kpatch.ko use
'struct kpatch_dynrela' to work with the dynrelas. 'addend' has type
'long' in kpatch_patch_dynrela but 'int' in kpatch_dynrela, so this
value can be truncated when read.
R_X86_64_64 dynrela can be created, for example, if a patch for vmlinux
refers to something like '(unsigned long)&idt_table+0x80000000' (a global
variable which is not exported, with some addend).
The addend == +0x80000000, however, effectively becomes 0xffffffff80000000
(== -0x80000000) due to this bug.
Unfortunately, 'struct kpatch_dynrela' is a part of the ABI between
kpatch.ko and patch modules. Plain changing 'int addend' into 'long addend'
there could be problematic. The patch module built using the new
'struct kpatch_dynrela' will either fail to load if kpatch.ko is using the old
'struct kpatch_dynrela' or cause crashes or data corruptions. Unloading
and reloading patch modules and kpatch.ko is not always an option
either.
Luckily, R_X86_64_64 dynrelas seem to be quite rare in the production
patch modules and R_X86_64_64 dynrelas with large addends are expected
to be even more rare.
So, instead of fixing the truncation of addends right away, I propose to
detect it, for now, when building a patch. If one never hits such conditions,
it is not worth it to fix the issue. If R_X86_64_64 dynrelas with large
addends do happen and cannot be avoided, we can try to figure out how to
fix this properly, without breaking too much.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
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>
Starting with Linux 5.8, vmlinux-specific KLP relas are applied early,
before all the special section initializations are done.
This means that jump labels can now be supported for cases where the
corresponding static keys live in the core kernel (vmlinux).
It also means that paravirt patching and alternatives can also now be
supported without the need for the .klp.arch sections.
This simplifies things greatly for newer kernels. We just have to make
sure that module-specific KLP relas aren't created for special sections.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
There were a few kernels (4.7 and 4.8) which didn't have support for
.klp.arch sections, but for which we still tried to use
CONFIG_LIVEPATCH. Those are inherently buggy, so just drop
CONFIG_LIVEPATCH support for them altogether.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
kpatch.ko has been quietly deprecated for a while, because there are
some known issues, including special section initialization ordering
issues. Starting with Linux 5.7, it will be completely broken because
kallsyms_lookup_name() will no longer be exported.
Add a warning to make its deprecation status more obvious.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Do some kpatch-build script cleanups to improve readability. This
is only a cleanup and shouldn't affect any functionality.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
With Linux commit 5190044c2965 ("modpost: move the namespace field in
Module.symvers last"), the format of Module.symvers has changed yet
again.
Use a completely different approach for figuring out the format. If a
column has "vmlinux", assume that's the "Module" column.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
We have recently encountered a situation when a patched function
had more than one jump label (static branches with the same static key
used to turn on/off some debugging feature). As it is often the case
with jump labels, their locations were far from obvious in the source
code, hidden in the chains of inline functions.
create-diff-object, however, exits after it has reported one jump label.
This is inconvenient, because, after one updates the patch to avoid
that jump label, the next build of the binary patch reveals another
one and fails again, and so on. It can be very time-consuming.
Let us report all jump labels first.
Before this commit the messages looked like this:
kpatch-build/create-diff-object: ERROR: dev.o:
kpatch_regenerate_special_section: 2084:
Found a jump label at ploop_req_state_process()+0x220, using key css_stacks_on.
Jump labels aren't currently supported. Use static_key_enabled() instead.
After:
dev.o: Found a jump label at ploop_req_state_process+0x220, key: css_stacks_on.
dev.o: Found a jump label at ploop_ioctl+0x2708, key: css_stacks_on.
kpatch-build/create-diff-object: ERROR: dev.o:
kpatch_regenerate_special_section: 2123:
Found 2 jump label(s) in the patched code.
Jump labels aren't currently supported. Use static_key_enabled() instead.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
The dynrela (aka .klp.rela) conversion logic is notoriously complex and
fragile. Simplify it and improve the comments.
This is mainly a cosmetic change. In theory it shouldn't change
functionality or break anything.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
On powerpc, "readelf -s" of vmlinux shows both .dynsym and .symtab.
.dynsym is just a subset of .symtab, so skip it to avoid duplicates.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
symtab_read() is quite fragile because it relies on the fact that the
first and second loops have the exact same conditions.
Instead just change the first loop to count all the lines in the file,
to get an upper bound for allocation. It's ok to over-allocate
slightly.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reverse the if condition and use a 'continue' statement to reduce
indentation and improve readability.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
A symbol in the original object might get split in several sub-functions
in the patched object, which can themselves be bundled (and use a
separate rela section). References to local static variables from the
original function, might have been moved in one of the sub-functions
in the patched object.
Look for references to local static variables in the rela section
of child symbols in the patched object.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Consider symbols containing .part. in their names as sub-function
of the symbols they are derived from (if such symbol still exists in the
object file).
Signed-off-by: Julien Thierry <jthierry@redhat.com>
A symbol associated to a function can be split into multiple
sub-functions. Currently, kpatch only supports one child per function.
Extend this to support an arbitrary number of sub-function per function.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
When a child symbol has changed, the parent symbol is only needed
in the output object if the child symbol is unpatchable on its own.
This is the case when the child symbol does not have its own profiling
call.
Only include unchanged parent symbols if their child has changed and
the child does not have a function profiling call.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
When a ppcle64 ".toc" section contains only constants, the compiler
might not (won't?) create a corresponding ".rela.toc" section.
In such cases, create-diff-object crashes, assuming ".rela.toc" exists
whenever .toc exists. Simply report that no rela are available when
looking up possible relocations in .toc.
Fixes#1078.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Internal CI is reporting a SIGSEGV in create-diff-object when it
processes macro-callbacks.patch, starting with 19baa5b7c7
("create-diff-object: process debug sections last").
The problem is that, after changing the order between callback and debug
section inclusion, kpatch_include_debug_sections() now tries to include
the callback section symbols. But kpatch_include_callback_elements()
inadvertently un-includes the callback section symbols (e.g.,
".kpatch.callbacks.pre_patch") when it un-includes the callback struct
symbols (e.g., "kpatch_pre_patch_data").
So after kpatch_elf_teardown(kelf_patched), the callback section symbols
get freed even though there are DWARF .debug_info relocations which
reference them. Then kpatch_check_relocations() goes off into the weeds
when it accesses one of the freed symbols.
Fix it by refining the callback un-include logic so that it *only*
strips the struct object symbols.
Fixes: 19baa5b7c7 ("create-diff-object: process debug sections last")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
