When compiling with -O2, it fails with:
gcc -MMD -MP -O2 -I../kmod/patch -Iinsn -Wall -g -Werror -c -o lookup.o lookup.c
lookup.c: In function ‘lookup_open’:
lookup.c:132:21: error: ‘file_sym’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
table->local_syms = file_sym;
~~~~~~~~~~~~~~~~~~^~~~~~~~~~
lookup.c:83:30: note: ‘file_sym’ was declared here
struct object_symbol *sym, *file_sym;
^~~~~~~~
lookup.c:129:27: error: ‘child_sym’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (in_file && !child_sym->name) {
~~~~~~~~~^~~~~~
lookup.c:85:27: note: ‘child_sym’ was declared here
struct sym_compare_type *child_sym;
^~~~~~~~~
cc1: all warnings being treated as errors
Makefile:17: recipe for target 'lookup.o' failed
make[1]: *** [lookup.o] Error 1
make[1]: Leaving directory '/home/jpoimboe/git/kpatch/kpatch-build'
Makefile:14: recipe for target 'build-kpatch-build' failed
make: *** [build-kpatch-build] Error 2
As far as I can tell, these are false positive warnings. When in_file
is 1, file_sym and child_sym are properly initialized. But silence the
warnings anyway so Gentoo users can build with -O2.
Fixes: #675
On Debian/Ubuntu, the `vmlinux` from `-dbg` package has a version number
appended to it. For example:
`/usr/lib/debug/boot/vmlinux-3.13.0-117-generic`. Make it work
nonetheless.
On Ubuntu Trusty, HWE kernels don't come with a linux-source
package. Use dget to retrieve the source package instead. This is not
the case anymore with Xenial as the linux-source package is also
provided for the HWE kernels. For Debian, backports always come with the
linux-source package.
SUSE-based kernels have a DWARF unwinder, so they build with the gcc
'-fasynchronous-unwind-tables' flag, which adds .eh_frame and
.eh_frame_hdr sections. Treat those sections like the other debug
sections.
Fixes: #703
Joe saw the following errors when loading Linux commit 128394eff343
("sg_write()/bsg_write() is not fit to be called under KERNEL_DS"):
Skipped dynrela for copy_user_generic_unrolled (0xffffffffa0475942 <- 0xffffffff813211e0): the instruction has been changed already.
Skipped dynrela for copy_user_generic_unrolled (0xffffffffa0475a57 <- 0xffffffff813211e0): the instruction has been changed already.
That is known issue #580, but it can be avoided by leaving
'copy_user_generic_unrolled' as a normal relocation instead of
converting it to a dynrela, because it's an exported symbol.
Also remove the manual check for '__fentry__' because it's covered by
the exported symbol check.
Also remove a duplicate comment about unexported global object symbols
being in another .o in the patch object.
Fixes#695.
Upstream kernel commit 7f2084fa55e6 ("[kbuild] handle exports in lib-y
objects reliably") (v4.9+) added temporary dummy .lib_exports.o objects
to the kernel build. As these ephemeral files don't contain any code,
update the kpatch-gcc glob pattern to ignore them.
(glob pattern suggested by flaming-toast)
Fixes#686.
Strip kpatch_ignore_func_* and __UNIQUE_ID_kpatch_ignore_section_*
symbols to prevent the inclusion of .kpatch.ignore.functions and
.kpatch.ignore.sections. Mark the symbols as SAME, otherwise they are
considered NEW and are recursively included. This includes the
corresponding ignore sections and rela sections and may also create new,
unnecessary dynrelas.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
A couple of minor cleanups:
- move the `if (locals)` check to find_local_syms()
- remove the explicit initialization of `local_syms`, the entire struct
was already previously cleared to zero.
When wiping out the ~/.kpatch cache before replacing it with a new
kernel source, there's no need to keep anything around. Just wipe it
all out and start over.
Also, when building with the -s option, it doesn't need to touch
~/.kpatch/version or ~/.kpatch/src, so it can just skip the cleaning.
That keeps the previous cache around for the next incantation of
kpatch-build without '-s'.
Once upon a time, kpatch-build did the kernel build in three passes.
The extra pass was done without '-ffunction-sections -fdata-sections',
so it could produce the original vmlinux file.
At that time, there was no ~/.kpatch/obj directory. The kernel was
built directly in ~/.kpatch/src. Because the same directory was used
for both the original kernel build and the '-ffunction-sections
-fdata-sections' build, the entire tree had to be rebuilt twice for
every kpatch-build incantation, making it very slow.
That situation was improved with the following commit:
5352d8b01a ("build objects in separate directory to fix caching")
That built the regular and special binaries in ~/.kpatch/obj and
~/.kpatch/obj2, respectively.
Since then we've simplified things so that it only does two build
passes: original and patched, both with '-ffunction-sections
-fdata-sections', and ~/.kpatch/obj2 was removed. However,
~/.kpatch/obj still remained. That's because we never had a reason to
change it, until now.
Recent commit aa2907df29 ("support dup file+symbol")
triggers a new warning:
create-diff-object: ERROR: dynamic_debug.o: find_local_syms: 124: find_local_syms for dynamic_debug.c: found_none
This was actually a preexisting issue which that commit helped uncover.
The root issue is that dynamic_debug.c has some creative uses of the
`__FILE__` macro. When building the kernel objects outside the source
tree, the macro results in a absolute path like:
/home/jpoimboe/.kpatch/src/lib/dynamic_debug.c
But when building inside the source tree it's a relative path:
lib/dynamic_debug.c
The Fedora kernel is built in-tree, and I would imagine most other
distros are also built that way. So the way kpatch builds can result in
a slightly different 'original' object than the distro version, thanks
to the __FILE__ macro.
In this case, the order of the symbol table changed slightly between
vmlinux and the 'orig' object. Presumably, the difference in string
lengths was enough to convince the compiler to shuffle things around a
bit.
So considering that bug, and the possibility of other mismatches, go
back to building the kernel in the source tree.
For some reason, the backticks on this line confuse my editor's syntax
highlighter! Make vim happy by using the other form of command
substition.
Also convert the function definition syntax to comply with the
kpatch-build coding guidelines ;-)
A few symbols are discarded in the kernel linking phase, which means
they won't be in the lookup table. Skip their comparison.
This fixes a bunch of warnings seen when building a patch which triggers
a tree-wide rebuild:
create-diff-object: ERROR: aes_glue.o: find_local_syms: 112: find_local_syms for aes_glue.c: found_none
create-diff-object: ERROR: aesni-intel_glue.o: find_local_syms: 112: find_local_syms for aesni-intel_glue.c: found_none
create-diff-object: ERROR: init.o: find_local_syms: 112: find_local_syms for init.c: found_none
create-diff-object: ERROR: iosf_mbi.o: find_local_syms: 112: find_local_syms for iosf_mbi.c: found_none
create-diff-object: ERROR: setup.o: find_local_syms: 112: find_local_syms for setup.c: found_none
...
After this patch, there's still one warning remaining:
create-diff-object: ERROR: dynamic_debug.o: find_local_syms: 133: find_local_syms for dynamic_debug.c: found_none
That one has a completely different cause, which I'll fix in another
pull request (coming soon).
Fixes: #676
Normal correlated symbols are marked the SAME initially but static local
variables are correlated in a separate function. Also mark these the
SAME.
This fixes an issue where patching a function which called printk_once
(which uses a static local variable) would fail to build because the
static local variable was considered new and thus introduced a new data
member into .data..read_mostly which is not allowed to change.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Rename a couple of the variables in find_local_syms() to better reflect
their purpose. The passed in 'locals' are from the childobj (e.g.
foo.o) rather than the parent (e.g. vmlinux).
When CONFIG_DEBUG_ATOMIC_SLEEP is enabled, might_sleep calls will add
the line number to the instruction stream. Detect and ignore any such
changes.
Fixes: #657.
CONFIG_PARAVIRT is not required for building kpatch patch modules. The
sizeof paravirt_patch_site struct was only needed to create
.parainstructions sections as part of create-diff-object. As long as
the original objects were built without such sections then
this kernel option (and struct handling) can be considered optional.
The CONFIG_DEBUG_INFO_SPLIT kernel .config option places debug
information into separate .dwo files. As no known distribution is
currently shipping .dwo in their debuginfo packages, leave it as
unsupported for now.
When building a kernel with CONFIG_DEBUG_INFO_REDUCED, it was observed
that subsequent readelf -wi output may not always display structure size
information first. The affects the kpatch-build awk script that needs
to consider readelf output like the following:
<1><26393>: Abbrev Number: 12 (DW_TAG_structure_type)
<26394> DW_AT_name : (indirect string, offset: 0x914f): alt_instr
<26398> DW_AT_declaration : 12
...
<1><169d1b>: Abbrev Number: 13 (DW_TAG_structure_type)
<169d1c> DW_AT_name : (indirect string, offset: 0x914f): alt_instr
<169d20> DW_AT_byte_size : 13
Therefore the awk state machine should reset if it doesn't encounter
"DW_AT_byte_size" after given structure name match.
Fixes: #668.
Support for gawk '\s' (whitespace) GNU Regexp Operator was added
somewhere between gawk 3 and 4 (RHEL6 and RHEL7). Use the [[:space:]]
bracket expression to support older releases of gawk.
We use kelf_base->symbols to find a unique matching FILE+locals combination
when we call lookup_open(). If we can't find one matching or we find more
than one matching, we error out.
If we find a unique one, we setup table->local_syms in lookup_open(),
so later lookup_local_symbol() could do its lookup based on table->local_syms.
Fixes#604.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
Introduce a second phase in the kpatch-build process that creates kpatch
modules or livepatch modules that use the new klp rela sections depending on
the kernel version being worked on. This change uses the two new programs to
either create a patch module that uses dynrelas (create-kpatch-module) or a
patch module that uses klp rela and arch sections + klp symbols marked with the
correct Elf flags (create-klp-module).
For klp patch modules, the --unique flag for ld is needed to prevent
.parainstructions and .altinstructions sections from different objects
from being merged, as arch_klp_init_object_loaded() applies these sections
per-object.
Add new program create-kpatch-module, that, given an intermediate object
outputted by create-diff-object, outputs an object (.o) that contains the
.kpatch.dynrelas section required by kpatch.
Add a new program, create-klp-module, that, given a built module (.ko),
will create a patch module with klp rela sections, klp arch sections, and
klp symbols.
In addition to .kpatch.relocations and .kpatch.symbols, have
create-diff-object create an .kpatch.arch section. This section can be used
to create .klp.arch. sections that are required for klp modules built for
versions >= 4.9. Each entry in the .kpatch.arch section represents an
arch-specific section (.altinstructions or .parainstructions) and contains
a pointer to the arch-specific section itself (see kpatch_arch struct
member 'sec') and a pointer to the objname string (see kpatch_arch struct
member 'objname'). This is enough information to be able to build
.klp.arch. sections in a later phase of kpatch-build.
Instead of creating dynrela sections, have create-diff-object create
intermediate sections .kpatch.relocations and .kpatch.symbols which can
then be used to build (depending on kernel version) either dynrela sections
or klp rela/klp arch sections + klp symbols in a later phase of kpatch-build.
Have lookup_open() also parse Module.symvers and add the resulting symbols
and their objnames to the lookup table. This code was essentially
cherry-picked from Josh Poimboeuf's lookup code found here:
8cdca59c88
That patch was modified to fix a bug in obj_read() (calling elf_end()
without strdup'ing the symbol name strings, which was causing null
dereferences) and to fix up the module name after reading it from
Module.symvers (replacing '-' with '_' and stripping the path prefixes).
Also, add lookup_exported_symbol_objname(), which looks up the objname of
an exported symbol by making use of the objname information obtained from
Module.symvers.
If there exist multiple sections with the same name (which can happen when
using the --unique option with ld, which will be used to keep multiple
(per-object) .parainstructions and .altinstructions sections separate),
find_section_by_name() will only return the first section name match, which
leads to incorrect base section assignments for rela sections. Fix this by
using the sh_info field of the rela section to find its base section
instead, which contains the index of the section to which the relocation
applies.
Make sure sym->sec is not NULL before checking for its rela section
(sym->sec->rela). This fixes a case where an object may have STT_FUNC
symbols whose the sections (sym->sec) were not selected for inclusion (or
are located in another object) and hence these symbols do not have sym->sec
set. This corner case only recently popped up after reusing kpatch_elf_open()
on objects that have been outputted by create-diff-object (and these
objects only contain the necessary sections needed for the patch module).
This will also automatically exclude livepatch symbols from the check,
because they do not have sections associated with them (i.e., sym->sec is
NULL). We do not have to check for fentry calls for klp (SHN_LIVEPATCH)
symbols, because [1] they do not have sections associated with them, [2]
they are not the target functions to be patched, and [3] they are
technically just placeholder symbols for symbol resolution in livepatch.
Move functions kpatch_reindex_elements() and kpatch_rebuild_rela_section_data()
from create-diff-object.c to kpatch-elf.c. These functions will be used
to rebuild kpatch elf data in create-klp-module and create-kpatch-module,
i.e. during the second "phase" of kpatch-build.
commit eb55adc52d ("use livepatch 4.5 features in Ubuntu Xenial
kernel") will trigger following build failure, while building stock
kernel on Ubuntu:
make[2]: Entering directory '/root/.kpatch/obj'
CC [M] /root/.kpatch/tmp/patch/patch-hook.o
In file included from
/root/.kpatch/tmp/patch/livepatch-patch-hook.c:28:0,
from /root/.kpatch/tmp/patch/patch-hook.c:21:
/root/.kpatch/tmp/patch/livepatch-patch-hook.c: In functionpatch_ini:
/root/linux-4.8.15/include/generated/utsrelease.h:2:32: error: too many
decimal points in number
#define UTS_UBUNTU_RELEASE_ABI 4.8.15
^
/root/.kpatch/tmp/patch/livepatch-patch-hook.c:252:7: note: in expansion
of macro UTS_UBUNTU_RELEASE_ABI
UTS_UBUNTU_RELEASE_ABI >= 7 ) \
^
Stock kernel version string might differ from the ubuntu kernel
versioning format. This patch sets UBUNTU_KERNEL flag, when kpatch
module is being build for ubuntu distro kernel and check for this
flag before echoing UTS_UBUNTU_RELEASE_ABI tag.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Cc: Chris J Arges <christopherarges@gmail.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Fixes sparse complaints:
create-diff-object.c:2302:24: warning: Using plain integer as NULL pointer
create-diff-object.c:2303:11: warning: Using plain integer as NULL pointer
create-diff-object.c:2334:59: warning: Using plain integer as NULL pointer
create-diff-object.c:2347:43: warning: Using plain integer as NULL pointer
When passing '-d' to kpatch-build, it prints out some useful information
and keeps the related files around in ~/.kpatch/tmp. However, it also
passes '-d' to create-diff-object, which spits out way too much
information, drowning out all the other useful messages printed by
kpatch-build.
In my experience, the create-diff-object debug info is overkill for
debugging most issues, so disable it. The flag can still be used when
running create-diff-object manually.
GCC with KASAN instrumentation creates section ".rodata" with some static strings (i.e. some of them go to ".rodata.str1.1" for release build).
This change makes possible to build patch and check if it fixes issue found with KASAN, such as CVE-2016-9555.
The UTS_UBUNTU_RELEASE_ABI symbol is in utsrelease.h as installed by
linux-headers-`uname -r`. However when building a module with kpatch-build
utsrelease.h gets regenerated and doesn't include the ABI variable. This
patch just adds the additional define based on the input ARCHVERSION.
Give a slightly better error message for the dup file+symbol issue.
It's still cryptic but it's good enough to at least give us kpatch
developers a better idea about what went wrong. This would have helped
diagnose issue #633 much more quickly.
On RHEL 7 based kernels, copy_user_64.o misuses the .fixup section by
placing a normal function in it. That confuses create-diff-object.
Work around it by just skipping the file altogether, which is fine to do
because it's an assembly file which should never change anyway.
Fixes#625.
For newer kernels, some new objects have been added to the 'head-y'
build target. These objects aren't directly traceable to vmlinux so
they have to be added manually.
Fixes#626.
When building the patched version of the kernel, vmlinux has to be
linked with the '--warn-unresolved-symbols' linker flag. Otherwise the
link will fail if the patch uses kpatch-specific symbols like
kpatch_shadow_alloc() and friends.
As of upstream Linux commit b36fad65d61f ("kbuild: Initialize exported
variables"), LDFLAGS_vmlinux= no longer works from the command line,
resulting in '--warn-unresolved-symbols' no longer getting set.
Instead we can use kpatch-gcc to pass the flag to the linker.
Fixes#627.
When pruning entries from the fixup table, update the offsets in
.rela__ex_table otherwise the relas might point to the wrong fixup entry
or even out of the .fixup section.
Fixes#615.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
If $SRCDIR was a git repo, we leave the repo with a dirty index even after
reversing the patch during cleanup. This gets picked up by
scripts/setlocalversion and consequently subsequent kpatch-builds using the
same $SRCDIR end up with a '+' sign appended to the version string. Fix
this by properly refreshing the index during cleanup.
Source RPMs for recent Fedora kernels have a '.git' subdirectory, which
causes '+' to be appended to the module version magic, causing the
module to fail to load:
kpatch_readdir: version magic '4.8.6-201.fc24.x86_64+ SMP mod_unload ' should be '4.8.6-201.fc24.x86_64 SMP mod_unload '
Add a switch to kpatch-build that provides an opt-out to the cleanup
portion of the script. This can be handy when debugging $TEMPDIR or
$RPMTOPDIR contents, as well as inspecting the patched source code
itself.
The user's environment might have TEMPDIR exported. If so, then kpatch-build
dies with a bogus "invalid ancestor" error. If you turn those bogus errors into
warnings, then the script goes on to incorrectly put into the generated .ko file
every single function that was compiled in the *original* kernel build, thereby
producing an immense .ko file with more than 64k sections that the linux kernel
cannot load. This fix makes sure that TEMPDIR is unexported on the build of the
original kernel. Actually, this fix uses a separate KPATCH_GCC_TEMPDIR variable,
so that if the kernel build is interrupted, the cleanup function in the kpatch-kbuild
script will still have TEMPDIR set correctly.
Signed-off-by: Martin Carroll <martin.carroll@alcatel-lucent.com>
This fixes the detection of WARN_ON_ONCE, WARN_ONCE, and WARN_TAINT_ONCE
on Linux 4.6 and newer.
The signature for those macros changed with upstream Linux commit
dfbf2897d004 ("bug: set warn variable before calling WARN()").
Fixes#602.
Since is_bundleable() is only called once by kpatch_create_symbol_list(),
and no other kpatch-build tool will need to call this function, we can
simply make it static and local to kpatch-elf.c
Introduce a common kpatch elf api by moving all functions and struct
declarations related to manipulating kpatch_elf objects from
create-diff-object to kpatch-elf.{h,c}. Move logging macros to a separate
file log.h, and have kpatch-elf.h include it. These changes will generalize
the kpatch-elf and logging api and make it available to other kpatch-build
tools.
Including the .altinstr_replacement section by itself and without
.altinstructions doesn't make sense, as it only serves as a memory area to
hold replacement instructions to be copied over when alternatives are
applied. Don't include .altinstr_replacement unconditionally and only
include it when .altinstructions is also marked as included.
While the officially supported distributions all have
CONFIG_DEBUG_KERNEL enabled, this is not true for some other
distributions.
This option is necessary when kpatch-build retrieves the
SPECIAL_VARS using readelf command.
Signed-off-by: Quey-Liang Kao <s101062801@m101.nthu.edu.tw>
kpatch-build currently requires Module.symvers for the Kpatch core
module unconditionally and fails if it is not found. This does not allow
using kpatch-build to prepare livepatch-based patches.
This patch fixes the problem.
Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Process the patch name correctly that only concern the fuffix with
.patch or .diff. Otherwise if the patch name is not end with .patch
or .diff but has it as substring, the fuffix will be removed
unreasonably.
Signed-off-by: Li Bin <huawei.libin@huawei.com>
Support patching objects that have duplicated function names. This feature was
introduced upstream in Linux v4.5.
This patch appends the symbol position to the symbol structure when
lookup_local_symbol is called. This pos variable is then used when creating the
funcs and dynrelas sections. Finally, incorporate sympos into the livepatch
patch hook only if the kernel version is greater than v4.5. In other cases the
older format is used.
Fixes: #493
Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
The uncorrelation logic is incomplete. For bundled symbols, in addition
to uncorrelating the sections, it should also uncorrelate the section
symbols and any rela sections.
Similarly the correlation logic needs to correlate section symbols. (It
already correlates rela sections.)
If a kernel SRPM is used to get the kernel sources, the target kernel
version is determined from the name of the SRPM.
One cannot obtain the target kernel version this way if the source tree
is used instead of an SRPM, so let us extract that information from
vmlinux.
Signed-off-by: Evgenii Shatokhin <eshatokhin@odin.com>
This fix is an addition to 9fedd0d283 "kpatch-build: fix
gcc_version_check".
On some systems, the GCC version stored in vmlinux may have the
following format:
(GNU) 4.8.3 20140911 (Red Hat 4.8.3-9)
while GCC returns
(GCC) 4.8.3 20140911 (Red Hat 4.8.3-9)
As a result, binary patches cannot be built, although the compiler is
the same.
gcc_version_check() now takes this into account.
Signed-off-by: Evgenii Shatokhin <eshatokhin@odin.com>
Deal with a special case where gcc needs a pointer to the address at the end of
a data section.
This is usually used with a compare instruction to determine when to end a
loop. The code doesn't actually dereference the pointer so this is "normal"
and we just replace the section reference with a reference to the last symbol
in the section.
Note that this only catches the issue when it happens at the end of a section.
It can also happen in the middle of a section. In that case, the wrong symbol
will be associated with the reference. But that's ok because:
1) This situation only occurs when gcc is trying to get the address of the
symbol, not the contents of its data; and
2) Because kpatch doesn't allow data sections to change, &(var1+sizeof(var1))
will always be the same as &var2.
Fixes: #553
Refine the static local variable handling again. This builds on a
previous patch by Zhou Chengming.
This fixes the following bugs reported by Zhou:
1. xxx.123 ---> xxx.123 (previous correlation by coincidence)
xxx.256 ---> xxx.256 (previous correlation by coincidence)
But real xxx.123 ---> xxx.256
In this case, the code doesn't work. Because when find patched_sym for
xxx.123, the xxx.256 in patched_object hasn't been de-correlated.
2. old-object | new-object
func1 | func1
xxx.123 | xxx.123 (inline)
func2 | func2
xxx.256 | xxx.256
xxx.123 | xxx.123 (inline)
When find patched_sym for xxx.123, first find xxx.123 in func1 of new-object,
But then find xxx.256 in func2 of new-object.
So I think should not iterate the base-sections, when find one, just go out to next symbol.
Both of these problems can be fixed by splitting the code up into
multiple passes:
1. uncorrelate all static locals
2. correlate all static locals
3. ensure each static local is referenced by all the same sections in
both objects
4. print warning on any new static locals
Fixes: #545
Before this patch, kpatch_build dependends on bash version >4.0
that support declare -A. This patch remove this dependency by
replacing dict(declare -A) with array.
Signed-off-by: Li Bin <huawei.libin@huawei.com>
When find kobj, it should use 'cat changed_objs' to get the changed
objects, in order to process the following object format:
a/b/c/../../object.o. If using patched dir to get changed object,
the object will be a/object.o, but it is a/b/c/../../object.o in
*.cmd file.
This patch also fix the find_parent_obj that change the format
'a/b/c/../../object.o' to 'a/object.o' in deep find, otherwise
it will fail with "two parent matches for *.o".
Signed-off-by: Li Bin <huawei.libin@huawei.com>
readelf -wi may output trailing spaces in the lines with section names
('alt_instr', etc.). The regexps should take this into account,
otherwise kpatch-build may fail with error:
"can't find special struct size"
This script works on other distros and can target source linux directories.
Adjust comments to match this.
Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
Rewrite the static local variable correlation logic. The algorithm now
traverses all the static locals in the original object rather than the
patched object, ensuring that each symbol in the original object has a
twin. It adds a new restriction that static local variables can't be
removed.
This adds support for the following:
- Multiple static locals with the same name in the same function
- Two separate static locals which happen to have the same numbered
suffix
- Static locals which are referenced by data sections
- CSWTCH and other static locals which are sometimes unused due to
sharing of their data sections
Fixes: #514
It turns out this is a more general issue which exists for more than
just CSWTCH symbols. The new static local handling code will handle it.
This reverts commit fd0c1bbe9c.
create-diff-object now checks if the original functions have fentry calls.
If an original function to be affected by the patch does not have the
fentry call, it cannot be patched. Error is reported in that case.
kpatch_create_mcount_sections() now also takes into account if a changed
or a new function has fentry call. If it does, mcount record is
generated for it as before. If a changed or a new function has no fentry
call, it is not an error in this case.
All this fixes the following issues.
1. If an original function has no fentry call (e.g. a "notrace" function)
but the patched function has it, the original function can not be
patched, but it would only be detected when applying the patch.
2. kpatch_create_mcount_sections() crashed if a patched function had no
relocation at all.
I observed such crashes when experimenting with a modified version of
the patch "tcp_cubic: better follow cubic curve after idle period" in
CentOS 7 x64.
Besides that, for a function with the first instruction starting with
0x0f, it would be incorrectly detemined that the function had fentry call.
The first bytes of the function would be overwritten in that case.
3. create-diff-object output an error if a new (an added) function had
no fentry call. This restriction is not necessary.
v2:
* Moved the check for fentry calls after the call to
kpatch_compare_correlated_elements() and before info about the original
ELF file is destroyed. The original symbols are now checked there (via
sym->twin) rather than the patched ones.
* Removed an excessive error check.
Signed-off-by: Evgenii Shatokhin <eshatokhin@odin.com>
Build artifacts are stored in $CACHEDIR/tmp instead of /tmp. This includes
files such as the build log and the temp directories used to build the patch.
In addition, allow $CACHEDIR to be set as an environment variable.
Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
Hard-coding the special section group sizes is unreliable. Instead,
determine them dynamically by finding the related struct definitions in
the DWARF metadata.
Fixes#517.
Fixes#523.
-ffunction-sections and -fdata-sections are needed when building the
original and the patched kernels.
It is not necessary, however, to use these options when building a
patch module itself, its functions and data are OK in the sections they
are.
Let us remove these options from KCGLAGS after the kernels have been
built.
If a source RPM is used to obtain the kernel sources, kpatch-build
executes rpmdev-setuptree to prepare ~/rpmbuild directory tree, installs
the source RPM there. Then it calls 'rpmbuild -bp' to prepare the
kernel source tree.
All this, however, may clobber the existing contents of ~/rpmbuild,
which is very inconvenient if one uses rpmbuild to build other packages.
To avoid that, I could not find a better way than to specify a fake home
directory (~/.kpatch/tempsrc) for that portion of kpatch-build. It seems,
neither rpmdev-setuptree nor rpm have appropriate options for that.
I put the affected commands into a subshell so that the changes in $HOME
could not propagate to other parts of kpatch-build.
If kpatch core module is packaged in an RPM and the package is installed,
the likely location of the module and its symvers file is
/lib/modules/<kernel_version>/extra/kpatch/.
kpatch-build checks this location too now when looking for the .symvers
file. This is convenient for distributing the Kpatch tools as RPMs and
the like.
Signed-off-by: Evgenii Shatokhin <eshatokhin@odin.com>
Before this fix, kpatch-build looked for Module.symvers for the core
module built for the currently running kernel. So, if one tried to build
a patch module for a kernel, different from the current one, an error
would occur. This patch fixed the problem.
Signed-off-by: Evgenii Shatokhin <eshatokhin@odin.com>
Before this patch, if changed function is weak symbol, it is not
be allowed to create live patch, and it will trigger the following
error:
/usr/local/libexec/kpatch/create-diff-object: ERROR: ***.o:
kpatch_create_patches_sections: 2294: lookup_global_symbol ***
And if the changed function reference the weak symbol, when loading
the patch module will trigger the following error:
module kpatch-***: overflow in relocation type *** val 0
insmod: can't insert 'kpatch-***.ko': invalid module format
This patch fix it and add support for patching weak function.
Signed-off-by: Li Bin <huawei.libin@huawei.com>
kpatch_verify_patchability can detect the change of .bss or .data or
.init section, but it must be processed before verify num_changed.
Otherwise, for example, if only .init section changed, it will fail
with 'no changed functions were found', but not 'unsupported section
change(s)'.
With this patch,
for .init section: .init section will not a bundled section, so if
the section changed, not sync the function status, kpatch_verify_patchability
will give 'changed section <secname> not selected for inclusion' and
'unsupported section change(s)' error.
for .bss/.data section: kpatch_verify_patchability will ensure not
including .data or .bss section, otherwise it will give 'data section
<secname> selected for inclusion' and 'unsupported section change(s)'
error.
Signed-off-by: Li Bin <huawei.libin@huawei.com>
If a static variable is a pointer, it has rela section.
Example:
static int *p = &a;
changed to:
static int *p = &b;
so its rela section has changed.
Then this change of data should be found and report error.
But if we don't correlate its rela section, we won't
find this change.
Signed-off-by: Zhou ChengMing <zhouchengming1@outlook.com>
kpatch-build was failing on centos7 with
mv: cannot stat '/home/vagrant/rpmbuild/BUILD/kernel-*/linux-3.10.0-229.el7.x86_64': No such file or directory
in the error log. This was due to the actual directory being named
linux-3.10.0-229.el7.centos.x86_64. This patch avoids this failure by
adding a wildcard before the arch.
Signed-off-by: Louis Taylor <louis@kragniz.eu>
Bash doesn't correctly format the version string which causes the source
package to not be downloaded correctly.
Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
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
Fixes an issue where attempting to call the shadow functions from a
module results in modpost failures:
ERROR: "kpatch_shadow_get" [net/mac80211/mac80211.ko] undefined!
ERROR: "kpatch_shadow_alloc" [net/mac80211/mac80211.ko] undefined!
LDFLAGS_MODULE is apparently not really needed.
Right now, unless the entire gcc version string, including build date
and package version, matches the distro kernel exactly, kpatch-build
won't proceed.
For some distros, it is very difficult to rollback to a previous
version of gcc and keep that version pinned on the system so that the
package manager doesn't update it.
For these user, add a --skip-gcc-check flag to kpatch-build to allow the
version check to be skipped. If the user does this, it is assumed they
know what they are doing. This flag is documented as "not recommended".
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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.
Right now, the makefile has one target, create-diff-object, which
contains all the source/headers as one long list and all the source
files compiled in one command to make create-diff-object.
This doesn't scale well and doesn't accurately portray the dependencies
of each object that contribute to the final binary.
This commit renames create-diff-object.c to main.c so that it can be
compiled and linked seperately and cleanly in Make and autogenerates
dependencies for each .o. This should make it easier to add additional
object files, or refactor the very large main.o into seperate object
file, later.
A recent commit 74316588e is unconditionally setting the SRCRPM path
overwriting a user specified path.
Only set SRCRPM if SRCRPM is not already set.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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).
With some obscure drivers, the same object file can be linked into
multiple parent objects. Only call this out as an error if the object
has changed, otherwise it doesn't matter.
Fixes the following issue:
ERROR: two parent matches for drivers/media/radio/si470x/radio-si470x-common.o.
vdso files aren't kpatch-compatible, and give errors like the following:
ERROR: invalid ancestor arch/x86/vdso/vdso32-sysenter.so.dbg for arch/x86/vdso/vdso32/sysenter.o
If we have to do a deep find (e.g. search the entire tree) to find a
parent object, first try searching in the last successful deep find
directory. This is a performance improvement in the case of a full tree
rebuild, because deep finds are very expensive, and it's not uncommon
for there to be multiple objects in a directory being linked into an
object in another directory.
There are a few more valid ancestors for vmlinux other than built-in.o.
This fixes errors similar to the following:
ERROR: invalid ancestor arch/x86/lib/lib.a for arch/x86/lib/usercopy_64.o
Fixes an error when the following is an argument to gcc:
'-DIPATH_IDSTR="QLogic' kernel.org 'driver"'
gcc: error: kernel.org: No such file or directory
gcc: error: driver": No such file or directory
yumdownloader is problematic because it doesn't allow you to download
anything but the latest released kernel. It can also be slow at times.
Instead, for Fedora, download the RPMs from koji.
Add KVER and KREL variables, and use them where appropriate. Also
remove the setting of ARCHVERSION in the '-s' case, since it's not
actually used anywhere in that case.
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.
If hyphen doesn't exist in uname -r (ARCHVERSION), then it is probably a
non-distro kernel and we don't need to create the localversion file.
Fixes#376
Signed-off-by: Seth Jennings <sjenning@redhat.com>
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.
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.
When working on large patches that are bound to have lots of
errors, it can be frustrating to have to re-run the build and wait
after every error you fix. With this patch, you get a chance to see
most (if not all) of the errors you'll be facing, at least across
the different object files.
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>
On RHEL 7 I see the following error when trying to patch meminfo.o:
cp: cannot stat ‘/home/user/.kpatch/obj/fs/proc/.tmp_meminfo.o’: No such file or directory
It turns out that on RHEL 7, a given object foo.o is compiled as
.tmp_foo.o before then being linked as foo.o. I have no idea why. The
fix is to record .tmp_foo.o as foo.o in the changed_objs file.
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>
The FILES variable can contain unchanged objects, so don't use it to
determine which objects to link. Instead, just use all the objects that
were placed in the output directory by create-diff-object.