we remove the pre_patch_callback/post_unpatch_callback from the
stop_machine context. If a schedule/sleep happend in callbacks while the
process to be scheuded later will send IPI, because all interrupts
are disabled, the machine will trap into a deadlock in such situation.
So we remove the pre_patch_callback and post_unpatch_callback from
the stop_machine to avoid such situation. On the other hand, to avoid
the race between the patched code and post-patch/pre-unpatch callbacks when
run in parallel, we didn't remove the post_patch_callback and
pre_unpatch_callback from stop_machine.
User stettberger noticed that the kpatch support module does not
apply the addend for R_X86_64_64 in kpatch_write_relocations().
The AMD64 ABI draft doc [1], Table 4.10: Relocation Types lists that
relocation type as:
Name Value Field Calculation
R_X86_64_64 1 word64 S + A
where:
S : Represents the value of the symbol whose index resides in the
relocation entry.
A : Represents the addend used to compute the value of the relocatable
field.
[1] http://refspecs.linuxfoundation.org/elf/x86_64-abi-0.99.pdfFixes: #1093
Reported-by: Christian Dietrich <stettberger@dokucode.de>
Signed-off-by: Joe Lawrence <joe.lawrence@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>
kpatch_verify_activeness_safety() calls kpatch_backtrace_address_verify()
for each address in the call traces of the processes.
Among other things, kpatch_backtrace_address_verify() searches the whole
set of functions for the ones being replaced (func->op == KPATCH_OP_UNPATCH).
This is a waste of time when the patch is loaded or unloaded rather than
replaced. Let us do the searching only if patch replacement is in
progress.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
If atomic replacement is used for the old-style patches (the patches
that depend on kpatch.ko), the kernel might crash if the new patch
changes a smaller set of functions than the patch being replaced.
kpatch_apply_patch() does check if the functions from the patch to be
replaced are currently running. However, the functions are removed from
'kpatch_func_hash' in kpatch_register() only after stop_machine() and
kpatch_apply_patch() have finished:
ret = stop_machine(kpatch_apply_patch, kpmod, NULL);
/*
* For the replace case, remove any obsolete funcs from the hash and
* the ftrace filter, and disable the owning patch module so that it
* can be removed.
*/
if (!ret && replace) {
struct kpatch_module *kpmod2, *safe;
hash_for_each_rcu(kpatch_func_hash, i, func, node) {
if (func->op != KPATCH_OP_UNPATCH)
continue;
if (func->force)
force = 1;
hash_del_rcu(&func->node);
WARN_ON(kpatch_ftrace_remove_func(func->old_addr));
}
<...>
As a result, the kernel may end up with an inconsistent set of patched
functions. Some of the functions from the replaced patch could
still be running, while some would be already reverted to the original
ones.
I observed kernel crashes in such situations when I was trying to
replace a patch with a new one without a faulty fix.
Let us remove the replaced patched functions from 'kpatch_func_hash'
in kpatch_apply_patch() to avoid such issues.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Make kpatch_apply_patch() aware of whether the patch should replace other
patches.
This will be used by subsequent fixes.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
The module notifier currently only handles newly loaded modules in the
MODULE_STATE_COMING state. If target modules need to be unloaded, the
any kpatch module that patches it must first be disabled, releasing
module references held against the target module. When the kpatch
modules are disabled, the target module is unpatched and the kpatch
core's data structures updated accordingly.
If a loading module happens to fail its init routine (missing hardware
for example), that module will not complete loading. The kpatch core
doesn't properly account for this "phantom" target module, so when the
kpatch patch module is removed, it spews out an ugly warning when
attempting to remove a non-existing ftrace filter on the target module.
Register an additional module notifier (first in the list) to handle the
MODULE_STATE_GOING case. This handler needs to do the inverse of the
MODULE_STATE_COMING handler.
Fixes#699.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Fixes sparse warnings:
kmod/core/core.c:142:20: warning: symbol 'trace' was not declared. Should it be static?
livepatch-patch-hook.c:73:18: warning: symbol 'lpatch' was not declared. Should it be static?
Signed-off-by: Joe Lawrence <joe.lawrence@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>
Starting with kernel 4.11, CONFIG_DEBUG_SET_MODULE_RONX has been
replaced with CONFIG_ARCH_HAS_SET_MEMORY. This fixes the following
error:
kpatch: write to 0xffffffffc0d7650e failed for symbol copy_mnt_ns
Fixes#721.
When the core module loops through an object's list of dynrelas, it
determines whether or not the target location of the dynrela is in a
read-only region of the patch module. If it is, the readonly flag is set to
1 and it calls set_memory_{rw,ro} before and after the probe_kernel_write()
operation. This flag gets set once, and never gets reset for subsequent
iterations. Therefore, if a target happens to be in a RW section of the
patch module, and readonly = 1 had been set before, we may unintentionally
set a normally RW page to RO. Fix this by setting the readonly flag with
each iteration of the loop.
Fixes#681.
Restructure kpatch's sysfs interface and mirror the sysfs tree after
livepatch's sysfs layout. With the current sysfs layout, we cannot
distinguish which object a function belongs to, and we cannot tell which
modules/objects are patched. Therefore, restructure the kpatch sysfs tree
such that module/object information is available. With the new layout, each
patched object has its own directory, with each function being a
subdirectory of its object.
Implement this by embedding a kobject struct within the kpatch_module,
kpatch_func, and kpatch_object structs and supplying their ktypes and
kobject release methods.
Before:
/sys/kernel/kpatch
└── patches
└── <patch_module>
├── checksum
├── enabled
└── functions
├── <function> # from <object1>
│ ├── new_addr
│ └── old_addr
├── <function> # from <object2>
│ ├── new_addr
│ └── old_addr
└─── <function> # from <object3>
├── new_addr
└── old_addr
After:
/sys/kernel/kpatch
└── <patch_module>
├── <object1>
│ └── <function,sympos>
│ ├── new_addr
│ └── old_addr
├── <object2>
│ └── <function,sympos>
│ ├── new_addr
│ └── old_addr
├── checksum
├── enabled
└── <object3>
└── <function,sympos>
├── new_addr
└── old_addr
Upstream 2992ef29ae01 "livepatch/module: make TAINT_LIVEPATCH module-specific"
added a TAINT_LIVEPATCH flag to the module-specific taint flags. This
commit is v4.9+ and the modules taint field is an unsigned int.
Upstream 7fd8329ba502 "taint/module: Clean up global and module taint
flags handling" modified the modules taint field to be an unsigned long.
This commit is v4.10+.
Adjust the module tainting code in kpatch_register() to consider v4.9
kernels as well as v4.10 (and any distro-specific behavior).
Fixes: #666.
The dump_trace interface was deprecated in v4.9: instead of adding yet
another kernel-specific code block to kpatch's stack safety checks, use
save_stack_trace_tsk. It's relatively simple (no callbacks like
dump_trace), arch-independent, and its interface is stable across kernel
releases.
Fixes: #623.
Previous commit "kmod: let kernel apply TAINT_LIVEPATCH" modified the
kpatch patch module to set the "livepatch" module info. This breaks
module loading for kernel config CONFIG_LIVEPATCH=n
kpatch_kmalloc: module is marked as livepatch module, but livepatch support is disabled
kpatch modules can still use TAINT_LIVEPATCH as a per-module taint flag,
but only if it is set after the module loads.
Fixes: 660.
Upstream commit 2992ef29ae01 ("livepatch/module: make TAINT_LIVEPATCH
module-specific") v4.9+ modified the kernel to add the TAINT_LIVEPATCH
flag on module load. To support this feature, add the "livepatch"
module info in the {k,live}patch modules and drop the add_taint() in the
core module.
Fixes smatch warning:
kmod/core/core.c:64:1: warning: symbol 'kpmod_list' was not declared. Should it be static?
Fixes sparse warnings:
kmod/core/core.c:680 kpatch_write_relocations() warn: inconsistent indenting
kmod/core/core.c:750 kpatch_write_relocations() warn: inconsistent indenting
Some features were backported into the 4.4 kernel which change the fields
of the livepatch structures. Ensure we can work with either v4.5 or greater,
or Ubuntu 4.4.0-7 or greater.
If an activeness safety check fails for kernels newer than 4.6, the
error is silently ignored because the newer version of
kpatch_backtrace_address_verify() doesn't set args.ret on error.
It would be an easy fix to just set args->ret on error, but I think a
better approach is just to combine the two versions of the function into
a single function with the use of a little macro trickery.
Backport the symbol lookup and checking code from upstream livepatch
code that relies on a symbol position enumeration rather than a fixed
memory address.
Fixes#617.
ftrace only allows a single user of this flag to register for a given
function. This prevents kpatch conflicts with kprobes handlers which
also might want to change regs->ip for a function.
We should have done this a few years ago. Better late than never...
Upstream commit 568b329a "perf: generalize perf_callchain" modified the
return type (void -> int) of the address member of struct stacktrace_ops.
Use the void function if the kernel version is < 4.6 or return an int
otherwise.
When a patch module is loaded, the kernel facilities like alternatives
and paravirt may alter some of its instructions. This happens before
Kpatch core module is notified and tries to apply dynrelas to it. If an
instruction to apply a dynrela to has already been changed by these
facilities, an incorrect instruction might be written as a result.
The core module now detects such conditions and does not apply dynrela
to the changed instructions.
Suggested by Josh Poimboeuf in the discussion of
https://github.com/dynup/kpatch/issues/580.
Changes in v.2:
* Used pr_notice to give more emphasis to the messages.
* Added an explanation message.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Commit 7523e4dc5057 upstream ("module: use a structure to encapsulate
layout") uses a new field to access module memory. Account for this change
and ensure backwards compatibility with kernel versions < 4.5
Unload of kpatch module (and kpatch_shadow_hash table) before
all shadow variables free requests are processed can lead to
kernel crash.
Add rcu_barrier() to kpatch_exit() to wait for all outstanding
RCU callbacks to complete.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
When patching a kernel module, if we can't find a needed dynrela symbol,
we currently assume it's exported. However, it's also possible that
it's provided by another .o in the patch module. Add support for that.
Fixes#445.
Fix the object unlink error handling so that each function cleans up
after itself properly.
Also use find_symbol() instead of __symbol_get() to make cleanup easier.
When patching a module we don't need a reference to each symbol, since
we already have done a try_module_get() on the module.
Fixes#392.
In order to safely re-enable patch modules, add a special
.kpatch.checksum section containing an md5sum of a patch module's
contents. The contents of this section are exported to sysfs via
patch_init and double checked when kpatch load finds that a module of
the same name is already loaded.
To reduce redundancy, remove/change the old_offset fields in the
kpatch_func and kpatch_patch_func structs to just old_addr. Since
old_offset is being used as a placeholder for old_addr, might as well
consolidate it to just one variable.
When patching a module, I ran into a "can't set ftrace filter at
address" error. The root cause was due to the fact that
mod->module_core + old_offset is apparently not a reliable way to
determine the function's address.
Instead, just get the address from kallsyms like we do for module
dynrelas.
I found a bad bug:
- Module A is loaded, and registers function foo() with
KPATCH_FORCE_UNSAFE.
- Module A is unloaded. The new version of foo() is on the backtrace of
a task, but the core module ignores it because of the force flag, so
the unloading succeeds.
- The task returns to the new version of foo() which no longer exists.
- BOOM.
The only way I can think of to prevent this scenario is to prevent
forced modules from being unloaded (but still allow them to be
disabled).
An annoying side effect of this approach is that forced modules stay
loaded and in memory forever. And that after "kpatch unload" of a
forced module, you can't ever load it again because the previous
instance of it is still loaded (but permanently disabled).
This is ugly but I can't really think of a better way to handle it. If
necessary we could create a workqueue and periodically check to see if
we can safely call module_put() so that the module could be eventually
removed.
Some functions in the kernel are always on the stack of some thread
in the system. Attempts to patch these function will currently always
fail the activeness safety check.
However, through human inspection, it can be determined that, for a
particular function, consistency is maintained even if the old and new
versions of the function run concurrently.
This commit introduces a KPATCH_FORCE_UNSAFE() macro to define patched
functions that such be exempted from the activeness safety check.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Steven Rostedt recommended to return "regs->ip + MCOUNT_INSN_SIZE",
which is what the function_graph tracer expects. This fixes
function_graph tracing for a patched function.
This change also means that the function tracer will only show the
patched function once (corresponding to a trace of the original
function) rather than twice. This is probably more in line with what a
user would expect.
zhenjiang.mzj
Joe Lawrence
Josh Poimboeuf
Evgenii Shatokhin
Artem Savkov
Joe Lawrence
Jessica Yu
Chris J Arges
Jan Stancek
Seth Jennings
Jessica Yu
Seth Jennings