This reverts commit 5852ddb6a2.
The __jump_table section is more complex than the initial analysis
determined. The __jump_table has three relocs per entry that must
be pulled in together and one of the relocs is to symbols contained
in the __tracepoints section whose rela section references the
__tracepoint_strings section. So it's more complex and should just
fail rather than appear that it is being handled properly.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This is an attempt to both simplify and improve the correctness of the
NMI synchronization code.
There's a race in kpatch_ftrace_handler() between the kpatch_get_func()
and kpatch_finish_status() calls which could result in func being NULL.
The retry was supposed to fix this. However, this race would still be a
problem in the repatching case (if the function had already been
previously patched), in which case func would not be NULL, but could
instead point to the previously patched version of the function. In
this case it wouldn't retry and it would be possible for the previous
version of the function to run.
The fix is to use a memory barrier between accesses of the func hash and
the status variable, and then just call kpatch_get_func() *after*
accessing the status variable. For OP_PATCH, if status is SUCCESS, then
func is guaranteed to point to the new function. If status is FAILURE,
func might point to the new function, in which case we can use
get_prev_func to get the previous version of the function.
I also made some pretty big changes to try to simplify the design so
that there are less moving parts and so that it's hopefully easier to
understand. I moved the OP field into the kpatch_func struct. This
allows us to merge the two global state variables (status + op) into a
single global state variable (state), which helps make the code quite a
bit simpler. I turned it into a proper state machine and documented the
meaning of each state in the comments.
Moving the OP field to the kpatch_func struct also paves the way for an
upcoming pull request which will allow patch modules to be atomically
replaced ("kpatch load --replace <module>").
In kpatch_unregister(), if kpatch_remove_patch succeeds but one of the
subsequent ftrace unregistering calls fails, it returns an error and
fails to module_put() the patch module, even though the patch has been
removed. This causes the patch module to get stuck in a weird place
where its patch has been unregistered but the patch module can't ever be
removed.
These errors aren't serious and wouldn't cause any real problems if they
did somehow fail, so instead just WARN if they fail.
Almost a line-for-line copy/paste of the smp locks function. The only
differences are the section name, and an offset increment of 8 instead
of 4.
Fixes#157.
If a patch changes a single function which is in a special section that
we don't support, create-diff-object reports "no changed functions were
found". Give a clearer error message in that case, by checking
reachability errors before unchanged errors and by printing all
reachability errors errors instead of the first one it encounters.
Fixes#150.
At this point the module does build (i.e. kpatch-build is correct);
however, the addresses in the generated vmlinux don't match that
of the running kernel so the modules fail to load with an ftrace
registration error. So that is something to be investigated.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
During the test whether the patch applies, if it partially applies, the
patch utility returns an error but leaves the source tree in a partially
patched state. Use --dry-run instead.
No need to accumulate errors if the load or unload fails. Leaving the
testprog failure non-fatal so that the test will then call unload to
clean up after itself.
This is a basic integration test framework for kpatch, which tests
building, loading, and unloading patches, as well as any other related
custom tests.
The kpatch-test script looks for test input files in the
tests/integration directory. It expects certain file naming
conventions:
- foo.patch - patch that should build successfully
- bar-FAIL.patch - patch that should fail to build
- foo-LOADED.test - executable which tests whether the foo.patch module
is loaded. It will be used to test that loading/unloading the patch
module works as expected.
Any other *.test files will be executed after all the patch modules have
been built from the *.patch files. They can be used for more custom
tests above and beyond the simple loading and unloading tests.
I just have one test here, but many more to come eventually. I'm
constantly doing manual testing of patches and am planning on automating
them with this framework.
Currently the patch module calls kpatch_unregister in the patch module
exit path. If the activeness safety check fails in kpatch_unregister,
it's too late for the patch module to stop exiting, so all it can do is
panic.
Prevent this scenario by requiring the user to disable the patch module
via sysfs before allowing the module to be unloaded. The sysfs write
will fail if the activeness safety check fails. An rmmod will fail if
the patch is still enabled.
Also add support for this new unloading model in "kpatch unload".
Following in the same solution, regenerate [.rela].parainstructions
sections if table entries contain relocations that reference changed
functions (if any).
Fixes#135
Signed-off-by: Seth Jennings <sjenning@redhat.com>
The initial commit had a bug where the offset field of the
.rela.smp_locks entries was not updated to reflect the correct
offset in the truncated .smp_locks section.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This commit uses the same approach as the bug table support,
mangling the .smp_locks and .rela.smp_locks sections so that
they only contain entries for changed functions (if any).
Fixes#107
Signed-off-by: Seth Jennings <sjenning@redhat.com>
When compiling core.c, it may report error like:
"error: implicit declaration of function ‘in_nmi’"
Adding header file in_nmi defined could avoid this.
Signed-off-by: Jincheng Miao <jincheng.miao@gmail.com>
Put funcs, num_funcs, and mod in their own struct called kpatch_module.
This allows us to keep patch module specific variables in one place (and
we'll have more of these variables soon).
Support live patching on NMI handlers. This adds checks for
possible inconsistency of live patching on NMI handlers.
The inconsistency problem means that any concurrent execution
of old function and new function, which can lead unexpected results.
Current kpatch checks possible inconsistency problem with
stop_machine, which can cover only threads and normal interrupts.
However, beacuse NMI can not stop with it, stop_machine is not
enough for live patching on NMI handlers or sub-functions which are
invoked in the NMI context.
To check for possible inconsistency of live patching on those
functions, add an atomic flag to count patching target functions
invoked in NMI context while updating kpatch hash table. If the
flag is set by the target functions in NMI, we can not ensure
there is no concurrent execution on it.
This fixes the issue #65.
Changes from v5:
- Fix to add a NULL check in kpatch_get_committed_func().
Changes from v4:
- Change kpatch_operation to atomic_t.
- Use smp_rmb/wmb barriers between kpatch_operation and kpatch_status.
- Check in_nmi() first and if true, access kpatch_operation.
Changes from v3:
- Fix kpatch_apply/remove_patch to return 0 if succeeded.
Changes from v2:
- Clean up kpatch_get_committed_func as same style of kpatch_get_func.
- Rename opr to op in kpatch_ftrace_handler.
- Consolidate in_nmi() and kpatch_operation check into one condition.
- Fix UNPATCH/PATCH mistype in kpatch_register.
Changes from v1:
- Rename inconsistent_flag to kpatch_status.
- Introduce new enums and helper functions for kpatch_status.
- Use hash_del_rcu instead of hlist_del_rcu.
- Rename get_committed_func to kpatch_get_committed_func.
- Use ACCESS_ONCE for kpatch_operation to prevent compiler optimization.
- Fix to remove (!func || func->updating) condition from NMI check.
- Add more precise comments.
- Fix setting order of kpatch_status and kpatch_operation.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Handle registering error to unroll the ftrace filter.
This also introduces get_kpatch_func() and
kpatch_remove_funcs_from_filter() for holding up
redundant loops.
Changes from v2:
- Rebased on the latest kpatch.
Changes from v1:
- Rename get_kpatch_func to kpatch_get_func.
- Fix function definition style issue.
- Do not jump to a label in "if" block.
- Rollback the ftrace user counter if we hit an error.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
