newdata variable is allocated through malloc call and requires a NULL
check.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Since ORC_STRUCT_SIZE is used for division in
kpatch_regenerate_orc_sections() we need to make sure that it is
properly set.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure fixup section was found before dereferencing it.
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
Make sure rela_toc(1|2) are not null before dereferencing them in
rela_equal().
Found by covscan, see issue #984 for full log.
Signed-off-by: Artem Savkov <asavkov@redhat.com>
The flag -gz[=type] was added in GCC 5. To support older GCC versions
check if the flag is supported before adding it to KCFLAGS.
Fixes: #1012
Signed-off-by: Stefan Strogin <steils@gentoo.org>
On some systems the linker produces compressed debug sections by
default. It is not supported by create-diff-object for now.
Fixes: #877
Signed-off-by: Stefan Strogin <steils@gentoo.org>
With the following patch:
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index e008aefc3a9d..7c70e369390d 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2228,6 +2228,8 @@ static void xs_tcp_shutdown(struct rpc_xprt *xprt)
struct socket *sock = transport->sock;
int skst = transport->inet ? transport->inet->sk_state : TCP_CLOSE;
+ asm("nop");
+
if (sock == NULL)
return;
switch (skst) {
We saw the following panic on a RHEL7.6 kernel:
Unable to handle kernel paging request for data at address 0xd00000000577f390
Faulting instruction address: 0xd000000002e918f4
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: kpatch_3_10_0_957_1_3_1_1(OEK) nfsd nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache sunrpc virtio_balloon ip_tables xfs libcrc32c virtio_net virtio_console virtio_blk virtio_pci virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod
CPU: 9 PID: 5961 Comm: kworker/9:1H Kdump: loaded Tainted: G OE K------------ 3.10.0-957.1.3.el7.ppc64le #1
Workqueue: xprtiod xprt_autoclose [sunrpc]
task: c00000000300c3c0 ti: c0000003f1814000 task.ti: c0000003f1814000
NIP: d000000002e918f4 LR: d000000002e57394 CTR: c00000000089d100
REGS: c0000003f1817980 TRAP: 0300 Tainted: G OE K------------ (3.10.0-957.1.3.el7.ppc64le)
MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE> CR: 240f2084 XER: 20000000
CFAR: 000000010bb5270c DAR: d00000000577f390 DSISR: 40000000 SOFTE: 1
GPR00: c00000000000b054 c0000003f1817c00 d00000000579add8 c000000214f0f4d0
GPR04: c0000003fd618200 c0000003fd618200 0000000000000001 0000000000000dc2
GPR08: 0000000000000dc3 0000000000000000 0000000000000000 d00000000577f370
GPR12: c0000003f1814000 c000000007b85100 c00000000012fd88 c0000003f711bb40
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000001 c0000000013510b0 0000000000000001 fffffffffffffef7
GPR24: 0000000000000000 0000000000000000 0000000000000000 c000000001b60600
GPR28: c000000214f0f000 c000000214f0f4d0 c000000214f0f408 c000000214f0f448
NIP [d000000002e918f4] __rpc_create_common.part.6+0x640/0x533c [sunrpc]
LR [d000000002e57394] xprt_autoclose+0x74/0xe0 [sunrpc]
Call Trace:
[c0000003f1817c00] [c00000000000b054] livepatch_handler+0x30/0x80 (unreliable)
[c0000003f1817c40] [c00000000012333c] process_one_work+0x1dc/0x680
[c0000003f1817ce0] [c000000000123980] worker_thread+0x1a0/0x520
[c0000003f1817d80] [c00000000012fe74] kthread+0xf4/0x100
[c0000003f1817e30] [c00000000000a628] ret_from_kernel_thread+0x5c/0xb4
Instruction dump:
396b4570 f8410018 e98b0020 7d8903a6 4e800420 00000000 73747562 000f49c0
c0000000 3d62fffe 396b4598 f8410018 <e98b0020> 7d8903a6 4e800420 00000000
---[ end trace 98e026b8fa880db7 ]---
The original version of xs_tcp_shutdown() has the following sequence:
0xd000000003cfda44 <xs_tcp_shutdown+148>: addi r1,r1,64
0xd000000003cfda48 <xs_tcp_shutdown+152>: ld r0,16(r1)
0xd000000003cfda4c <xs_tcp_shutdown+156>: ld r29,-24(r1)
0xd000000003cfda50 <xs_tcp_shutdown+160>: ld r30,-16(r1)
0xd000000003cfda54 <xs_tcp_shutdown+164>: ld r31,-8(r1)
0xd000000003cfda58 <xs_tcp_shutdown+168>: mtlr r0
0xd000000003cfda5c <xs_tcp_shutdown+172>: b 0xd000000003cfd768
That is, it restores the stack to the caller's stack frame and then does
a sibling call to the localentry point of xs_reset_transport()). So
when xs_reset_transport() returns, it will return straight to
xs_tcp_shutdown()'s caller (xprt_autoclose).
The patched version of the function has this instead (dumped from a
vmcore):
0xd000000003df0834 <xs_tcp_shutdown+148>: addi r1,r1,64
0xd000000003df0838 <xs_tcp_shutdown+152>: ld r0,16(r1)
0xd000000003df083c <xs_tcp_shutdown+156>: ld r29,-24(r1)
0xd000000003df0840 <xs_tcp_shutdown+160>: ld r30,-16(r1)
0xd000000003df0844 <xs_tcp_shutdown+164>: ld r31,-8(r1)
0xd000000003df0848 <xs_tcp_shutdown+168>: mtlr r0
0xd000000003df084c <xs_tcp_shutdown+172>: b 0xd000000003df0ad0
After restoring the stack, instead of branching directly to
xs_reset_transport(), it (rightfully) branches to a toc stub. A stub is
needed because the function it's branching to is in another module
(branching from the patch module to the sunrpc module).
The stub is:
0xd000000003df0ad0 <xs_tcp_shutdown+816>: addis r11,r2,-1
0xd000000003df0ad4 <xs_tcp_shutdown+820>: addi r11,r11,26328
0xd000000003df0ad8 <xs_tcp_shutdown+824>: std r2,24(r1)
0xd000000003df0adc <xs_tcp_shutdown+828>: ld r12,32(r11)
0xd000000003df0ae0 <xs_tcp_shutdown+832>: mtctr r12
0xd000000003df0ae4 <xs_tcp_shutdown+836>: bctr
And the "std r2,24(r1)" corrupts the caller's stack.
This stub makes sense for a normal call, because the stack would be
owned by the caller of the stub, so it's ok to write r2 to it. But
because this is a sibling call, the stack has been restored and r2 gets
incorrectly saved to the original caller's stack (i.e., xprt_autoclose's
stack).
So xprt_autoclose() -- which is in the sunrpc module -- gets the
livepatch module's toc pointer written to its stack. It panics on when
it tries to use that vlue on its very next call.
Fix it by disallowing sibling calls from patched functions on ppc64le.
In theory we could instead a) generate a custom stub, or b) modify the
kernel livepatch_handler code to save/restore the stack r2 value, but
this is easier for now.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Apply a sed filter to remove "[<localentry>: 8] " info from
readelf --wide --symbols output. This ensures consistent column
data for the awk script creating the new_symbols file.
Fixes#994
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
The list of prerequsite RPMs which are needed to build the kernel RPM is
constantly growing. But at least some of those RPMs aren't strictly
necessary for building the kernel, at least for kpatch-build's purposes.
Requiring them all to be installed is a bit overkill, and sometimes
causes kpatch-build to fail when it doesn't need to.
If the build does fail, we can always check the kpatch.log file and
update the dependencies listed in the README as needed.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Create an error if a patched function uses a jump label. We need this
until upstream livepatch supports them with a .klp.arch section.
Fixes#946.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
After Linux commit 47cdd64be483 ("dynamic_debug: refactor
dynamic_pr_debug and friends"), the name of the static local variable
used for dynamic printks is no longer "descriptor".
Make the is_special_static() check broader such that it doesn't care
about the variable name, and just whitelists any variable in the
__verbose section.
Fixes#990.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Starting with binutils 2.31, the Linux kernel may have R_X86_64_PLT32
relocations. Make sure we support them. This should be as simple as
treating R_X86_64_PLT32 exactly like R_X86_64_PC32 everywhere. For more
details see upstream commit torvalds/linux@b21ebf2.
This also fixes the following issue seen on Fedora 29:
```
$ kpatch-build/kpatch-build -t vmlinux ./test/integration/fedora-27/convert-global-local.patch
Using cache at /home/jpoimboe/.kpatch/src
Testing patch file(s)
Reading special section data
Building original source
Building patched source
Extracting new and modified ELF sections
ERROR: slub.o: 1 function(s) can not be patched
slub.o: function __kmalloc has no fentry/mcount call, unable to patch
/home/jpoimboe/git/kpatch/kpatch-build/create-diff-object: unreconcilable difference
ERROR: 1 error(s) encountered. Check /home/jpoimboe/.kpatch/build.log for more details.
```
Fixes#975.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Currently we do not support changes to functions referring to any of the
*_fixup sections on ppc64le. This patch introduces check for such
changes during the patchability check, where we abort building the
patch module.
This patch implements the phase 1 fix of 3 phases discussed at
https://github.com/dynup/kpatch/issues/974:
"
Phase 1 fix:
For kernel versions which don't have livepatch-specific powerpc code
(currently all kernel versions), kpatch-build needs to assert an error
if it detects that one of the following sections refers to a patched
function: __ftr_fixup, __mmu_ftr_fixup, __fw_ftr_fixup.
"
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Add support for __spec_barrier_fixup (barrier nospec fixup) special
section on ppc64le.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Fix the size of special group __lwsync_fixup on ppc64le.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
commit f8213c87f6 ("lookup: Fix format string for symtab_read() on
PPC64LE") fixed the symbol table lookup when readelf adds ppc64le
"[<localentry>: 8]" info for functions like so:
23: 0000000000000008 96 FUNC LOCAL DEFAULT [<localentry>: 8] 4 cmdline_proc_show
however, it seems that readelf 2.30-57.el8 displays this in a slightly
different format:
24493: c000000000587970 96 FUNC LOCAL DEFAULT 2 cmdline_proc_show [<localentry>: 8]
Instead of adding more cases to kpatch-build's lookup.c scanf format,
let's just delete this information from the symtab file with a quick and
dirty sed regex. This allows us to handle both observed cases (and
perhaps others) while removing the arch-specific scanf formatting in
lookup.c
Fixes: f8213c87f6 ("lookup: Fix format string for symtab_read() on PPC64LE")
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
commit 767d9669bd ("kpatch-build: use readelf instead of eu-readelf")
replaced eu-readelf with readelf for constructing symbol table. The
format of symbol table entries differs a little on Power when the symbol
is a function with binding type LOCAL. For example, consider:
23: 0000000000000008 96 FUNC LOCAL DEFAULT [<localentry>: 8] 4 cmdline_proc_show
An extra column preceding index of the symbol denoting symbol value to
be local entry point offset of the function is printed, with the
current sscanf format string in lookup::symtab_read the values will
mismatch ending with in accurate lookup table getting constructed. This
patch fixes it by introducing an Power specific format string for
function symbols with bind type LOCAL.
Fixes: 767d9669 ("kpatch-build: use readelf instead of eu-readelf")
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
kpatch-elf::create_section_pair would create new rela section, and the
relasec->data->d_type is not set, which is a random value, and it will
use in kpatch-elf::kpatch_write_output_elf
data->d_type = sec->data->d_type;
which would cause Segmentation fault in kpatch_write_output_elf::elf_update.
Program received signal SIGSEGV, Segmentation fault.
(gdb) bt
0 0x00007ffff7bcd8d2 in __elf64_updatefile at elf64_updatefile.c
1 0x00007ffff7bc9bed in write_file at elf_update.c
2 0x00007ffff7bc9f16 in elf_update at elf_update.c
3 0x000000000040ca3d in kpatch_write_output_elf at kpatch-elf.c
4 0x0000000000409a92 in main at create-diff-object.c
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
readelf is more standard, using readelf insteaded we should solve there
issues:
First, using "readelf -s", the symbol name would truncated by 25 chars,
to solve this issue, add option "--wide".
Second, the size may be mixed of decimal and hex, we get the size by "%s",
and use strtoul(size, NULL, 0) to convert the size.
Third, the symbol type is SHN_UNDE, the Ndx display "UND", so changed to
compare with "UND".
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
.altinstr_replacement section may have relocation symbols which need to
be included, therefore we should call kpatch_include_symbol() to ensure
that its section is included as well.
The special section processing should also occur before
kpatch_print_changes() to provide accurate logging info.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
kpatch-elf::kpatch_write_output_elf will call the gelf_getclass()
to acquire the output elf's class. But the input parameter kelf->elf
is NULL, the gelf_getclass(kelf->elf) will return ELFCLASSNONE, not
the value we expect ELFCLASS32 or ELFCLASS64.
the gelf_getclass function code:
int
gelf_getclass (Elf *elf)
{
return elf == NULL || elf->kind != ELF_K_ELF ? ELFCLASSNONE : elf->class;
}
the gelf_newehdr fuction code:
void *
gelf_newehdr (Elf *elf, int class)
{
return (class == ELFCLASS32
? (void *) INTUSE(elf32_newehdr) (elf)
: (void *) INTUSE(elf64_newehdr) (elf));
}
Luckily, when we create a patch for x86_64 or powerpc64, if we pass the
ELFCLASSNONE for the function gelf_newehdr, it will return elf64_newehdr,
so don't cause the fault. But it's better to use the gelf_getclass(elf)
instead of gelf_getclass(kelf->elf).
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
make check using shellcheck version 0.6.0 suggests following
improvements:
In kpatch/kpatch line 160:
if [[ ! -z "$checksum" ]] && [[ -e "$SYSFS/${modname}/checksum"]] ; then
^-- SC2236: Use -n instead of ! -z.
In kpatch-build/kpatch-build line 953:
[[ ! -z "$UNDEFINED" ]] && die "Undefined symbols: $UNDEFINED"
^-- SC2236: Use -n instead of ! -z.
'-n' and '! -z' are used interchangeably across the scripts, let's use
'-n' consistently to check a non-empty string instead of using negation.
Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
The create-diff-object.c create intermediate ".kpatch.relocations"
sections instead of ".kpatch.dynrelas" sections, and add a new
section ".rela.kpatch.symbols", so we should update the conditions
in function kpatch_create_intermediate_sections for these changed.
Fixes: 87643703a7 ("create-diff-object: create .kpatch.relocations and .kpatch.symbols sections")
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
After changing the gcc name in a linux tree to gcc72, kpatch-build failed to
produce hotpatches with the error message "ERROR: no changed objects found."
This is due to a wrapper script called kpatch-gcc, called while kpatch-build
builds the kernel, which checks if the compiler name matches exactly gcc,
failing the check when comparing to gcc72, and thus not producing the expected
file changed_objs containing the list of changed objects.
This commit fixes this issue by loosening the check on the gcc name.
Signed-off-by: Bruno Loreto <loretob@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.com>
Reviewed-by: Amit Shah <aams@amazon.com>
Reviewed-by: Pawel Wieczorkiewicz <wipawel@amazon.com>
The kpatch-build :: find_parent_obj() function's "deep find" may
failed to find objects if they are not located in current directory:
ERROR: invalid ancestor xxx/xxx.o for xxx/xxx.o.
This is reproducable when building an out-of-tree module of the
following structure:
wwheart@linux41:~/helloworld 0 > tree -a
.
├── buffer_overflow1.ko
├── .buffer_overflow1.ko.cmd
├── buffer_overflow1.mod.c
├── buffer_overflow1.mod.o
├── .buffer_overflow1.mod.o.cmd
├── buffer_overflow1.o
├── .buffer_overflow1.o.cmd
├── hello.c
├── hello.o
├── .hello.o.cmd
├── Makefile
├── modules.order
├── Module.symvers
├── test.patch
├── .tmp_versions
│ └── buffer_overflow1.mod
└── xxx
├── xxx.c
├── xxx.h
├── xxx.o
└── .xxx.o.cmd
wwheart@linux41:~/helloworld 0 > cat test.patch
diff --git a/xxx/xxx.c b/xxx/xxx.c
index aab3c67..d81ad00 100644
--- a/xxx/xxx.c
+++ b/xxx/xxx.c
@@ -1,6 +1,7 @@
#include <linux/kernel.h>
void czf_test(void)
{
+ printk("livepatch test\n");
printk("xxx\n");
}
wwheart@linux41:~/helloworld 0 > cat Makefile
obj-m += buffer_overflow1.o
buffer_overflow1-y += hello.o xxx/xxx.o
Modify the deep find to traverse sub-directories in order to search
the entire tree instead of only the current directory.
Fixes: 8c2792af6c ("kpatch-build: deep find performance improvement")
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
reason: The strdup() function returns a pointer to a new string
which is a duplicate of the string s. Memory for the
new string is obtained with malloc, and can be freed
with free.
here, fix memleak by removing the strdup.
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
reason: Firstly, in the function lookup_open use the malloc to
allocate some memory, but call the function lookup_close
to free the memory.
Secondly, table->obj_sym->name, table->exp_sym->name and
table->exp_sym->objname used the strdup, so them should
free also.
Thirdly, adjust the order of make_nodname, if not, it
will cause an exception when free(exp_sym->objname) in
lookup_close.
Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
This reverts commit 87c64519fc.
The jump label support doesn't work with upstream livepatch. Joe
Lawrence found the following ordering issue:
load_module
apply_relocations
/* Livepatch relocation sections are applied by livepatch */
if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
continue;
post_relocation
module_finalize
jump_label_apply_nops << crash
...
do_init_module
do_one_initcall(mod->init)
__init patch_init [kpatch-patch]
klp_register_patch
klp_init_patch
klp_init_object
klp_init_object_loaded
klp_write_object_relocations
So jump_label_apply_nops() is called *before*
klp_write_object_relocations() has had a chance to write the klp
relocations (.klp.rela.kvm_intel.__jump_table, for example).
We need to resolve this upstream first.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
When building out-of-tree modules, gcc may be passed full source
pathnames (like /home/user/testmod/testmod.c). Adjust the filepath
filtering in kpatch-gcc to match against files relative to the
KPATCH_GCC_SRCDIR / kpatch-build SRCDIR prefix.
Fixes: #941
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Add support for jump labels, also known as static jumps, static keys,
static branches, and jump tables. Luckily,
kpatch_process_special_sections() is already generic enough to make this
an easy fix.
Fixes: #931
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
For fun I tried to create a livepatch of upstream patch
ad211f3e94b314a910d4af03178a0b52a7d1ee0a for my kernel. This
caused kpatch-build to fail with a NULL pointer derefence because
base_locals was NULL (returned via kpatch_elf_locals(), which
can return a NULL pointer). This patch fixes the SIGSEGV
via a NULL check. The end result is a live patch is created
and loaded.
Signed-off-by: Balbir singh <bsingharora@gmail.com>
kpatch_mark_ignored_sections include .rodata.str1.1 section but does
not include its section symbol, causing its section symbol can not be
included any more in kpatch_include_standard_elements. After the
section symbol is freed in kpatch_elf_teardown, we got a segmentation
fault in kpatch_create_intermediate_sections.
Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
Both generate randomly modified object files on each build. This breaks
comparing original and patched object file. See also #924.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
Prevents the following shellcheck warning:
In kpatch-build/kpatch-build line 583:
which yumdownloader &>/dev/null || die "yumdownloader (yum-utils or dnf-utils) not installed"
^-- SC2230: which is non-standard. Use builtin 'command -v' instead.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
