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>
Skip building insn/* on x86 and gcc-plugin on Power with -Wconversion,
-Wno-sign-converion flags.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Add -Wconversion and -Wno-sign-conversion to CFLAGS. The first flag
should catch any implicit conversions like the one seen with #1065 and
the second flag suppress the warnings between signed and unsigned
integers.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
On x86_64, GCC generates the following instruction to compute
'empty_zero_page - __START_KERNEL_map' (__phys_addr_nodebug(), used in
the implementation of ZERO_PAGE()):
48 ba 00 00 00 00 00 00 00 00 movabs $0x0,%rdx
R_X86_64_64 empty_zero_page+0x80000000
__START_KERNEL_map is 0xffffffff80000000.
However, the relocation addend becomes wrong in the patch module:
48 ba 00 00 00 00 00 00 00 00 movabs $0x0,%rdx
R_X86_64_64 empty_zero_page-0x80000000
Note the sign of the addend.
As a result, ZERO_PAGE(0) returns a wrong value in any function touched
by the patch, which may lead to memory corruption and difficult-to-debug
kernel crashes.
The cause is that 'struct rela' uses 'int' for the addend, which is not
enough to store such values. r_addend from Elf64_Rela is int64_t
(Elf64_Sxword) for that.
Let us use 'long' instead of 'int' for the addend in 'struct rela'.
v2:
* Moved 'addend' field after 'offset' in struct rela to facilitate
structure packing (suggested by Kamalesh Babulal).
Fixes https://github.com/dynup/kpatch/issues/1064.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
When patching kernel module dm-persistent-data, I found
that the KOBJFILE_NAME is incorrectly replaced to
dm_persistent-data while the module name in kernel is
dm_persistent_data.
Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
The current code to find the twin of a local static variable allows two
variables of the same name to be wrongly matched with the other's twin.
While there isn't a magic formula to avoid this, make stricter
requirements for twining static local from the original object with
a symbol from the patched object. This reduces the risk of erroneous
matches.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Process the debug sections only after all the other inclusion logic has
finished, since it makes decisions based on what else has already been
included.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Simplify static local variable correlation and renaming code by using
the newly introduced helpers for section and symbol correlation.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Change 935f199875 ('create-diff-object: simplify mangled function
correlation') simplified the way symbols are correlated and got rid of
symbol section renaming.
As a result a symbol/section can now have a CHANGED status, being
correlated to an element that doesn't have the exact same name. This
will cause lookups to the original object fail when creating the new
patch object.
So lets bring back the symbol/section renaming, but only once they
have actually been correlated.
Fixes: 935f199875 ('create-diff-object: simplify mangled function
correlation')
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Elements from the original object and the patched object can be
correlated using their mangled names. In case an elements (section or
symbol) could be matched with more than one object through mangling,
make sure all elements related to a section are correlated with the
corresponding elements of the twin section.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
When freeing a kpatch_elf, another object might have symbols and
sections twined with elements that are getting freed.
Clear the twin references, so if they are used after the object they
reference is freed, the program will crash.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Kernel commit cb9b55d21fe0 modpost: add support for symbol namespaces
adds a new namespace column to Module.symvers file which can be blank.
fscanf is no longer a viable solution to parse that. Switch to the way
scripts/mod/modpost.c handles this and try to support both versions with
and without namespace column.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Currently, only rela section get freed. This seems like a simple
scope mistake.
Free all sections regardless of their nature in kpatch_elf_teardown()
Signed-off-by: Julien Thierry <jthierry@redhat.com>
The RHEL powerpc kernel is compiled with -O3, which triggers some
"interesting" new optimizations. One of them, which seems to be
relatively common, is the replacing of a function with two separate
"constprop" functions.
Previously we only ever saw a single constprop clone, so we just renamed
the patched version of the function to match the original version. Now
that we can have two such clones, that no longer makes sense.
Instead of renaming functions, just improve the correlation logic such
that they can be correlated despite having slightly different symbol
names. The first clone in the original object is correlated with the
first clone in the patched object; the second clone is correlated with
the second clone; and so on.
This assumes that the order of the symbols and sections doesn't change,
which seems to be a reasonable assumption based on past experience with
the compiler. Otherwise it will just unnecessarily mark the cloned
constprop functions as changed, which is annoying but harmless, and
noticeable by a human anyway.
Fixes#935.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Building a kpatch for a module with this Makefile:
The Makefile is as follow:
obj-m += m_hello.o
m_hello-y = hello.o
default:
$(MAKE) -C /lib/modules/4.4.21-69-default/build M=$(shell pwd) modules
clean:
$(MAKE) -C /lib/modules/4.4.21-69-default/build M=$(shell pwd) clean
results in kpatch-build "ERROR: two parent matches for hello.o".
The problem is that find_parent_obj() looks for filenames like so:
% grep -l hello.o ./.*.cmd | grep -Fv hello.o
.m_hello.ko.cmd
.m_hello.o.cmd
where .m_hello.ko.cmd is the parant for m_hello.o, and .m_hello.o.cmd is the
parant for hello.o, but because the "hello.o" is a substring of "m_hello.o",
it will cause "m_hello.o" to be matched for the "hello.o" as well.
Fix this by using grep's -w|--word-regexp option to force it to match
whole words instead of substrings.
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
If the symbol associated with a relocation does not have a section set,
nothing is done for that relocation.
Skip iterating through all the symbols of the ELF file in such a case.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
rela_insn() only retrieves information about an instruction and does not
modify sections or relocations.
Add const to make this explicit.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
There is no point inspecting through the symbols of the ELF files
(original and patched) when the ELF headers do not meet requirements.
Check ELF headers as soon as the files are mapped.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
We are seeing the following error on a real world patch:
unsupported reference to special section __barrier_nospec_fixup
The kpatch commit bb444c2168 ("create-diff-object: Check for *_fixup
sections changes") created this error because we were trying to be
future proof. However, that may have been overly paranoid, as it
doesn't seem likely that those fixup sections will need relocations
anytime soon, because the replacement instructions are manually
generated in code. And anyway that "future proof" commit breaks the
present.
Also we decided at LPC that we are going to remove .klp.arch sections
anyway, so once that happens we will be fully future-proof anyway.
This reverts commit bb444c2168.
Fixes#974.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Somewhere starting with 5.3 (probably with 9f69a496f100 "kbuild: split
out *.mod out of {single,multi}-used-m rules", but that is not
confirmed) .mod and correcponding .mod.cmd files started showing up
during module builds throwing off kpatch-build's find_parent_obj() func.
Filter out any files ending with .mod.cmd as they are definitely not the
parent.
Signed-off-by: Artem Savkov <artem.savkov@gmail.com>
Not every distro out there supports /etc/os-release file.
This file is useful for obtaining given distro defaults, but not
essential for the script to work (when all parameters are passed
on a command line).
To avoid warnings or unwanted errors, make sourcing of this file
conditional.
Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Run the input patch(es) through lsdiff and then verify that no obviously
unsupported files are directly modified (e.g. assembly .S files).
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Recent distros don't require you to set 'ulimit -c unlimited'. Instead
they place core files in a distro-specific location. Update the SIGSEGV
error message accordingly.
Fixes: #1025
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
While static keys (jump labels) are currently broken in livepatch, a
broken dynamic debug static key is harmless since it just disables
dynamically enabled debug printks in the patched code.
Fixes: #1021
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
We saw the following panic on ppc64le when loading the macro-callbacks
integration test:
livepatch: enabling patch 'kpatch_macro_callbacks'
Oops: Exception in kernel mode, sig: 4 [#1]
LE SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: kpatch_macro_callbacks(OEK+) rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache sunrpc sg pseries_rng xts vmx_crypto xfs libcrc32c sd_mod ibmvscsi scsi_transport_srp ibmveth dm_mirror dm_region_hash dm_log dm_mod [last unloaded: kpatch_gcc_static_local_var_6]
CPU: 2 PID: 17445 Comm: insmod Kdump: loaded Tainted: G OE K --------- - - 4.18.0-128.el8.ppc64le #1
NIP: d00000000bb708e0 LR: c0000000001fd610 CTR: d00000000bb708e0
REGS: c00000040e98f640 TRAP: 0700 Tainted: G OE K --------- - - (4.18.0-128.el8.ppc64le)
MSR: 800000000288b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28008228 XER: 20040003
CFAR: c0000000001fd60c IRQMASK: 0
GPR00: c0000000001fd5c0 c00000040e98f8c0 c000000001662a00 c000000733525400
GPR04: 0000000000000800 0000000000000800 c0000000015e2c00 c0000007335254a8
GPR08: 0000000000000001 d00000000bb708e0 c0000007eeb68400 0000000000000000
GPR12: d00000000bb708e0 c000000007fad600 0000000000000001 aaaaaaaaaaaaaaab
GPR16: 000000000000ff20 000000000000fff1 000000000000fff2 d00000000bb90000
GPR20: 00000000000000a9 c00000040e98fc00 c000000000d8a728 c00000040e98fc00
GPR24: d00000000bb73f88 00000000006080c0 d00000000bb73a38 c000000733525400
GPR28: 0000000000000001 c000000733525400 ffffffffffffffed c0000007eeb60900
NIP [d00000000bb708e0] callback_info.isra.0+0x7c/0x66c [kpatch_macro_callbacks]
LR [c0000000001fd610] __klp_enable_patch+0x130/0x230
Call Trace:
[c00000040e98f8c0] [c0000000001fd5c0] __klp_enable_patch+0xe0/0x230 (unreliable)
[c00000040e98f940] [c0000000001fd7d8] klp_enable_patch+0xc8/0x100
[c00000040e98f980] [d00000000bb7079c] patch_init+0x460/0x4cc [kpatch_macro_callbacks]
[c00000040e98fa20] [c000000000010108] do_one_initcall+0x58/0x248
[c00000040e98fae0] [c00000000023b860] do_init_module+0x80/0x330
[c00000040e98fb70] [c0000000002416a4] load_module+0x3994/0x3d00
[c00000040e98fd30] [c000000000241cf4] sys_finit_module+0xc4/0x130
[c00000040e98fe30] [c00000000000b388] system_call+0x5c/0x70
Instruction dump:
7cea482a 48000235 e8410018 48000014 3c620000 e8638160 48000221 e8410018
38210060 e8010010 7c0803a6 4e800020 <0000ae18> 00000000 3c4c0001 3842ae18
The problem was introduced by a recent fix:
e8f7f2dfe8 ("create-diff-object/ppc64le: Fix replace_sections_syms() for bundled symbols")
We didn't notice the fact that there's a hack in
kpatch_include_callback_elements() which reverts the work of
kpatch_replace_sections_syms() for callback function symbols.
The problem is that that revert is only partial, causing the callback
pointers to point to the .TOC data which is located 8 bytes before the
start of the function code. This happens because
kpatch_include_callback_elements() makes the same assumption that
kpatch_replace_sections_syms() had previously made: that bundled symbols
are always located at the start of their corresponding sections.
kpatch_include_callback_elements() mysteriously strips references to the
callback function symbols, replacing them with section symbols. In this
case it replaced a 'pre_patch_callback' function reference with a
'.text.unlikely.pre_patch_callback' section reference. But it didn't
adjust the rela->addend accordingly.
Joe discovered the reasoning for why kpatch_include_callback_elements()
removes function symbol references in the commit log for 7dfad2fb76
("fix dynrela corruption in load/unload hooks"):
In the case of the hook functions, we strip the FUNC symbol to prevent
it from being added to the kpatch.funcs section as a patched function.
But that justification doesn't really make sense, at least not with the
current code. Callbacks aren't added to .kpatch.funcs anyway. They're
classifed as NEW. Only CHANGED functions are added to .kpatch.funcs.
So remove that hack, fixing this bug in the process.
This does have a side effect of showing the callback functions as new
functions, because their symbols are now included.
Before:
aio.o: found callback: post_unpatch_callback
aio.o: found callback: pre_patch_callback
aio.o: found callback: pre_unpatch_callback
aio.o: new function: callback_info.isra.0
After:
aio.o: found callback: post_unpatch_callback
aio.o: found callback: pre_patch_callback
aio.o: found callback: pre_unpatch_callback
aio.o: new function: callback_info.isra.0
aio.o: new function: pre_patch_callback
aio.o: new function: post_patch_callback
aio.o: new function: pre_unpatch_callback
aio.o: new function: post_unpatch_callback
But anyway they _are_ new functions, so the new output seems more
correct to me.
Fixes: e8f7f2dfe8 ("create-diff-object/ppc64le: Fix replace_sections_syms() for bundled symbols")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
The existing comment is wrong. It confusingly conflates the function's
offset, which is 8 bytes from the beginning of the section, with the
function's localentry offset which is 8 bytes from the beginning of the
function.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
group_size variable is assigned right after we enter for loop without
ever being read so there is no need to initialize it to 0 beforehand.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
"funcs" in kpatch_create_patches_sections() and "entries" in
kpatch_create_kpatch_arch_section() were only used by sizeof, replaced
those with corresponding types.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Actually exit on strdup error instead of just printing a warning message
in make_modname().
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
There were 2 insances where return value of find_section_by_name wasn't
checked before dereference.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure symtab section was found before dereferencing it.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure symtab section was found before dereferencing it.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Only user of "entries" variable was sizeof and the value was never
actually used. Use struct name directly instead.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure symtab section was found before dereferencing it.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
newdata variable is allocated through malloc call and requires a NULL
check.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Since ORC_STRUCT_SIZE is used for division in
kpatch_regenerate_orc_sections() we need to make sure that it is
properly set.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure fixup section was found before dereferencing it.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure rela_toc(1|2) are not null before dereferencing them in
rela_equal().
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
The flag -gz[=type] was added in GCC 5. To support older GCC versions
check if the flag is supported before adding it to KCFLAGS.
Fixes: #1012
Signed-off-by: Stefan Strogin <steils@gentoo.org>
On some systems the linker produces compressed debug sections by
default. It is not supported by create-diff-object for now.
Fixes: #877
Signed-off-by: Stefan Strogin <steils@gentoo.org>
With the following patch:
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index e008aefc3a9d..7c70e369390d 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2228,6 +2228,8 @@ static void xs_tcp_shutdown(struct rpc_xprt *xprt)
struct socket *sock = transport->sock;
int skst = transport->inet ? transport->inet->sk_state : TCP_CLOSE;
+ asm("nop");
+
if (sock == NULL)
return;
switch (skst) {
We saw the following panic on a RHEL7.6 kernel:
Unable to handle kernel paging request for data at address 0xd00000000577f390
Faulting instruction address: 0xd000000002e918f4
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: kpatch_3_10_0_957_1_3_1_1(OEK) nfsd nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache sunrpc virtio_balloon ip_tables xfs libcrc32c virtio_net virtio_console virtio_blk virtio_pci virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod
CPU: 9 PID: 5961 Comm: kworker/9:1H Kdump: loaded Tainted: G OE K------------ 3.10.0-957.1.3.el7.ppc64le #1
Workqueue: xprtiod xprt_autoclose [sunrpc]
task: c00000000300c3c0 ti: c0000003f1814000 task.ti: c0000003f1814000
NIP: d000000002e918f4 LR: d000000002e57394 CTR: c00000000089d100
REGS: c0000003f1817980 TRAP: 0300 Tainted: G OE K------------ (3.10.0-957.1.3.el7.ppc64le)
MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE> CR: 240f2084 XER: 20000000
CFAR: 000000010bb5270c DAR: d00000000577f390 DSISR: 40000000 SOFTE: 1
GPR00: c00000000000b054 c0000003f1817c00 d00000000579add8 c000000214f0f4d0
GPR04: c0000003fd618200 c0000003fd618200 0000000000000001 0000000000000dc2
GPR08: 0000000000000dc3 0000000000000000 0000000000000000 d00000000577f370
GPR12: c0000003f1814000 c000000007b85100 c00000000012fd88 c0000003f711bb40
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000001 c0000000013510b0 0000000000000001 fffffffffffffef7
GPR24: 0000000000000000 0000000000000000 0000000000000000 c000000001b60600
GPR28: c000000214f0f000 c000000214f0f4d0 c000000214f0f408 c000000214f0f448
NIP [d000000002e918f4] __rpc_create_common.part.6+0x640/0x533c [sunrpc]
LR [d000000002e57394] xprt_autoclose+0x74/0xe0 [sunrpc]
Call Trace:
[c0000003f1817c00] [c00000000000b054] livepatch_handler+0x30/0x80 (unreliable)
[c0000003f1817c40] [c00000000012333c] process_one_work+0x1dc/0x680
[c0000003f1817ce0] [c000000000123980] worker_thread+0x1a0/0x520
[c0000003f1817d80] [c00000000012fe74] kthread+0xf4/0x100
[c0000003f1817e30] [c00000000000a628] ret_from_kernel_thread+0x5c/0xb4
Instruction dump:
396b4570 f8410018 e98b0020 7d8903a6 4e800420 00000000 73747562 000f49c0
c0000000 3d62fffe 396b4598 f8410018 <e98b0020> 7d8903a6 4e800420 00000000
---[ end trace 98e026b8fa880db7 ]---
The original version of xs_tcp_shutdown() has the following sequence:
0xd000000003cfda44 <xs_tcp_shutdown+148>: addi r1,r1,64
0xd000000003cfda48 <xs_tcp_shutdown+152>: ld r0,16(r1)
0xd000000003cfda4c <xs_tcp_shutdown+156>: ld r29,-24(r1)
0xd000000003cfda50 <xs_tcp_shutdown+160>: ld r30,-16(r1)
0xd000000003cfda54 <xs_tcp_shutdown+164>: ld r31,-8(r1)
0xd000000003cfda58 <xs_tcp_shutdown+168>: mtlr r0
0xd000000003cfda5c <xs_tcp_shutdown+172>: b 0xd000000003cfd768
That is, it restores the stack to the caller's stack frame and then does
a sibling call to the localentry point of xs_reset_transport()). So
when xs_reset_transport() returns, it will return straight to
xs_tcp_shutdown()'s caller (xprt_autoclose).
The patched version of the function has this instead (dumped from a
vmcore):
0xd000000003df0834 <xs_tcp_shutdown+148>: addi r1,r1,64
0xd000000003df0838 <xs_tcp_shutdown+152>: ld r0,16(r1)
0xd000000003df083c <xs_tcp_shutdown+156>: ld r29,-24(r1)
0xd000000003df0840 <xs_tcp_shutdown+160>: ld r30,-16(r1)
0xd000000003df0844 <xs_tcp_shutdown+164>: ld r31,-8(r1)
0xd000000003df0848 <xs_tcp_shutdown+168>: mtlr r0
0xd000000003df084c <xs_tcp_shutdown+172>: b 0xd000000003df0ad0
After restoring the stack, instead of branching directly to
xs_reset_transport(), it (rightfully) branches to a toc stub. A stub is
needed because the function it's branching to is in another module
(branching from the patch module to the sunrpc module).
The stub is:
0xd000000003df0ad0 <xs_tcp_shutdown+816>: addis r11,r2,-1
0xd000000003df0ad4 <xs_tcp_shutdown+820>: addi r11,r11,26328
0xd000000003df0ad8 <xs_tcp_shutdown+824>: std r2,24(r1)
0xd000000003df0adc <xs_tcp_shutdown+828>: ld r12,32(r11)
0xd000000003df0ae0 <xs_tcp_shutdown+832>: mtctr r12
0xd000000003df0ae4 <xs_tcp_shutdown+836>: bctr
And the "std r2,24(r1)" corrupts the caller's stack.
This stub makes sense for a normal call, because the stack would be
owned by the caller of the stub, so it's ok to write r2 to it. But
because this is a sibling call, the stack has been restored and r2 gets
incorrectly saved to the original caller's stack (i.e., xprt_autoclose's
stack).
So xprt_autoclose() -- which is in the sunrpc module -- gets the
livepatch module's toc pointer written to its stack. It panics on when
it tries to use that vlue on its very next call.
Fix it by disallowing sibling calls from patched functions on ppc64le.
In theory we could instead a) generate a custom stub, or b) modify the
kernel livepatch_handler code to save/restore the stack r2 value, but
this is easier for now.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Apply a sed filter to remove "[<localentry>: 8] " info from
readelf --wide --symbols output. This ensures consistent column
data for the awk script creating the new_symbols file.
Fixes#994
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
The list of prerequsite RPMs which are needed to build the kernel RPM is
constantly growing. But at least some of those RPMs aren't strictly
necessary for building the kernel, at least for kpatch-build's purposes.
Requiring them all to be installed is a bit overkill, and sometimes
causes kpatch-build to fail when it doesn't need to.
If the build does fail, we can always check the kpatch.log file and
update the dependencies listed in the README as needed.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Create an error if a patched function uses a jump label. We need this
until upstream livepatch supports them with a .klp.arch section.
Fixes#946.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
After Linux commit 47cdd64be483 ("dynamic_debug: refactor
dynamic_pr_debug and friends"), the name of the static local variable
used for dynamic printks is no longer "descriptor".
Make the is_special_static() check broader such that it doesn't care
about the variable name, and just whitelists any variable in the
__verbose section.
Fixes#990.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Starting with binutils 2.31, the Linux kernel may have R_X86_64_PLT32
relocations. Make sure we support them. This should be as simple as
treating R_X86_64_PLT32 exactly like R_X86_64_PC32 everywhere. For more
details see upstream commit torvalds/linux@b21ebf2.
This also fixes the following issue seen on Fedora 29:
```
$ kpatch-build/kpatch-build -t vmlinux ./test/integration/fedora-27/convert-global-local.patch
Using cache at /home/jpoimboe/.kpatch/src
Testing patch file(s)
Reading special section data
Building original source
Building patched source
Extracting new and modified ELF sections
ERROR: slub.o: 1 function(s) can not be patched
slub.o: function __kmalloc has no fentry/mcount call, unable to patch
/home/jpoimboe/git/kpatch/kpatch-build/create-diff-object: unreconcilable difference
ERROR: 1 error(s) encountered. Check /home/jpoimboe/.kpatch/build.log for more details.
```
Fixes#975.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Currently we do not support changes to functions referring to any of the
*_fixup sections on ppc64le. This patch introduces check for such
changes during the patchability check, where we abort building the
patch module.
This patch implements the phase 1 fix of 3 phases discussed at
https://github.com/dynup/kpatch/issues/974:
"
Phase 1 fix:
For kernel versions which don't have livepatch-specific powerpc code
(currently all kernel versions), kpatch-build needs to assert an error
if it detects that one of the following sections refers to a patched
function: __ftr_fixup, __mmu_ftr_fixup, __fw_ftr_fixup.
"
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Add support for __spec_barrier_fixup (barrier nospec fixup) special
section on ppc64le.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Fix the size of special group __lwsync_fixup on ppc64le.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
commit f8213c87f6 ("lookup: Fix format string for symtab_read() on
PPC64LE") fixed the symbol table lookup when readelf adds ppc64le
"[<localentry>: 8]" info for functions like so:
23: 0000000000000008 96 FUNC LOCAL DEFAULT [<localentry>: 8] 4 cmdline_proc_show
however, it seems that readelf 2.30-57.el8 displays this in a slightly
different format:
24493: c000000000587970 96 FUNC LOCAL DEFAULT 2 cmdline_proc_show [<localentry>: 8]
Instead of adding more cases to kpatch-build's lookup.c scanf format,
let's just delete this information from the symtab file with a quick and
dirty sed regex. This allows us to handle both observed cases (and
perhaps others) while removing the arch-specific scanf formatting in
lookup.c
Fixes: f8213c87f6 ("lookup: Fix format string for symtab_read() on PPC64LE")
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
commit 767d9669bd ("kpatch-build: use readelf instead of eu-readelf")
replaced eu-readelf with readelf for constructing symbol table. The
format of symbol table entries differs a little on Power when the symbol
is a function with binding type LOCAL. For example, consider:
23: 0000000000000008 96 FUNC LOCAL DEFAULT [<localentry>: 8] 4 cmdline_proc_show
An extra column preceding index of the symbol denoting symbol value to
be local entry point offset of the function is printed, with the
current sscanf format string in lookup::symtab_read the values will
mismatch ending with in accurate lookup table getting constructed. This
patch fixes it by introducing an Power specific format string for
function symbols with bind type LOCAL.
Fixes: 767d9669 ("kpatch-build: use readelf instead of eu-readelf")
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
kpatch-elf::create_section_pair would create new rela section, and the
relasec->data->d_type is not set, which is a random value, and it will
use in kpatch-elf::kpatch_write_output_elf
data->d_type = sec->data->d_type;
which would cause Segmentation fault in kpatch_write_output_elf::elf_update.
Program received signal SIGSEGV, Segmentation fault.
(gdb) bt
0 0x00007ffff7bcd8d2 in __elf64_updatefile at elf64_updatefile.c
1 0x00007ffff7bc9bed in write_file at elf_update.c
2 0x00007ffff7bc9f16 in elf_update at elf_update.c
3 0x000000000040ca3d in kpatch_write_output_elf at kpatch-elf.c
4 0x0000000000409a92 in main at create-diff-object.c
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
readelf is more standard, using readelf insteaded we should solve there
issues:
First, using "readelf -s", the symbol name would truncated by 25 chars,
to solve this issue, add option "--wide".
Second, the size may be mixed of decimal and hex, we get the size by "%s",
and use strtoul(size, NULL, 0) to convert the size.
Third, the symbol type is SHN_UNDE, the Ndx display "UND", so changed to
compare with "UND".
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
.altinstr_replacement section may have relocation symbols which need to
be included, therefore we should call kpatch_include_symbol() to ensure
that its section is included as well.
The special section processing should also occur before
kpatch_print_changes() to provide accurate logging info.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
kpatch-elf::kpatch_write_output_elf will call the gelf_getclass()
to acquire the output elf's class. But the input parameter kelf->elf
is NULL, the gelf_getclass(kelf->elf) will return ELFCLASSNONE, not
the value we expect ELFCLASS32 or ELFCLASS64.
the gelf_getclass function code:
int
gelf_getclass (Elf *elf)
{
return elf == NULL || elf->kind != ELF_K_ELF ? ELFCLASSNONE : elf->class;
}
the gelf_newehdr fuction code:
void *
gelf_newehdr (Elf *elf, int class)
{
return (class == ELFCLASS32
? (void *) INTUSE(elf32_newehdr) (elf)
: (void *) INTUSE(elf64_newehdr) (elf));
}
Luckily, when we create a patch for x86_64 or powerpc64, if we pass the
ELFCLASSNONE for the function gelf_newehdr, it will return elf64_newehdr,
so don't cause the fault. But it's better to use the gelf_getclass(elf)
instead of gelf_getclass(kelf->elf).
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
make check using shellcheck version 0.6.0 suggests following
improvements:
In kpatch/kpatch line 160:
if [[ ! -z "$checksum" ]] && [[ -e "$SYSFS/${modname}/checksum"]] ; then
^-- SC2236: Use -n instead of ! -z.
In kpatch-build/kpatch-build line 953:
[[ ! -z "$UNDEFINED" ]] && die "Undefined symbols: $UNDEFINED"
^-- SC2236: Use -n instead of ! -z.
'-n' and '! -z' are used interchangeably across the scripts, let's use
'-n' consistently to check a non-empty string instead of using negation.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
The create-diff-object.c create intermediate ".kpatch.relocations"
sections instead of ".kpatch.dynrelas" sections, and add a new
section ".rela.kpatch.symbols", so we should update the conditions
in function kpatch_create_intermediate_sections for these changed.
Fixes: 87643703a7 ("create-diff-object: create .kpatch.relocations and .kpatch.symbols sections")
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
After changing the gcc name in a linux tree to gcc72, kpatch-build failed to
produce hotpatches with the error message "ERROR: no changed objects found."
This is due to a wrapper script called kpatch-gcc, called while kpatch-build
builds the kernel, which checks if the compiler name matches exactly gcc,
failing the check when comparing to gcc72, and thus not producing the expected
file changed_objs containing the list of changed objects.
This commit fixes this issue by loosening the check on the gcc name.
Signed-off-by: Bruno Loreto <loretob@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.com>
Reviewed-by: Amit Shah <aams@amazon.com>
Reviewed-by: Pawel Wieczorkiewicz <wipawel@amazon.com>
The kpatch-build :: find_parent_obj() function's "deep find" may
failed to find objects if they are not located in current directory:
ERROR: invalid ancestor xxx/xxx.o for xxx/xxx.o.
This is reproducable when building an out-of-tree module of the
following structure:
wwheart@linux41:~/helloworld 0 > tree -a
.
├── buffer_overflow1.ko
├── .buffer_overflow1.ko.cmd
├── buffer_overflow1.mod.c
├── buffer_overflow1.mod.o
├── .buffer_overflow1.mod.o.cmd
├── buffer_overflow1.o
├── .buffer_overflow1.o.cmd
├── hello.c
├── hello.o
├── .hello.o.cmd
├── Makefile
├── modules.order
├── Module.symvers
├── test.patch
├── .tmp_versions
│ └── buffer_overflow1.mod
└── xxx
├── xxx.c
├── xxx.h
├── xxx.o
└── .xxx.o.cmd
wwheart@linux41:~/helloworld 0 > cat test.patch
diff --git a/xxx/xxx.c b/xxx/xxx.c
index aab3c67..d81ad00 100644
--- a/xxx/xxx.c
+++ b/xxx/xxx.c
@@ -1,6 +1,7 @@
#include <linux/kernel.h>
void czf_test(void)
{
+ printk("livepatch test\n");
printk("xxx\n");
}
wwheart@linux41:~/helloworld 0 > cat Makefile
obj-m += buffer_overflow1.o
buffer_overflow1-y += hello.o xxx/xxx.o
Modify the deep find to traverse sub-directories in order to search
the entire tree instead of only the current directory.
Fixes: 8c2792af6c ("kpatch-build: deep find performance improvement")
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
reason: The strdup() function returns a pointer to a new string
which is a duplicate of the string s. Memory for the
new string is obtained with malloc, and can be freed
with free.
here, fix memleak by removing the strdup.
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
reason: Firstly, in the function lookup_open use the malloc to
allocate some memory, but call the function lookup_close
to free the memory.
Secondly, table->obj_sym->name, table->exp_sym->name and
table->exp_sym->objname used the strdup, so them should
free also.
Thirdly, adjust the order of make_nodname, if not, it
will cause an exception when free(exp_sym->objname) in
lookup_close.
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
This reverts commit 87c64519fc.
The jump label support doesn't work with upstream livepatch. Joe
Lawrence found the following ordering issue:
load_module
apply_relocations
/* Livepatch relocation sections are applied by livepatch */
if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
continue;
post_relocation
module_finalize
jump_label_apply_nops << crash
...
do_init_module
do_one_initcall(mod->init)
__init patch_init [kpatch-patch]
klp_register_patch
klp_init_patch
klp_init_object
klp_init_object_loaded
klp_write_object_relocations
So jump_label_apply_nops() is called *before*
klp_write_object_relocations() has had a chance to write the klp
relocations (.klp.rela.kvm_intel.__jump_table, for example).
We need to resolve this upstream first.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
When building out-of-tree modules, gcc may be passed full source
pathnames (like /home/user/testmod/testmod.c). Adjust the filepath
filtering in kpatch-gcc to match against files relative to the
KPATCH_GCC_SRCDIR / kpatch-build SRCDIR prefix.
Fixes: #941
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Add support for jump labels, also known as static jumps, static keys,
static branches, and jump tables. Luckily,
kpatch_process_special_sections() is already generic enough to make this
an easy fix.
Fixes: #931
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
For fun I tried to create a livepatch of upstream patch
ad211f3e94b314a910d4af03178a0b52a7d1ee0a for my kernel. This
caused kpatch-build to fail with a NULL pointer derefence because
base_locals was NULL (returned via kpatch_elf_locals(), which
can return a NULL pointer). This patch fixes the SIGSEGV
via a NULL check. The end result is a live patch is created
and loaded.
Signed-off-by: Balbir singh <bsingharora@gmail.com>
kpatch_mark_ignored_sections include .rodata.str1.1 section but does
not include its section symbol, causing its section symbol can not be
included any more in kpatch_include_standard_elements. After the
section symbol is freed in kpatch_elf_teardown, we got a segmentation
fault in kpatch_create_intermediate_sections.
Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
Both generate randomly modified object files on each build. This breaks
comparing original and patched object file. See also #924.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
Prevents the following shellcheck warning:
In kpatch-build/kpatch-build line 583:
which yumdownloader &>/dev/null || die "yumdownloader (yum-utils or dnf-utils) not installed"
^-- SC2230: which is non-standard. Use builtin 'command -v' instead.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
As discovered in #918, the `__FUNCTION__` static local variable is
similar to the `__func__` variable, in that it refers to the current
function name.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Sometimes due to config-dependency issues or other reasons whole
object-files would get optimized out from final vmlinux/module, in cases
like this create-diff-object would fail during symbol lookup table
creation in lookup_open(). Because lookup_open() call is situated before
we established that objectfile has changed this triggers not only on
real problems, but also during mass-rebulds caused by changes to
header-files. While it usually indicates a real issue with config this
should not prevent kpatch from building.
Move lookup_open() call so that it is called only for changed
object-files.
Fixes#910
Signed-off-by: Artem Savkov <asavkov@redhat.com>
strdup symbol names in kpatch_elf_locals and when noting down hint
instead of just copying pointers so that they are still usable after
we teardown/free kelf_base.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Starting with 1b1eeca7e4c1 "init: allow initcall tables to be emitted using
relative references" [1] __init functions are generating an "__addressable_"
symbol in a ".discarded.addressable" section so it does not show up in final
vmlinux triggering find_local_syms failures. Add "_addressable_" to the list
in maybe_discarded_sym().
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1b1eeca7e4c19fa76d409d4c7b338dba21f2df45
Signed-off-by: Artem Savkov <asavkov@redhat.com>
symtab_read() would previously skip entries with blank names resulting
in some of important entries being skipped. For instance vmlinux file
has an STT_FILE entry at the end with a blank name that contains global
offset table. Because it was skipped all of the global entries from this
table were considered a part of previous processed file resulting in
create-diff-object failing in find_local_syms().
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Nothing critical, but find_special_section_data_ppc64le() could run
longer than needed: the exit condition was not met after all the values
had been found.
Fixes: 77f8fd09 "kpatch-build: ppc64le - Add special section support"
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
GCC puts the constant variable requiring relocation into .data.rel. or
.data.rel.ro depending upon the bind type of the symbol. Extend
is_bundledable() to check these .data sections too.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
- Future releases of RHEL / CentOS will provide the yumdownloader
program with the 'dnf-utils' package (not 'yum-utils'). Instead of
looking to see that the package is installed, just look for the
program itself.
- RHEL / CentOS 8 kernel release names (as returned by 'uname -r') may
not match the SRPM buildroot release subdirectory name. Relax the
wildcard when moving this directory to $SRCDIR.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
4.18 adds -mcount-record to KBUILD_FLAGS when supported by the compiler.
This results in most of kpatch_create_mcount_sections()'s work being
already done, so we can at least skip the last part of it that updates
the first instruction in patched functions.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
In Ubuntu 18.04 LTS (but not in 16.04 LTS), the "linux" source package
no longer builds the "linux-image-*" binary kernel image packages
directly, but instead, it produces the "linux-image-unsigned-*" binary
packages, and the "linux-signed" source package then produces the
(signed) "linux-image-*" binary packages from the unsigned binaries.
This means that querying the target kernel's linux-image-* package for
its source package will yield a source package that is just a wrapper,
and does not actually contain the kernel source code.
Deal with this by removing the "-signed" substring from the kernel
source package name if it is present. This makes kpatch-build work
on Ubuntu 18.04.
Signed-off-by: Lennert Buytenhek <buytenh@wantstofly.org>
Commit d86c1113cc ("kpatch-build: less aggressive clean_cache()")
broke clean_cache(). Instead of expanding the wildcard, it tries to
delete a file named '*'.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
According to gcc8's man pages gcc can put functions into .text.unlikely
or .text.hot subfunctions during optimization. Add ".text.hot" to the
list of bundleable functions in is_bundleable().
Signed-off-by: Artem Savkov <asavkov@redhat.com>
gcc8 can place functions to .text.unlikely and .text.hot subsections
during optimizations. Allow symbols to change subsections instead of
failing.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Add a function that would detect parent/child symbol relations. So far
it only supports .cold.* symbols as children.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
kpatch-build detects RHEL-ALT kernel support by looking for a ".el7a."
substring in the kernel release string. Look for that substring in the
unchanged $ARCHVERSION instead of $KVER, which may not have the
trailing '.' character that our regex expects.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Source RPMs don't have an architecture associated with them, so to avoid
confusion, drop that part of the kernel release string when calling
yumdownloader.
Fixes#887.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Some of the provisioned machines I sometimes use don't have enough
diskspace for a full kpatch-patch build in home partition. I usually
solve this by symlinking .kpatch(and .ccache) dirs to a different
partition, however this only works with -s option because of
clean_cache().
clean_cache() currently removes .kpatch directory completely, recreating
it from scratch, change it to only remove the contents of the directory
instead.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Building with GCC 7.3.0 on Debian sid fails with the following error:
gcc -g -O2 -fdebug-prefix-map=/build/kpatch-0.6.0=. -fstack-protector-strong -Wformat -Werror=format-security -MMD -MP -I../kmod/patch -Iinsn -Wall -Wsign-compare -g -Werror -Wdate-time -D_FORTIFY_SOURCE=2 -c -c
create-diff-object.c: In function 'kpatch_compare_correlated_rela_section':
create-diff-object.c:316:20: error: 'toc_data1' may be used uninitialized in this function [-Werror=maybe-uninitialized]
return toc_data1 == toc_data2;
~~~~~~~~~~^~~~~~~~~~~~
create-diff-object.c:256:16: note: 'toc_data1' was declared here
unsigned long toc_data1, toc_data2;
^~~~~~~~~
cc1: all warnings being treated as errors
This is a false positive as the code only compares those two values
after initializing them. But lets keep GCC happy.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
If $AWK_OPTIONS are blank gawk would treat "" as a blank script
resulting in none of the special struct being detected.
Fixes: 1330dcc "create-diff-object: add ORC section support"
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Convert magic exit status values into a common enum for clarity.
Suggested-by: Artem Savkov <asavkov@redhat.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
None of the lib/* file are built with fentry calls, so we can't patch
them. Add these files to the list that kpatch-gcc skips.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Add a little more context ("in the vmlinux symbol table") to the
symbol-not-found message in find_local_syms().
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Change the "FILE symbol not found in base. Stripped?" (fatal) error
message into a warning. These crop up whenever a change is made to an
assembly file.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Finally add support for processing the ORC unwinder sections.
The ORC unwinder sections are more special than the other special
sections, so they need their own dedicated function to process them,
though the code is similar to kpatch_regenerate_special_sections().
BTW, upstream livepatch still doesn't support the ORC unwinder. That
change will be coming soon (probably Linux 4.19).
Fixes#785.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Since the codeset supports just the 64 bit variant, lets move
to __powerpc64__ and use it. I checked the ABI doc as well
and the kernel/gcc.
Signed-off-by: Balbir singh <bsingharora@gmail.com>
As discussed in #848, there is no known reason to do "make mrproper" on
every build. It seems to be an artifact from previous iterations (we
used to use 'O=' to build the kernel in a separate object tree.
It has many downsides:
- massive performance degradation
- breaks the '-t' option
- prevents the user from manually saving/restoring ~/.kpatch
Only do it where it's really needed, which is after first extracting the
source from an RPM.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
"make mrproper" combined with the '-t' flag is dangerous, as it results
in the Module.symvers file getting truncated, which causes
create-diff-object to create some funky dynrelas. Detect this condition
in kpatch-build and error out.
We will hopefully also be removing "make mrproper" soon, which will make
'-t' even more useful.
Fixes#589.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
In recent versions of Fedora, when building from a source RPM,
kpatch-build fails because it can't find the .config file. Get the file
from the canonical location: the configs subdirectory.
This also works with older versions of Fedora and RHEL, and ensures we
always have the right config file for the arch we're building for.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Before we were adding the undefined symbols to the lookup table, but we
were skipping them by setting the sym.skip flag.
With 3aa5abb807 ("kpatch-build: use symbol table instead of kobject"),
the skip flag was removed but the undefined symbol check was removed
with it.
The skip flag can remain gone. Instead of adding undefined symbols to
the table and skipping them when iterating the table, just don't add
them to start with.
Also make the sscanf conditional lines identical, to ease maintenance.
Fixes#869.
Fixes: 3aa5abb807 ("kpatch-build: use symbol table instead of kobject")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Deal with a few RHEL kernel-alt quirks for ppc64le:
- The RPM and spec names are "kernel-alt".
- 7.6 ALT is based on 4.14 but it doesn't have the 'immediate' flag.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
gcc8 introduces ".cold." optimization symbols that have arbitrary
trainling numbers in their names just like ".isra." and others.
Add ".cold." to a condition in kpatch_rename_mangled_functions()
Signed-off-by: Artem Savkov <asavkov@redhat.com>
plugin compilation fails on GCC 8:
In file included from gcc-plugins/gcc-common.h:100,
from gcc-plugins/ppc64le-plugin.c:1:
/usr/lib/gcc/powerpc64le-linux-gnu/8/plugin/include/attribs.h: In function ‘tree_node* canonicalize_attr_name(tree)’:
/usr/lib/gcc/powerpc64le-linux-gnu/8/plugin/include/attribs.h:118:11: error: ‘get_identifier_with_length’ was not declared in this scope
return get_identifier_with_length (s + 2, l - 4);
^~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/powerpc64le-linux-gnu/8/plugin/include/attribs.h:118:11: note: suggested alternative: ‘get_attr_min_length’
return get_identifier_with_length (s + 2, l - 4);
^~~~~~~~~~~~~~~~~~~~~~~~~~
get_attr_min_length
Makefile:34: recipe for target 'gcc-plugins/ppc64le-plugin.so' failed
get_identifier_with_length() is defined under stringpool.h, include this
header file for GCC 8, before including attribs.h
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
When I made a patch to the nfsd module on a ppc64le system with a RHEL 7
based kernel, livepatch prevented the target module from loading with:
livepatch: symbol '.TOC.' not found in symbol table
References to this symbol are treated specially by the kernel module
loader, so references to it should never be converted to dynrelas.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
While building a gcc-consprop patch from integration tests gcc8 would place a
__timekeeping_inject_sleeptime.constprop.18.cold.27 symbol into
.text.unlikely.__timekeeping_inject_sleeptime.constprop.18 section. Because
section name doesn't have the '.cold.27' suffix this symbol fails
is_bundleable() check while still being bundleable and later exits early in
kpatch_rename_mangled_functions() without renaming the corresponding patched
function. All of this results in a create-diff-object errror:
ERROR: timekeeping.o: symbol changed sections: __timekeeping_inject_sleeptime.constprop.18.cold.27
/home/asavkov/dev/kpatch/kpatch-build/create-diff-object: unreconcilable difference
Fix by ignoring .cold.* name suffix in is_bundleable() for.text.unlikely
sections.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
From Oracle's Linker and Libraries Guide [1]:
"The symbols in a symbol table are written in the following order ...
The global symbols immediately follow the local symbols in the symbol
table. The first global symbol is identified by the symbol table sh_info
value. Local and global symbols are always kept separate in this manner,
and cannot be mixed together."
[1] https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-79797/index.htmlFixes#854.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
create-diff-object doesn't really need the full kernel object file as
input. All it requires is a symbol table. Switch to using "eu-readelf -s"'s
output instead of object files. This will enable us to cover more cases
in unit tests.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Artem Savkov <asavkov@redhat.com>
