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>
