The current WARN detection logic catches the majority of cases, but
there are still a lot of outliers which it doesn't catch (thanks, gcc).
I looked at a much larger sample of WARN calls and came up with a more
generic algorithm.
The _rs variable is used for printk ratelimiting, similar to __warned,
which makes it a logical candidate to be "special": don't correlate it,
yet don't mark a function as changed just because it references it.
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.
Currently unbundled section references are only replaced if the start of
the symbol is referenced. It's also useful to support replacement of
references which point to inside the symbol.
Improve the static local variable correlation logic, for the case where
a static local is used by multiple functions. For each usage of the
variable, look for a corresponding usage in the base object. If we find
at least one matching usage, consider it a twin.
Allow static locals to be used by two functions. This is possible if
the static's containing function is inlined. We only need to find one
of them to do the correlation.
The __func__ static local variable should be deemed "special", because
it doesn't need to be correlated and should be included when needed by
an include function.
I don't have a test case for F20, but this fixes the following types of
issues when doing a full-tree recompile on RHEL 7:
ERROR: cifssmb.o: object size mismatch: __func__.49322
ERROR: btmrvl_main.o: kpatch_correlate_static_local_variables: 982: static local variable __func__.44657 not used
ERROR: iwch_qp.o: .rodata.__func__.46024 section header details differ
When patching a shared header file, don't spam the user with hundreds of
lines of "no changed functions" messages. We expect the user to be
proactive with verifying that the right functions are being patched
anyway, so this message isn't strictly necessary.
The "descriptor" static local variables and their containing __verbose
section are used for dynamic debug printks. They should be considered
as special static local variable symbols because they have the same
requirements: they should never be correlated and they should only be
included if referenced by an included function.
The fixup_group_size() function assumes that all .fixup rela groups end
with a jmpq instruction. That assumption turns out to be false when you
take into account the ____kvm_handle_fault_on_reboot() macro which is
used by kvm.
This is a new, more reliable method. It turns out that each .fixup
group is referenced by the __ex_table section. The new algorithm goes
through the __ex_table relas to figure out the size of each .fixup
group.
Also the .fixup section is now processed before __ex_table, because it
needs to access the original __ex_table relas before the unused ones
have been stripped.
Fixes the following error:
ERROR: vmx.o: fixup_group_size: 1554: can't find jump instruction in .fixup section
Currently we're checking for several special cases when deciding whether
to convert unbundled section references to their corresponding symbol
references. We do it for all unbundled text sections as well as three
specific data sections.
There's no reason I can think of for why we shouldn't just do it for
_all_ unbundled sections.
There are two distinct usages of "objname" as a variable name:
- the parent object being patched (e.g. vmlinux)
- the child object being analyzed (e.g. meminfo.o)
The name of the global objname variable conflicts with several
functions' usage of a local objname variable, resulting in some error
messages of e.g., "ERROR: vmlinux:" instead of "ERROR: meminfo.o:".
Rename the global objname variable to childobj.
There's no need to process special sections if we're returning due to no
functions changing.
Also this means we don't have to deal with extra-special usage of the
.fixup section (here's looking at you arch/x86/lib/copy_user_64.S -- we
can't patch functions in .S files anyway).
The special sections should be processed after all the other inclusion
logic has run, so that should_keep_rela_group() can work properly.
Otherwise it might remove a needed rela group from a special section.
Fix the mangled function strcmp so that it compares all of the string
except for the numbered parts. foo.isra.35 should match foo.isra.1, but
not foo.isra.35.part.36.
Fixes#352.
It's possible for a static local variable's data section to have
a relocation which refers to the variable symbol itself. Fix the logic
which searches for the user of a static local variable by only looking
in text sections (i.e. functions).
Fixes#411.
This fixes a seg fault in the test suite caused by a debug section
referencing an un-included unbundled symbol (though its section was
included). The symbol was a __warned symbol and the section was
.data.unlikely.
For debug sections, there is no need to replace section references with
symbols because we don't compare debug sections.
Add support for the __key and __warned "special" static local variables.
I'm calling them that for lack of a better term, analagous to the
kernel's special sections that we have to deal with.
__warned: Used by WARN_ONCE et al as an indicator as to whether a
message has already been printed. I think it makes sense (and is much
easier) to reset this counter for a given function when replacing the
function, since the user may expect the new function to warn again.
__key: Used by lockdep as an identifier for a given lock initialization
code path (see http://lwn.net/Articles/185666/ for more info). I think
it makes sense (and is much easier) to create a new key for a given
function when replacing the function, because the locking semantics may
have changed, so it makes sense for lockdep to use a new key to validate
the new locking behavior.
So for both __warned and __key static variables, the new version of the
variable should be used when referenced by an included function.
Made the following changes to support these special variables:
- Ignore their suffixes when comparing them in rela_equal, so that gcc
renaming them will not result in a function being marked as changed
just because it referenced a renamed static local
- Don't ever correlate them, so that their new versions will be included
if a changed or new function uses their corresponding symbols
Fixes#402.
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.
In the case that a new global symbol is defined in a file but not used
by a changed function, the symbol will currently not be included.
However, since it is global, another file in the patch my reference it,
but it will not be there.
This commit includes new global symbols so that they may be referenced
by changes in other files within the same patch.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
WARN_ON_ONCE places the __warned static local variable in the
.data.unlikely section, so it's not bundled (i.e. ignored by the
-fdata-sections gcc flag). There's no reason why we can't rename
unbundled symbols, so add support for them.
Fixes#394.
If a patch adds a new function in foo.c, and calls that function from
bar.c, currently it fails with something like:
kpatch_create_dynamic_rela_sections: 2115: lookup_global_symbol failed for tpe_allow_file, needed for .text.do_mmap_pgoff
This (crudely) fixes the issue by assuming that if we can't find the
global symbol in the original vmlinux, that it will be provided by
another object in the patch module. If that assumption is incorrect,
the module will fail to load due to the missing symbol dependency.
A (perhaps) better way to fix this is to search for the symbol in the
patched version of the vmlinux. But I think this approach is good
enough, for now at least.
Fixes#388.
The naming of variables in this function is confusing, and really threw
me for a loop: sec is first used as an iterator, then sec is reused to
point to the dynrela section, then sec2 is used as another iterator.
Instead make sec the iterator for both loops and dynsec the dynrela
section pointer.
When a function foo.isra.1 has a switch statement, it might have a
corresponding .rodata.foo.isra.1 section (in addition to its
.text.foo.isra.1 section). If so, rename that section too.
Otherwise kpatch-build will get confused when comparing the function's
relas which reference the .rodata section, and will mark the function's
rela section as changed because the rela symbol names differ.
I found this bug when trying to build the patch from upstream Linux
commit a3c54931. Unfortunately this issue is already fixed on F20 and I
wasn't able to come up with a similarly failing test case for the
integration test suite.
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.
In kpatch_create_dynamic_rela_sections() the dest field is filled in
with either the function symbol or the section symbol that contains the
function depending on whether or not the sym field of the base section
is NULL or not (around line 2153).
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.
However we weren't unbundling the stripped symbol from the section.
This resulted in the sym field pointing to the null symbol (index 0),
corrupting the dynrelas rela section.
Before:
Relocation section [14] '.rela.kpatch.dynrelas' for section [13] '.kpatch.dynrelas' at offset 0x8b8 contains 6 entries:
Offset Type Value Addend Name
000000000000000000 X86_64_64 000000000000000000 +9
0x0000000000000018 X86_64_64 000000000000000000 +8 .kpatch.strings
0x0000000000000020 X86_64_64 000000000000000000 +0 .kpatch.strings
0x0000000000000030 X86_64_64 000000000000000000 +9
0x0000000000000048 X86_64_64 000000000000000000 +8 .kpatch.strings
0x0000000000000050 X86_64_64 000000000000000000 +0 .kpatch.strings
This commit unbundles the stripped symbol from the section so that the
section symbol is used in the dynrelas rela section.
After:
Relocation section [14] '.rela.kpatch.dynrelas' for section [13] '.kpatch.dynrelas' at offset 0x8b8 contains 6 entries:
Offset Type Value Addend Name
000000000000000000 X86_64_64 000000000000000000 +9 .text.kpatch_load_aio_max_nr
0x0000000000000018 X86_64_64 000000000000000000 +8 .kpatch.strings
0x0000000000000020 X86_64_64 000000000000000000 +0 .kpatch.strings
0x0000000000000030 X86_64_64 000000000000000000 +9 .text.kpatch_unload_aio_max_nr
0x0000000000000048 X86_64_64 000000000000000000 +8 .kpatch.strings
0x0000000000000050 X86_64_64 000000000000000000 +0 .kpatch.strings
Signed-off-by: Seth Jennings <sjenning@redhat.com>
GROUP section are rare and are a mechanism in the ELF to indicated that
certain groups of section must be included or excluded (stripped)
together.
It is valid to have more than one of these section with the same
".group" name. This currently messes up the section correlation code
with correlates based solely on name.
This commit adds additional correlation criteria for GROUP sections;
namely, the section content must be the same. Changing of groups
sections (i.e. reindexing of the section indexes the GROUP section
includes in their section data) is not supported and will result in a
"new/changed section not included" error.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Fixes the following error:
kpatch_correlate_static_local_variables: 850: found another static local variable matching __func__.49968 in patched .rela__verbose
Detect .rodata.* bundled sections so that .rodata.__func__.* relocation
references can be converted to refer to their corresponding object
symbols.
Fixes the following error:
kpatch_correlate_static_local_variables: 830: static local variable __func__.49968 not used
The __verbose section stores several static local structs named
"descriptor". These structs contain information related to dynamic
debugging printks and are specific to the patched object, so they
shouldn't be correlated with their base object counterparts.
Fixes the following error:
kpatch_correlate_static_local_variables: 830: static local variable descriptor.49967 not used
Right now, the test patch unnecessarily includes hrtimer_nanosleep()
because the call to do_nanosleep() generates a rela the references the
unbundled .sched.text section. This section symbol is not currently
replaced by kpatch_replace_sections_syms() as it only replaces bundled
sections symbols.
This commit adds logic to kpatch_replace_sections_syms() to replace
unbundled section symbols as well by scanning the symbol table for
symbols that start at the rela entry's offset within the matching
section.
This allows for properly rela section correlation when the functions
have moved from an unbundled section to a bundled section.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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>
Right now, in the case that the mcount sections have changed, we get a
"changed section not included" error on them. Since we rebuild them
from scratch, just mark them as SAME even if they are different so that
we don't cause an error.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This sh_link line currently has a bug with both operands being sec1; the
second should be sec2. However the bug is masking a logical flaw; that
is that the sh_link is the index of either the symtab or the strtab and
that can change if sections have been added or removed by the patch.
This commit removes the comparison.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
gcc renames static local variables by appending a period and a number.
For example, __key could be renamed to __key.31452. Unfortunately this
number can arbitrarily change. Try to rename the patched version of the
symbol to match the base version and then correlate them.
Fixes#313.
The correlation logic could get confused if it compares two relas whose
symbols haven't been converted from section symbols to object or
function symbols. So we should replace section symbols for both the
base and the patched object before correlating, so that it can compare
the function and object symbols rather than the section symbols.
This is also a prerequisite for dealing properly with gcc's renaming of
static local variables, because relas which reference static locals
usually use section symbols.
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 running kpatch-build with -d, I was getting a seg fault. It was
faulting in kpatch_dump_kelf() when trying to print sec->secsym->name
for the .smp_locks section. It turns out that the section was included
but its section symbol wasn't included, so sec->secsym pointed to freed
memory.
This fixes the following issue for a patch which changes a module:
kpatch_create_mcount_sections: 1968: bad first rela in .rela.text.e_show
The first rela is "bad" because the real first rela was converted to a
dynrela and then removed from the rela list.
This is a temporary fix. The more permanent fix should be to allow
lookups in vmlinux for patched modules so we don't create any
unnecessary dynrelas.
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>
For ftrace to be able to trace a patched function, it requires that the
__mcount_loc section contains a pointer to the function, and that the
first instruction of the function is "callq __fentry__".
Normally that work is done by the recordmcount script, but it ignores
functions that aren't in a few standard sections (.text and a few
others).
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 original logic in the inclusion tree code worked under the
assumption that it was the only code path marking symbols for inclusion.
Therefore, if the symbol had been marked as included, it could be safely
assumed that we also already called kpatch_include_symbol() on it. With
the special section handling marking symbols as included, however, this
assumption is not valid.
We should call kpatch_include_symbol() regardless of whether or not the
symbol has already been marked as included or not in order to possible
include the symbol's entire bundle.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
With the inclusion of the debug sections, the debug output is so verbose
that it becomes less useful.
This commit reduces the verbosity by skipping rela listings of debug
sections.
It includes a new helper function, is_debug_section(), to consolidate
the logic for detecting debug sections.
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__.
This commit adds basic debuginfo support. It is "basic" in as much as
it does not try to parse the DWARF data to figure out which parts
pertain to the changed code. It simply includes all .debug_ and
.rela.debug_ section and strips out any rela entries that reference
unchanged symbols. This corrupts the debuginfo for unchanged symbols
but since they are not going to be included anyway, there should be no
way to reference that information.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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.
kpatch load fails on Ubuntu with:
kpatch: unable to find module 'vmlinux_3'
The root cause is that the vmlinux file on Ubuntu is named
vmlinux-3.13.0-24-generic instead of just vmlinux.
Let's just call it "vmlinux" in the objname field.
With test/integration/data-read-mostly.patch, create-diff-object
includes the __verbose section but not the .rela__verbose section, which
is a bug, resulting in the following printk during the integration
tests:
[13740.801920] dynamic debug error adding module: (null)
If a non-bundled section is included, its rela section should also be
included. Also add support for converting those relas to dynrelas.
When renaming a foo.isra.1 function, there might also be a foo_bar
function which would be falsely matched with the current strchr logic.
Instead of matching the "foo" prefix, match "foo.isra".
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>
Rather than keep the logic in sync between the counting and processing
code in kpatch_create_dynamic_rela_sections() just do a "dumb" count
establishing an upper bound and allocating the buffer, then determine
the actual size (i.e. number of dynrelas) in the processing section.
No functional change intended.
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>
The current approach of trying to include the tracepoint-related
sections doesn't work at all. The new tracepoints don't show up in
"perf list".
And also, with one patch (issue #219) I've seen a panic in
jump_label_del_module(). I suspect it's because the kernel is confused
by dynamic relocations' changing of the jump table after it was
registered with the jump table code.
I think the best approach for now is to just always exclude these
sections. It should be harmless, with the only consequence being that
tracepoints and jump labels can't be enabled in patched functions (which
is already the case with the current code anyway).
Fixes#221.
For a rela with type X86_64_PC32, the addend of the needed symbol is
relative to the address of the instruction _after_ the one which is the
target of the relocation.
When a changed function needs relocations for special data sections like
.data..percpu or .data..read_mostly, it's possible for those sections to
get included. We try to avoid that situation by converting section
references to data symbol references in kpatch_replace_sections_syms(),
but the conversion success rate isn't 100%, and we could be forgetting
about some other sections, so ensure that it never happens in
kpatch_verify_patchability().
Abstract out the common functionality for dealing with special sections
into a new kpatch_process_special_sections() function.
The base sections are partitioned into "groups". Only those groups
whose relas reference a changed function are kept. The only difference
in the logic for handling each special section is determining the size
of a given group. Each section has its own group_size() callback for
this. It's a callback instead of an integer because one of the
soon-to-be-supported special sections requires that its group sizes be
dynamically determined.
For a local non-included function or object which is needed by an
included function, its symbol table entry will still refer to a local
section index. Instead it should be changed to SHN_UNDEF.
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.
Both unpatched and patched objects may contain FILE
symbol with empty name. This is unexpected for
create-diff-object and could correlate 2 symbols
with same (empty) name but different types:
sym 00, type 0, bind 0, ndx 00, name (SAME)
...
sym 425, type 4, bind 0, ndx 65521, name (SAME)
...
signal.o: changed function: do_rt_tgsigqueueinfo
signal.o: changed function: do_rt_sigqueueinfo
signal.o: changed function: get_signal_to_deliver
signal.o: signal.o: changed section .rela__jump_table not selected for inclusion
signal.o: 1 unsupported section change(s)
/root/kpatch/kpatch-build/create-diff-object: unreconcilable difference
Introduce condition to match symbols also by type.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
The inventory based testing for create-diff-object was introduced at a
time when create-diff-object only needed the two object files to operate.
Now, it requires vmlinux as well. This makes the inventory testing (a
unit testing framework for create-diff-object) obsolete and difficult to
update in it's current form.
This commit removes the inventory test framework.
Signed-off-by: Seth Jennings <sjenning@redhat.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>
Right now the matching criteria for the NULL sym is type LOCAL and shndx
UNDEF. Unfortunately, that would also match any new LOCAL symbol
added to the symbol table with uninit'd sym.* fields i.e. the upcoming
__kpatch_strings and .kpatch.strings symbols.
Change the matching criteria to be symbols that have a zero-length name;
a property unique to the NULL sym.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
kpatch_migrate_included_symbols() is called from
kpatch_reorder_symbols() now, not kpatch_migrate_included_elements().
The difference is the kpatch_reorder_symbols() is operating on the
output kpatch_elf structure, and thus all symbols are by definition
included.
Remove the check and rename the function since it is redundant.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This fixes the weird ld errors we've been seeing lately.
According to the "ELF-64 Object File Format" spec, the symtab sh_info
field should contain "Index of first non-local symbol (i.e., number of
local symbols)".
Right now, reindexing of the included sections and symbols is done
when they migrate to the output kpatch_elf structure. However, due
to recently added features, the section and symbol list is not
final at this point, leading to constant tracking of the indexes for
addition sections and symbols added after this point. Additionally,
symbols have to be in a particular order, adding to the complexity.
This commit delays the reindexing and symbol reordering until the
section and symbol lists are finalized, removing the need to
track indexes and placeholders in the symbol list.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Because create-diff-object is a one-shot program (not a long lived
process) we haven't really bothered with cleaning up and freeing any
allocated memory. However, freeing data when it passes out of the
logical scope does have debugging benefits.
This commit adds two new functions for tearing down and freeing the
primary struct kpatch_elf data structures. The idea is the if a stale
pointer still references the old data structure that has passed out of
the logical scope, an issue will be more immediately apparent (i.e. NULL
references).
Signed-off-by: Seth Jennings <sjenning@redhat.com>
We rebuild the rela section data buffer in kpatch_create_rela_section()
just to rebuild it again later in kpatch_rebuild_rela_section_data()
before writing the output ELF file.
This commit removes the redundant rebuild while retaining the update
for the section header data.
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
In preparation for dynamic symbol linking, the symbol lookup logic
is going to move into create-diff-obj anyway. We might as well
minimize the code duplication and pull this into create-diff-obj.
This avoids having to re-parse the ELF file modify it in-place.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Conflicts:
kpatch-build/kpatch-build
Right now, there is a case where a symbol is included but not its
section. This is the case when the symbol is a rela dependency of
another section by the symbol section (the object or function) has not
changed. When we migrate the included symbols over to the output kelf
structure however, these symbols are still referencing their old
non-included section via their sec fields. This is a bug.
This commit adds code to the symbol migration to test whether the
symbol's section was also included. If so, it updates the symbol's
section index. If not it sets the section index to UNDEF and its sec
field to NULL.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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.
The kpatch_regenerate_* functions use a local list_head to construct the
new list. While the local list_head is copied to the sec->relas after
it is built, the neighboring nodes in the list are not updated, leading
to list corruption.
This commit uses list_replace() which updates the neighbor nodes properly.
Regression introduced by PR #1175d36dd1.
Fixes#185.
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>
In preparation for dynamic symbol linking, the symbol lookup logic
is going to move into create-diff-obj anyway. We might as well
minimize the code duplication and pull this into create-diff-obj.
This avoids having to re-parse the ELF file modify it in-place.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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>
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.
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>
While debugging the code for the bug table logic, I found it useful to
know which rela section and entry the error occurred on.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
This commit adds a new function to properly handle the bug table.
It works by going through .rela__bug_table, after the changed
function symbols have already been marked, and rewrites the section
including only the relocations pertaining to bug entries for
changed functions.
The __bug_table section itself is not modified resulting in
"blank" bug entries: ones whose IP and filename pointers will
not be relocated and, therefore, will be zero. While a waste
of space, it simplifies the code not to remove these blank
entries. They do no harm.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
The section header size is calculated at output time by libelf
and we use it as a read-only value from read files.
With the next patch we are changing the size of the .rela__bug_table
section. Lets use d_size instead since it is the value that tells
libelf how to calculate sh_size at output time.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Allow bundling of .bss.* sections that are the result of -fdata-sections
so that rela sections referencing data in bss sections by section symbol
can be replaced with the object symbol so it can be linked to the existing
data object in the kernel.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
For log_normal and DIFF_FATAL messages, prefix them with the object name
to give more context, which is useful for patches which change multiple
objects. Also, no need to add the function and line number to
DIFF_FATAL messages, as the error strings already give enough
information.
Example messages:
meminfo.o: changed function: meminfo_proc_show
cmdline.o: no changed functions were found
There are many cases where a section may have
changed due to soure-level change but the inclusion
logic has not selected it for output. Some of these
cases are real no-go situations like changing data
structures. Some are just situations that
create-diff-object isn't smart enough to figure out
(yet).
Either way, it should be considered fatal when a
changed section hasn't been selected for output.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
The indexes are in order when being read from the
table. Just index directly into the table; a benefit
of using an array for this structure instead of a linked
list.
Removes another hot path during the rela table initialization.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
remove "ughs" by changing macro to start at symbol
index 1. new for_each_symbol_zero will start at zero
for rare cases that need it.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Upon realizing that there is no point in correlating rela entries,
I also realized that tracking the status of rela entries is also
not needed.
Additionally, the rela section correlation path (really misnamed
as it is the rela section _comparison path) is VERY hot. Particularly
on files like fs/ext4/ext4.o (which create-diff-object currently can't
successfully parse entirely):
Samples: 40K of event 'cycles', Event count (approx.): 36516578362
49.49% create-diff-obj create-diff-object [.] rela_equal
31.85% create-diff-obj create-diff-object [.] kpatch_correlate_relas
16.22% create-diff-obj create-diff-object [.] find_symbol_by_index
The refactor does a few things:
- replaces nested for loops with single for loop when comparing rela entries
- removes status field for rela entires
- compares rela and nonrela sections in the same path
- removes unnecessary setting of status fields as the inclusion tree
will include them even if the section status isn't set to CHANGED. This is
even better as unchanged sections won't appear as CHANGED just because
their partner .text or .rela section is CHANGED.
This drastically reduced runtime for larger objects and cooled the rela
comparison path:
87.64% create-diff-obj create-diff-object [.] find_symbol_by_index
6.98% create-diff-obj libc-2.18.so [.] __GI___strcmp_ssse3
1.33% create-diff-obj create-diff-object [.] find_section_by_index
1.16% create-diff-obj create-diff-object [.] kpatch_correlate_symbols
0.61% create-diff-obj create-diff-object [.] kpatch_create_rela_table
0.52% create-diff-obj create-diff-object [.] kpatch_correlate_sections
Signed-off-by: Seth Jennings <sjenning@redhat.com>
I've created a test designed to exercise the ability
of create-diff-object to parse, compare, and return
"no changed functions" for the entire kernel source
tree one file at a time by passing the same file as
both the original and patched file.
Through this process, I realized that excluding every
special case from being bundled it not feasible. There
are many sections in the kernel that don't honor
-ffunction|data-sections, not just __ksymtab_strings and
.init.text.
Plus, excluding situations is not the best way. We are
really only looking for sections that _were_ the result
of -f[function|data]-sections for bundling.
To that end, this commit looks to bundle only symbol/section
pairs that should be bundled ensuringthe .text/.data suffix
and the FUNC/OBJECT symbol name match.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
A local changed function will not appear with a "changed function"
notice if it is a dependency of another changed function and that
function occurs before it in the symbol table.
Rearrange some logic to print the notice regardless of whether or not
the function symbols has already been selected for inclusion.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
When linking function bundles (.text/.rela/section sym/func sym)
ignore __init functions as they do not honor -ffunction-sections
and violate the one-to-one func/section assumption of the
function bundling.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Create a new function kpatch_copy_symbols() that copies symbols from
one kelf to another if the "select" function to return true for the
symbol.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
ld drops FUNC syms that appear after the last SECTION
sym in the symbol table.
Make sure we order the FUNC syms before the SECTION syms.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
My apologies for the size of this commit. I combined these two features
(updating API and using a hash table) into a single commit because their
implementations are tightly coupled and I didn't want to have to add
support for the old kpatch_funcs array with the new API just for the
sake of splitting up the commit :-)
- Update the core module API to get a more clear separation between core
module and patch module. This is cleaner and will help our case for
getting the core module merged upstream into the kernel.
- Convert the old kpatch_funcs array into a hash table. This is so much
nicer performance-wise and everything-else-wise than that ugly old
array.
- Do the incremental patching in stop machine. This ensures that the
funcs hash is up to date and we don't miss anything.
- Disable preemption in the ftrace handler when accessing the func hash.
That way we don't get conflicts with the stop_machine handler updating
the hash.
For a local object or function symbol, we expect that
the section offset, sym.st_value, be 0 because we used
-ffunction-sections and -fdata-section during compile.
If value != 0, it undermines assumptions we make and
should return an error. Exceptions should be handled
on a case by case basis, like __ksymtab_strings.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
In preparation for adding an automated test framework,
add an ability to create-object-diff that will create
a human readable list of included sections and symbols
with type and bind information so that the test framework
can compare against a known-good reference list with the
expected set of sections and symbols.
The file is created when the -i/--inventory option is
used. The inventory filename is the user supplied output
file name suffixed by .inventory
Signed-off-by: Seth Jennings <sjenning@redhat.com>
I'm tired of setting CFLAGS and people shouldn't have to
recompile to get debug output. This lays the foundations
proper option handling and logging levels.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
- Fixup debug messages
- Remove dead code
- No more DEPENDENCY state
- Reachability test is now the "Inclusion tree" for determining
which syms/sections will be included in the output
- 'reachable' field is now and 'include' and is the sole
consideration in including sections/symbols (no more complex
conditional checks)
- Order LOCAL before GLOBAL in the symbol table. Apparently, after
a FILE sym, all LOCAL symbols should precede GLOBAL syms or readelf
shows <corrupt>
- Handle __ksymtab_strings section and __ksymtab_* syms
Signed-off-by: Seth Jennings <sjenning@redhat.com>
