ppc64le lacked kpatch_line_macro_change_only() implementation. Add one
based on x86_64 version and available unit tests.
Fixes: #843
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Update the `__LINE__` macro detection heuristics based on recent kernel
changes:
- __warn_printk(), used in the WARN macros, which put the line number in
%edx
- __might_fault(), used in the usercopy macros
- lockdep_rcu_suspicious(), used in the RCU_LOCKDEP_WARN macro
- printk(), used in the ASSERT_RTNL macro
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
With #755, we started using dynrelas for function pointers. However,
this behavior only makes sense for function pointers to existing
functions. For function pointers to *new* functions, just use a normal
rela.
The 'function-ptr-new' unit test is from the following patch:
https://github.com/dynup/kpatch/files/1927198/new-static-callback.patch.txtFixes#834.
Fixes: 495e619750 ("kpatch-build, x86: do not use the patched functions as callbacks directly")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Because $PLUGIN variable is a part of $TARGETS we are trying to install
gcc-plugins twice which is treated as an error by install command:
/usr/bin/install create-diff-object create-klp-module create-kpatch-module gcc-plugins/ppc64le-plugin.so kpatch-gcc gcc-plugins/ppc64le-plugin.so /usr/local/libexec/kpatch
/usr/bin/install: will not overwrite just-created '/usr/local/libexec/kpatch/ppc64le-plugin.so' with 'gcc-plugins/ppc64le-plugin.so'
Signed-off-by: Artem Savkov <asavkov@redhat.com>
- convert section/symbol indexes and rela->offset to unsigned int as I
couldn't find any way for them to become negative.
- cast a number of rela->addend comparisons to int (assuming an 64bit
system this should be enough)
- a number of simple for-loop counter conversions to the type it
compares against
Signed-off-by: Artem Savkov <asavkov@redhat.com>
If a patch failed a first time, kpatch-build is using the previous
cache directory on subsequent builds. The UBUNTU_KERNEL=1 variable is
not set in this case. Therefore, utsrelease.h is not updated correctly
and the appropriate structures are not used. Just check if distro is
Ubuntu and we didn't request our own specific directory.
Signed-off-by: Vincent Bernat <vincent@bernat.im>
Because of signedness difference kpatch_check_relocations() would trigger
an error on any negative addend.
Fix by casting Elf_Data->d_size from size_t to int.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Check that none of the relocations are out-of-range of their
corresponding sections before writing the output elf.
Fixes: #618
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Heavy lifting of reading .toc rela entries for rela symbols
referring to .toc + offset, can be simplified using toc_rela() in
rela_equal() and remove the #ifdery guarding PowerPC code.
This patch also trims the commentary related to PowerPC.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
On ppc64le, the static local variable correlation doesn't take into
account the .toc rela indirection for data references, meaning that it's
basically broken in many cases.
Fix it by making the code .toc-aware.
Fixes#793.
Reported-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Upstream 4.15 kernels provide support for pre and post (un)patch
callbacks, inspired by the kpatch load hooks. Add support for them
in the livepatch-patch-hook.
At the same time, convert the kpatch hooks to use the same API.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Cleanup some of the new bash script code to appease 'make check':
- kpatch: Use integer comparison for $MAX_LOAD_ATTEMPTS
- kpatch-build: Quote ${TEMPDIR} variable
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
kpatch_include_symbol() is confusing. Refactor it:
- Remove the "inclusion tree" debug messages. I never use them, and
they just help make the code more confusing and the debug output more
cluttered.
- Split it up into two functions: kpatch_include_symbol() and
kpatch_include_section(), so that kpatch_include_section() can be used
elsewhere.
- Call kpatch_include_section() from kpatch_include_standard_elements().
This covertly fixes#702, by also including the .rela.rodata section.
- Add a bunch of comments to clarify some of the trickier points.
Fixes#702.
Fixes#807.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
It was observed by Evgenii Shatokhin in PR#755, that when the RCU
callback was called on the patched function, from unloaded livepatch
module triggered a kernel crash.
This patch implements the approach on PowerPC outlined in PR#755.
With -mcmodel=large, like any other data, function pointers are also
loaded relative to the current TOC base and are populated as
relocation entries in .toc section. Every function passing a function
pointer as the argument need to load the function address through
.toc section + offset. Convert such .toc + offset relocation into
a dynamic rela, which resolves to original function address, during
module load.
Also move the comment related to nested function check, into
may_need_dynrela().
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Cc: Joe Lawrence <jdl1291@gmail.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
A kernel crash happened in __do_softirq() in very rare cases when the
binary patch created from mainline commit be82485fbcbb
("netlink: fix an use-after-free issue for nlk groups") was unloaded.
Investigation has shown that the kernel tried to execute an RCU
callback, deferred_put_nlk_sk(), defined in the patch module after the
module had been unloaded.
The callback was set by the patched variant of netlink_release() and
the address of the patched deferred_put_nlk_sk() was used, rather than
the address of the original function.
Similar problems occur with workqueue functions as well.
As suggested in
https://github.com/dynup/kpatch/pull/755#issuecomment-344135224,
create-diff-object was modified so that the addresses of the original
functions were used in such situations, at least for x86 systems. A
similar fix for PowerPC was added as well.
Changes in v4:
* '#ifdef __x86_64__' was removed. It is not actually needed right now
because the constants for relocation types are different on different
architectures.
Changes in v3:
* Minor refactoring and a comment explaining what this all is about.
Quite lengthy, but the dynrela-related code is really far from obvious.
Changes in v2:
* Handle the nested functions the same way as before, because they are
unlikely to be used as asynchronous callbacks. Example: cmp() in
bch_cache_show() from drivers/md/bcache/sysfs.c in the kernel 4.4.
As the nested functions are local to the functions they are defined in,
the compiler names them in a similar way to static locals: <name>.<number>.
Currently, we filter out all functions with '.' in their names. If there
are any asynchronous callbacks in the kernel that have a dot in their
names too, they could be handled in the future patches. It is unclear
though, if the callbacks with such names can appear in the kernel.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Don't die outright when encountering an 'undefined reference' error,
instead write those down and check if needed symbols are provided by the
resulting module or core kpatch module.
Fixes#783.
v2:
- make undefined_references and new_symbols unique before comparing
- remove leftover $KMOD_PATH from new_symbols readelf
Signed-off-by: Artem Savkov <asavkov@redhat.com>
for-next branch of kbuild repo contains a "kbuild: rename built-in.o to
built-in.a" which renames all built-in.o instances. Filter on both .o
and .a in kpatch-gcc/kpatch-build to be prepared for this change.
Fixes#800.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
With kernel commit b1fca27d384 ("kernel debug: support resetting
WARN*_ONCE") the *_ONCE warnings are placed .data.once section.
Including .data.once section is valid, so add an check in
kpatch_verify_patchability() while checking for included invalid
sections.
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Symbols with R_PPC64_REL24 relocation type are functions and it's
currently assumed that all functions are replaced with their respective
section symbols.
There are function whose reference are not straight forward section
symbol but section + offset. These function replacement should be
handled more like bundled sections. Remove the check, which imposes
the inital assumption.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
.toc section entries are mostly place holder for relocation entries,
specified in .rela.toc section. Sometimes, .toc section may have
constants as entries. These constants are not reference to any symbols,
but plain instructions mostly due to some arthimetics in the functions
referring them.
They are referred by the functions like normal .toc entries, these
entries can not be resolved to any symbols. This patch creates a list
of constants if available for .toc sections and compares them in
rela_equal() to ensure their is no mismatch in the generated constants
for original and patched .o files.
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
GCC 7.2.1 complains about the usage of the PPC64_LOCAL_ENTRY_OFFSET
macro:
create-diff-object.c: In function ‘is_gcc6_localentry_bundled_sym’:
create-diff-object.c:119:83: error: ‘<<’ in boolean context, did you mean ‘<’ ? [-Werror=int-in-bool-context]
(((1 << (((other) & STO_PPC64_LOCAL_MASK) >> STO_PPC64_LOCAL_BIT)) >> 2) << 2)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
create-diff-object.c:140:10: note: in expansion of macro ‘PPC64_LOCAL_ENTRY_OFFSET’
return (PPC64_LOCAL_ENTRY_OFFSET(sym->sym.st_other) &&
^~~~~~~~~~~~~~~~~~~~~~~~
Fix it by explicitly treating the macro as an integer instead of a bool.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
On ppc64le, building drivers/media/i2c/cx25840/cx25840.ko with GCC 4.85,
the plugin triggers the following errors:
{standard input}: Assembler messages:
{standard input}:1078: Error: operand out of range (0x0000000000008fd4 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:1288: Error: operand out of range (0x000000000000874c is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:3844: Error: operand out of range (0x00000000000080e8 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:4028: Error: operand out of range (0x00000000000082b4 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:4031: Error: operand out of range (0x00000000000080b4 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:6661: Error: operand out of range (0x000000000000841c is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:6664: Error: operand out of range (0x0000000000008214 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:6845: Error: operand out of range (0x00000000000089dc is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:6848: Error: operand out of range (0x00000000000087d4 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:7587: Error: operand out of range (0x0000000000008930 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:7590: Error: operand out of range (0x0000000000008728 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
{standard input}:14592: Error: operand out of range (0xffffffffffff79b8 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
make[1]: *** [drivers/media/i2c/cx25840/cx25840-core.o] Error 1
The problem is that the plugin nops were inserted too late. GCC laid
out the code with knowledge about the distance between branches, but
then the plugin came along after that and increased the branch distances
by inserting the nops.
Fix it by inserting the nops as early as possible. The 'vregs' pass is
where the insn codes get initialized, so run the plugin immediately
afterwards.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Sometimes git doesn't see that the patches have been reverted, if that
happens during ./scripts/setlocalversion call the resulting patch module
is built with a wrong vermagic because the tree is still considered
dirty.
Fix by moving git update-index call into remove_patches function so that
it is called every time the patches are reverted, not only on cleanup.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Found in the scope of https://github.com/dynup/kpatch/pull/755 but not
related to the main problem discussed there.
kpatch_create_patches_sections() and kpatch_create_intermediate_sections()
used 'hint' in error messages.
However, the string 'hint' refers to is owned by 'kelf_base' and is
freed before kpatch_create_*_sections() are called. As a result, if
these functions try to output errors and print 'hint',
create-diff-object will crash.
As suggested in the mentioned PR, 'hint' is actually no longer needed at
that stage, so I have removed it from kpatch_create_*_sections().
By specifying -d, --debug multiple times, the following additional
debug modes can be enabled:
-d -d: Writes everything that is written to the logfile also to
stdout.
-d -d -d: Same as '-d -d' plus sets 'xtrace' in kpatch-build.
-d -d -d -d: Same as '-d -d -d' plus sets 'xtrace' in kpatch-gcc.
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
Add a logger funcition that can be used to log to both stdout and the
logfile or only to the logfile. This is needed for subsequent patches
where we introduce an alternate debug mode.
Since we're piping to a logger now, we need to set 'pipefail' otherwise
the return status of such a pipeline is always 0 (the exit status of the
logger) and we won't catch any errors.
From the bash manpage:
The return status of a pipeline is the exit status of the last command,
unless the pipefail option is enabled
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
This is in response to an upstream discussion for the following patch:
https://lkml.kernel.org/r/1508217523-18885-1-git-send-email-kamalesh@linux.vnet.ibm.com
This should hopefully make it a lot easier for the ppc64le kernel module
code to support klp relocations.
The gcc-common.h and gcc-generate-rtl-pass.h header files are copied
from the upstream Linux source tree.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
When searching for 'Linux version ...' in vmlinux, stop after the first
match so that we don't keep reading a potentially huge file.
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
The current checks never fail, because the first grep in the pipeline
doesn't write anything to stdout.
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
This can be used for building a kpatch module for a non-running
kernel. Note that the correct kernel and debug packages still need
to be installed.
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
When creating .kpatch.relocations, there's no reason to convert the
relocation destinations to symbols. In fact, it's actively harmful
because it makes it harder for create-klp-module to deal with the GCC 6+
8-byte localentry gap.
This also fixes a regression which was introduced in 5888f316e6, which
broke ppc64le relocations.
Fixes#754.
Fixes: 5888f316e6 ("create-klp-module: support unbundled symbols")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
kpatch_relocation's 'dest' addend and 'offset' fields are redundant. In
fact, the 'offset' field isn't always accurate because it doesn't have a
relocation, so its value doesn't adjust when multiple .o files are
combined. Just use the 'dest' addend instead.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
