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.
Make the kpatch-patch-hook.c function and variable names consistent by
prefixing them all with 'patch_'. This makes it easier to distinguish
the patch hook sections from the patched sections when looking at the
ELF section data.
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.
This adds support for shadow variables, which allow you to add new
"shadow" fields to existing data structures.
To allow patches to call the shadow functions in the core module, I had
to add a funky hack to use --warn-unresolved-symbols when linking, which
allows the patched vmlinux to link with the missing symbols. I also
added greps to the log file to ensure that only unresolved symbols to
kpatch_shadow_* are allowed. We can remove this hack once the core
module gets moved into the kernel tree.
Fixes#314.
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.
Fix incorrect old_offsets for loadable modules during sysfs
initialization in patch_init.
sysfs will be initialized on patch module init regardless of whether
or not the module is loaded. func_old_addr_show() will read from func->old_addr,
which is initially set to 0; it'll be eventually filled in by the core module.
Use this instead of calling printk to avoid unwanted compiler
optimizations which cause kpatch-build errors.
The printk function is annotated with the __cold attribute, which tells
gcc that the function is unlikely to be called. A side effect of this
is that code paths containing calls to printk might also be marked cold,
leading to other functions called in those code paths getting moved into
.text.unlikely or being uninlined.
This macro places printk in its own code path so as not to make the
surrounding code path cold.
I have a related integration test to add, but right now it's broken
because we don't yet properly support the __verbose special section.
That'll be another PR.
Fixes#296.
On RHEL I'm seeing issues with putting the core module in the "extra"
path. On the next depmod run, it gets added to modules.dep, and on a
subsequent kpatch install I see the following errors:
/usr/lib/dracut/modules.d/50drm/module-setup.sh: line 26: /lib/modules/3.10.0-123.4.4.el7.x86_64//weak-updates/kpatch/kpatch.ko: No such file or directory
/usr/lib/dracut/modules.d/90kernel-modules/module-setup.sh: line 14: /lib/modules/3.10.0-123.4.4.el7.x86_64//weak-updates/kpatch/kpatch.ko: No such file or directory
modinfo: ERROR: Module /lib/modules/3.10.0-123.4.4.el7.x86_64/weak-updates/kpatch/kpatch.ko not found.
Until the core module gets merged into Linux, I think we can put it in
/usr/lib/kpatch, which is also where the patch modules are going to be
delivered in the RHEL RPM.
Making sure the other options still work with the kpatch utility for
now, so as to keep backwards compatibility between a newer kpatch
utility and older core modules. We can break this compatibility for
kpatch 0.2.0.
This macro is for ignoring sections that may change as a side effect of
another change or might be a non-bundlable section; that is one that
does not honor -ffunction-section and create a one-to-one relation from
function symbol to section.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This commit adds the KPATCH_IGNORE_FUNC() macro for ignoring functions
that may change as a side effect of a change in another function. The
WARN class of macros, for example, embed the line number in an
instruction, which will cause the function to be detected as changed
when, in fact, there has been no functional change.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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.
WARN macros are problematic because they embed the line number in an
instruction. As a result, when a function is changed higher in a file,
the line numbers for any WARN calls below that function in the file can
result in unnecessarily changed functions.
These macros allow a patch author to hard code the line numbers in WARN
macros to prevent functions from otherwise changing and getting pulled
into a patch module unnecessarily.
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.
Currently, when removing a patch module, the ftrace buffer gets flooded
with traces. This happens because we're clearing the ftrace ops filter
before unregistering the ops, which creates a small window where all
functions are being traced.
We should be doing the unregistering in the reverse order in which we
registered, meaning ops should be unregistered and _then_ the filter
should be cleared.
This commit enables the ability to create user-defined hooks as part of
the normal code patch that can do preparatory work for the application
of the patch. This work could include, but is not limited to, changing
data structure semantics.
The user may define a new function as part of the patch and mark it as a
load-time or unload-time hook with the kpatch_load_hook() and
kpatch_unload_hook() macros. These macros are in an include file that
gets copied into the source tree at include/linux/kpatch-hooks.h at
patch build time. The signature for both hooks is "int kpatch_unload_hook(void)".
For now, the return code is ignored. The hooks may not fail. They also
run in stop_machine() context and may not sleep. These hooks, more or
less, must follow all the rules of interrupt context code.
The integration test suite was intermittently giving the following
error:
[192685.907072] kpatch: write to 0xffffffffa082bffe failed for symbol call_netdevice_notifiers_info
The error was caused by a write across a page boundary without first
making the second page read/write.
Right now, if there is a failure in patch_make_dynrelas_list(),
patch_free_objects() is called twice; once in the error section of
patch_make_dynrelas_list() and again in the err_objects section of
patch_init().
This fixes this and cleans up the error handling a bit.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
When patching module A, if one of the new function's relas reference a
symbol in module B, we currently just leave it as a normal rela. But if
module B hasn't been loaded yet, the patch module will fail to load due
to the rela's reference to an undefined symbol.
The fix is to convert these relas to dynrelas, which can be resolved
later in the module notifier when A is loaded.
Also added support for the R_X86_64_NONE relocation type, needed for
dynrelas which reference __fentry__.
The recent module patching code has exposed some problems with our data
structures. We currently patch the funcs and dynrelas individually,
which is kind of scary now that different objects can be patched at
different times. Instead it's cleaner and safer to group them by
patched object.
This patch implements per-object patching and relocations by refactoring
the interfaces:
- Completely separate the create-diff-object <-> patch module interface
from the patch module <-> core module interface. create-diff-object
will include "kpatch-patch.h" but not "kpatch.h". Thus,
create-diff-object has no knowledge about the core module's
interfaces, and the core module has no knowledge about the patch
module's special sections.
- Newly added kpatch-patch.h defines the format of the patch module
special sections. It's used by create-diff-object to create the
special sections and used by the patch module to read them.
- kpatch.h still defines the core module interfaces. Each kpatch_module
has a list of kpatch_objects for each module object to be patched.
Each kpatch_object has a list of kpatch_funcs and a list of
kpatch_dynrelas. The patch module creates these lists when populating
kpatch_module.
This way of structuring the data allows us to patch funcs and dynrelas
on a per patched object basis, which will allow us to catch more error
scenarios and make the code easier to manage going forward. It also
allows the use of much more common code between kpatch_register() and
kpatch_module_notify().
This allows a patch module to contain patched functions for modules
which haven't been loaded yet. If/when the module is loaded later, it
will be patched from the module notifier function.
In the replace case, stop calling module_put on a patch module before
we're potentially done with it.
This will also be needed for future module patching if we want to
properly replace a patch module which only patches a future loaded
module (that's a mouthful).
Fixes#165.
Create a list of registered patch modules, which will be used in the
module notifier to patch future loaded modules. It will also be used to
fix the kpatch replace race condition where it calls module_put too
early.
The kpatch_internal struct is a good idea, in that it documents which
parts of kpatch_module shouldn't be used by the patch module. But it
creates extra code and will require more extra code if we want to keep a
list of kpmods, which is needed to create a module notifier for module
patching of future loaded modules.
Embedding the private data directly in the public struct allows the code
to be simpler: no extra kmallocs/kfrees, no need to store pointers
between the public and private structs. I think the simpler code is
worth the tradeoff (exposing implementation detail). Kernel code
usually doesn't bother with hiding a internal struct data from other
kernel code anyway. For example, see ftrace_ops or struct kprobe.
The private fields are documented with a "private" comment.
Move all the ftrace filtering and registering logic into a couple of new
helper functions. Change kpatch_num_registered to kpatch_num_patched,
which now tracks the number of patched functions rather than the number
of patch modules.
This simplifies the code a bit and will also prevent a future loaded
module scenario where ftrace_ops can be registered with an empty filter,
resulting in _all_ kernel functions getting registered with ftrace.
Use single quotes when printing the name of a patch module, rather than
double quotes. This is more consistent with other printk messages, and
looks better too!
If kpatch_ftrace_add_func fails, num_funcs will be one less than what it
needs to be for kpatch_put_modules to work properly. Instead give it
the full array size, and it can figure out which modules to put based on
whether func->mod is nonzero.
This commit adds support for patching modules. If a patch or dynrela is
determined to be for a kernel module, the old_offset/src is not used and
the symbol location is looked up using kallsyms. The module being
patched is also references to keep if from disappearing from underneath
us.
This commit introduces early and limited support. The kernel module to
be patched must already be loaded or the patch module will not apply.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This commit adds module patching support to create-diff-object by:
1) generalizing the vmlinux CLI parameter
2) adding the kernel object name to each patch and dynrela
3) adding slightly different logic for vmlinux/module in the dynrela
creation
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Make old addresses relative to the start address of the relocatable
kernel or module.
This commit has no functional effect; it just prepares the code for
future acceptance of the module patching support.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Ensure that dynrela destination addresses are within the patch module's
memory.
Also, use the module address ranges to check whether set_memory_rw() is
needed.
The -fdata-sections gcc flag doesn't work with objects in the
.data..percpu section. Any function which uses a percpu variable
references this section, causing the section to get incorrectly included
in the patch module.
Manually convert these section references to object symbol references so
that the needed symbol can be found in vmlinux.
Also, the core module symbol verification code will fail when looking up
a percpu variable, because sprint_symbol doesn't think a percpu address
is a valid kernel address. So rewrite the symbol verification code to
use kallsyms_on_each_symbol() instead. It's not ideal performance-wise:
it seems to cost about 1ms per symbol lookup. I think that's acceptable
for now. In the future we may want to try to get a better upstream
kallsyms interface.
This feature is implemented as:
```
[root@localhost kpatch]# insmod ./kpatch-meminfo.ko
[root@localhost kpatch]# ls /sys/kernel/kpatch/patches/kpatch_meminfo/functions/meminfo_proc_show/
new_addr old_addr
[root@localhost kpatch]# cat /sys/kernel/kpatch/patches/kpatch_meminfo/functions/meminfo_proc_show/new_addr
0xffffffffa05211e0
[root@localhost kpatch]# cat /sys/kernel/kpatch/patches/kpatch_meminfo/functions/meminfo_proc_show/old_addr
0xffffffff8125d0e0
```
The patch module init function will allocate and init kpatch_func_obj with
customized kobj_type func_ktype. The attribute new_addr and old_addr of
kpatch_func_obj is attached to this func_ktype, so that these files could
be created by kobject_add automatically.
Signed-off-by: Jincheng Miao <jincheng.miao@gmail.com>
This commit introduces functionality to verify the location of symbols
used in both the patch and dynrelas sections. It adds significant
protection from mismatches between the base and running kernels.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This adds dynamic linking support for the patch modules. It is the
first step toward supporting patching module code and relocatable
kernels.
Rela entries that reference non-included local and non-exported global
symbols are converted to "dynrelas". These dynrelas are relocations
that are done by the core module, not the kernel module linker. This
allows the core module to apply offsets to the base addresses found
in the base vmlinux or module.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Conflicts:
kpatch-build/kpatch-build
We merged PR #186 a little too hastily. It seg faults with the new
parainstructions-section.patch in the integration test suite. Reverting
it for now until we get it figured out.
This reverts commit e1177e3a03.
This reverts commit 880e271841.
This reverts commit 2de5f6cbfb.
This reverts commit 38b7ac74ad.
This reverts commit 108cd9f95e.
This adds dynamic linking support for the patch modules. It is the
first step toward supporting patching module code and relocatable
kernels.
Rela entries that reference non-included local and non-exported global
symbols are converted to "dynrelas". These dynrelas are relocations
that are done by the core module, not the kernel module linker. This
allows the core module to apply offsets to the base addresses found
in the base vmlinux or module.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Fixes the following warning:
kpatch-patch-hook.c:71:2: warning: initialization from incompatible pointer type [enabled by default]
__ATTR(enabled, 0644, patch_enabled_show, patch_enabled_store);
^
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Make kpatch_funs truly internal by:
Defining it in core.c
Adding a struct kpatch_internal, declared in kpatch.h and defined in
core.c, that contains per patch module internal data.
Adding an "internal" field to struct kpatch_modules.
Allocating internal and funcs data in core.c, not in the patch module,
since the patch module has no knowledge of kpatch_func anymore.
Adding a "patch" field to kpatch_func that points directly to the
kpatch_patch provided by the module (rather than a field-by-field copy)
Signed-off-by: Seth Jennings <sjenning@redhat.com>
The error likes:
make -C /lib/modules/3.11.0-12-generic/build M=/home/ryan/kpatch/kmod/core kpatch.ko
make[3]: Entering directory `/usr/src/linux-headers-3.11.0-12-generic'
CC [M] /home/ryan/kpatch/kmod/core/core.o
/home/ryan/kpatch/kmod/core/core.c:42:32: fatal error: linux/preempt_mask.h: No such file or directory
#include <linux/preempt_mask.h>
I feel sorry to introduce this problem from my laster commit 6c2d6444.
Some old kernel doesn't have header file preempt_mask.h, and the safe
way is using hardirq.h to find in_nmi().
Signed-off-by: Jincheng Miao <jincheng.miao@gmail.com>
It's probably cleaner to just return -EINVAL if the kpmod isn't enabled,
instead of warning and continuing, which would be dangerous.
I think the reason I had it WARN before is because this condition
shouldn't be possible given the source for the patch module. But the
core module can't necessarily assume that it's our trustworthy patch
module code on the other side.
Allow the user to atomically replace all existing modules with a new
"kpatch replace" command. This provides a safe way to do atomic
upgrades for cumulative patch module updates.
