Yulia reported a bug with module.patch which can be seen with
CONFIG_PROVE_RCU:
BUG: sleeping function called from invalid context at include/linux/percpu-rwsem.h:49
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1053, name: grep
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
2 locks held by grep/1053:
#0: ffff8881079bf1a0 (&p->lock){+.+.}-{3:3}, at: seq_read_iter+0x55/0x460
#1: ffffffff8275d620 (rcu_read_lock){....}-{1:2}, at: cache_seq_start_rcu+0x5/0x140 [sunrpc]
CPU: 3 PID: 1053 Comm: grep Tainted: G OE K 6.2.0 #57
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.1-2.fc37 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x5b/0x77
__might_resched.cold+0xff/0x13a
cpus_read_lock+0x16/0xd0
static_key_disable+0xe/0x20
e_show+0x5b/0xd70 [livepatch_module]
seq_read_iter+0x127/0x460
seq_read+0xa3/0xd0
proc_reg_read+0x52/0xa0
vfs_read+0xc9/0x2f0
? __do_sys_newfstat+0x57/0x60
? lock_is_held_type+0xe8/0x140
ksys_read+0x6c/0xf0
do_syscall_64+0x37/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
RIP: 0033:0x7fbd314f7af0
Code: b6 fe ff ff 48 8d 3d 2f 78 09 00 48 83 ec 08 e8 96 25 02 00 66 0f 1f 44 00 00 83 3d d9 db 2c 00 00 75 10 b8 00 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 7e e3 01 00 48 89 04 24
RSP: 002b:00007ffcd6f1c708 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 00007fbd314f7af0
RDX: 0000000000008000 RSI: 0000563c751ea000 RDI: 0000000000000003
RBP: 0000000000008000 R08: 0000000000009008 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000563c751ea000
R13: 0000000000000003 R14: 0000000000000003 R15: 0000563c751ea000
The problem is that the patched module's seq_operations take an RCU read
lock before calling e_show(), which is patched to call
static_key_disable(), which can sleep.
Fix the issue by moving all the module test code to a different module
(xfs) which doesn't take an RCU lock.
Also enable the pr_debug() test and make the static branch test
unconditional since they're supported by newer kernels and this test is
for 6.2.
Reported-by: Yulia Kopkova <ykopkova@redhat.com>
Debugged-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Backport this kernel commit to kpatch's version of gcc-common.h:
commit e6a71160cc145e18ab45195abf89884112e02dfb
Author: Kees Cook <keescook@chromium.org>
Date: Wed Jan 18 12:21:35 2023 -0800
gcc-plugins: Reorganize gimple includes for GCC 13
The gimple-iterator.h header must be included before gimple-fold.h
starting with GCC 13. Reorganize gimple headers to work for all GCC
versions.
Reported-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
Link: https://lore.kernel.org/all/20230113173033.4380-1-palmer@rivosinc.com/
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Note that we can't directly copy the gcc-plugin headers from the kernel
as has it has dropped gcc-4.9 and older support since v5.16. Distros
like rhel-7 still use gcc-4.8, so manually cherry pick changes into the
kpatch tree.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Kernel v6.2+ commit bea75b33895f ("x86/Kconfig: Introduce function
padding") added 16 bytes of NOP padding in front of each function.
For objects built with --function-sections, this means that function
symbols no longer sit at the beginning of their respective ELF sections,
but 16 bytes offset.
In the same release, kernel v6.2+ commit 9f2899fe36a6 ("objtool: Add
option to generate prefix symbols") adds ELF function symbols with
prefix "__pfx_" to indicate the start of a function, inclusive of
NOP-padding.
For example:
$ objdump -Dr -j.text.cmdline_proc_show fs/proc/cmdline.o
...
Disassembly of section .text.cmdline_proc_show:
0000000000000000 <__pfx_cmdline_proc_show>:
0: 90 nop
1: 90 nop
2: 90 nop
3: 90 nop
4: 90 nop
5: 90 nop
6: 90 nop
7: 90 nop
8: 90 nop
9: 90 nop
a: 90 nop
b: 90 nop
c: 90 nop
d: 90 nop
e: 90 nop
f: 90 nop
0000000000000010 <cmdline_proc_show>:
10: e8 00 00 00 00 callq 15 <cmdline_proc_show+0x5>
11: R_X86_64_PLT32 __fentry__-0x4
15: 55 push %rbp
16: 48 8b 35 00 00 00 00 mov 0x0(%rip),%rsi # 1d <cmdline_proc_show+0xd>
19: R_X86_64_PC32 saved_command_line-0x4
1d: 48 89 fd mov %rdi,%rbp
20: e8 00 00 00 00 callq 25 <cmdline_proc_show+0x15>
21: R_X86_64_PLT32 seq_puts-0x4
25: 48 89 ef mov %rbp,%rdi
28: be 0a 00 00 00 mov $0xa,%esi
2d: e8 00 00 00 00 callq 32 <cmdline_proc_show+0x22>
2e: R_X86_64_PLT32 seq_putc-0x4
32: 31 c0 xor %eax,%eax
34: 5d pop %rbp
35: e9 00 00 00 00 jmpq 3a <cmdline_proc_show+0x2a>
36: R_X86_64_PLT32 __x86_return_thunk-0x4
Kpatch-build needs to gracefully handle NOP-padding when it is present.
At the same time, it should include "__pfx_<function>" symbols when
their respective functions change, but not treat prefix such functions
as first-class functions.
This also adds support for CONFIG_CFI_CLANG, which also creates prefixed
symbols with the name "__cfi_<function>".
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Add patches rebased on top of upstream 6.2.0.
Integration tests for these can be ran as this:
$ make PATCH_DIR="linux-6.2.0" KPATCH_BUILD_OPTS="--non-replace --sourcedir /path/to/src/linux-6.2.0" integration-slow
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Kernel v6.2+ commit de3d098dd1fc ("powerpc/64: Add module check for ELF
ABI version") now verifies the ELF header e_flags for its ABI value and
refuse to load modules with complaint, "Invalid module architecture in
ELF header: 21"
Like other ELF header details, copy the flags from the reference ELF
input file to the new ELF file to avoid module load errors.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Kernel v6.2+ commit bea75b33895f ("x86/Kconfig: Introduce function
padding") introduces the -fpatchable-function-entry=16,16 build flag on
x86. This leverages compiler support for generating a
__patchable_function_entries section similar to __mcount_loc.
That said, x86 still utilizes __mcount_loc even when
__patchable_function_entries exists. The latter point to the __pfx
symbols, but the section is discarded in the vmlinux link and isn't used
regardless, for ftrace or for any other purpose.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Kernel v6.2+ commits 00abd3840812 ("objtool: Add .call_sites section")
and e81dc127ef69 ("x86/callthunks: Add call patching for call depth
tracking") added .call_sites sections to object files. These are filled
with an array of s32 values.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Kernel v6.1+ commit 7e92e01b7245 ("powerpc: Provide syscall wrapper")
introduced PowerPC specific macros. Add them to kpatch-syscall.h.
WIP: the syscall.patch integration test required
-fno-optimize-sibling-calls for sys_newuname() ... should we move the
attribute out to the patch?
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Kernel v6.1+ commit 2df8220cc511 ("kbuild: build init/built-in.a just
once") split init_uts_ns and linux_banner out to
init/version-timestamp.c from init/version.c
Add init/version-timestamp.o to the list of object files that kpatch-cc
won't add to its changed_objs list.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Architectures like ppc64le may set CONFIG_PARAVIRT=y but do not
necessarily implement via struct paravirt_patch_site.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
The term "dynrela" was invented before klp relocations ever existed.
They're basically the same thing: special livepatch-specific relocations
which get applied when the patched object gets loaded or patched.
They're necessary due to a) the need to access unexported symbols; and
b) late module patching.
The different names are confusing. Consolidate them by replacing
"dynrela" with "klp_reloc" (code) or "klp relocation" (English).
Note there's still some antiquated code in the kpatch core module and in
the pre-4.7 klp patch template which still use the "dynrela" naming.
That code is mostly dead anyway so I've just left it alone.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Now that we have KPATCH_STATIC_CALL(), document its usage. While at it,
give a more thorough description for why jump labels and static calls
aren't supported in some scenarios.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
For some reason the github version of 'make check' just started
complaining about these:
shellcheck kpatch/kpatch kpatch-build/kpatch-build kpatch-build/kpatch-cc
In kpatch-build/kpatch-build line 455:
while [[ "${filedir#$common/}" = "$filedir" ]]; do
^-----^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean:
while [[ "${filedir#"$common"/}" = "$filedir" ]]; do
In kpatch-build/kpatch-build line 460:
result="${result}${filedir#$common/}"
^-----^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean:
result="${result}${filedir#"$common"/}"
In kpatch-build/kpatch-cc line 26:
relobj=${obj##$KPATCH_GCC_SRCDIR/}
^----------------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean:
relobj=${obj##"$KPATCH_GCC_SRCDIR"/}
For more information:
https://www.shellcheck.net/wiki/SC2295 -- Expansions inside ${..} need to b...
make: *** [Makefile:70: check] Error 1
Error: Process completed with exit code 2.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Otherwise on recent distros it appends the errno to the error message,
like:
create-diff-object: ERROR: x86.o: kpatch_regenerate_special_section: 2633: Found 1 unsupported static call(s) in the patched code. Use KPATCH_STATIC_CALL() instead.: Success
which is not what we want in most cases.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Similar to jump labels, static calls aren't supported when the static
call key was originally defined in a module rather than in vmlinux.
Detect those cases and either remove them (in the case of tracepoints)
or error out.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Convert the hard-coded should_keep_jump_label() to a proper callback,
since static calls will need a similar filter.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Align the s390 special_section initializers to improve readability and
for consistency with the rest.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
kpatch_mangled_strcmp() only ignores the digits after the period, but in
the case of __UNIQUE_ID(), the symbol names have random digits before
the period due to the use of `__COUNTER__`. Make sure such symbols are
properly correlated.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Create a simple kpatch test that should apply across a wide range of
kernels. The version_proc_show() is a good candidate as it's easy to
verify and hasn't been touched upstream since 2008.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Rewrite kobj_find() to deal with Linux 5.19, where the .cmd files use
object file paths relative to the .cmd file rather than relative to the
root of the kernel tree.
While at it, add several performance enhancements to prevent all
currently known deep finds.
This is all quite fiddly. But it works.
Fixes#1277.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
When patching an OOT module, the parent object is always the OOT module.
Hard-code that to prevent the need for any further special casing in
find_kobj() (e.g., commit 9143e88f16 ("kpatch-build: fix
find_parent_obj")).
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Fix build error seen on gcc (GCC) 12.1.1 20220507 (Red Hat 12.1.1-1):
g++ -MMD -MP -I../kmod/patch -Iinsn -Wall -Wsign-compare -Wno-sign-conversion -g -Werror -shared -I/usr/lib/gcc/ppc64le-redhat-linux/12/plugin/include -Igcc-plugins -fPIC -fno-rtti -O2 -Wall gcc-plugins/ppc64le-plugin.c -o gcc-plugins/ppc64le-plugin.so
In file included from /usr/include/features.h:490,
from /usr/include/bits/libc-header-start.h:33,
from /usr/include/stdio.h:27,
from /usr/lib/gcc/ppc64le-redhat-linux/12/plugin/include/system.h:46,
from /usr/lib/gcc/ppc64le-redhat-linux/12/plugin/include/gcc-plugin.h:28,
from gcc-plugins/gcc-common.h:6,
from gcc-plugins/ppc64le-plugin.c:1:
/usr/include/bits/error-ldbl.h:23:1: error: type of ‘error’ is unknown
23 | __LDBL_REDIR_DECL (error)
| ^~~~~~~~~~~~~~~~~
/usr/include/bits/error-ldbl.h:23:1: error: ‘int error’ redeclared as different kind of entity
23 | __LDBL_REDIR_DECL (error)
| ^~~~~~~~~~~~~~~~~
In file included from gcc-plugins/ppc64le-plugin.c:2:
/usr/include/error.h:31:13: note: previous declaration ‘void error(int, int, const char*, ...)’
31 | extern void error (int __status, int __errnum, const char *__format, ...)
| ^~~~~
make[1]: *** [Makefile:39: gcc-plugins/ppc64le-plugin.so] Error 1
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Clang FDO adds a new, ignorable ELF section, .llvm.call-graph-profile
Generalize to ignore all .llvm.*
Signed-off-by: Pete Swain <swine@google.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> [subject line]
While gcc puts strings in .strtab and .shstrtab sections,
llvm toolchain just uses .strtab.
Adapt kpatch to handle both styles.
Signed-off-by: Pete Swain <swine@google.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com> [small changes]
Update the unit test submodule reference to include ppc64le files w/o
.eh_frame. At the same time, give additional guidance on stripping
these sections going forward.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
If two sections want to be the same, they need to satisfy
two conditions:
1) the result of memcmp is zero, which means they
have the same content.
2) they have the same relocation entries.
In one specific situation, two sections have the same content.
But one section has relocation entries while the other one has
no relocation entries. For example, in X86, consider the
following code:
original code
```
__noreturn noinline int kpatch_func(void)
{
while(1) {};
}
```
patched code
```
__noreturn notrace noinline int kpatch_func(void)
{
asm(".byte 0xe8, 0x00, 0x00, 0x00, 0x00");
while(1){};
}
```
Since the original code has a fentry call, these two functions have
the same compile result. But obviously, they are different functions.
Currently, kpatch would not find their differences since the patched
code has no relocation entries.
For the situation that one section has relocation entries while the
other one doesn't have, it should be set to be changed directly.
Cooperated-by: Zongwu Li <lizongwu@huawei.com>
Signed-off-by: Longjun Luo <luolongjuna@gmail.com>
