On Thu, Sep 12, 2024 at 09:44:04AM -0400, Joe Lawrence wrote:
> On Wed, Sep 11, 2024 at 12:39:42AM -0700, Josh Poimboeuf wrote:
> > On Mon, Sep 02, 2024 at 08:59:43PM -0700, Josh Poimboeuf wrote:
> > > Hi,
> > >
> > > Here's a new way to build livepatch modules called klp-build.
> > >
> > > I started working on it when I realized that objtool already does 99% of
> > > the work needed for detecting function changes.
> > >
> > > This is similar in concept to kpatch-build, but the implementation is
> > > much cleaner.
> > >
> > > Personally I still have reservations about the "source-based" approach
> > > (klp-convert and friends), including the fragility and performance
> > > concerns of -flive-patching. I would submit that klp-build might be
> > > considered the "official" way to make livepatch modules.
> > >
> > > Please try it out and let me know what you think. Based on v6.10.
> > >
> > > Also avaiable at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build-rfc
> >
> > Here's an updated branch with a bunch of fixes. It's still incompatible
> > with BTF at the moment, otherwise it should (hopefully) fix the rest of
> > the issues reported so far.
> >
> > While the known bugs are fixed, I haven't finished processing all the
> > review comments yet. Once that happens I'll post a proper v2.
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build-v1.5
>
> Hi Josh,
>
> I've had much better results with v1.5, thanks for collecting up those
> fixes in a branch.
>
Today's experiment used the centos-stream-10's kernel config with
CONFIG_MODULE_ALLOW_BTF_MISMATCH=y and cs-10's gcc (GCC) 14.2.1 20240801
(Red Hat 14.2.1-1).
First, more gcc nits (running top-level `make`):
check.c: In function ‘decode_instructions’:
check.c:410:54: error: ‘calloc’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
410 | insns = calloc(sizeof(*insn), INSN_CHUNK_SIZE);
| ^
check.c:410:54: note: earlier argument should specify number of elements, later size of each element
check.c: In function ‘init_pv_ops’:
check.c:551:38: error: ‘calloc’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
551 | file->pv_ops = calloc(sizeof(struct pv_state), nr);
| ^~~~~~
check.c:551:38: note: earlier argument should specify number of elements, later size of each element
-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 63c2d6c06..c6f192859 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -407,7 +407,7 @@ static void decode_instructions(struct objtool_file *file)
for (offset = 0; offset < sec_size(sec); offset += insn->len) {
if (!insns || idx == INSN_CHUNK_MAX) {
- insns = calloc(sizeof(*insn), INSN_CHUNK_SIZE);
+ insns = calloc(INSN_CHUNK_SIZE, sizeof(*insn));
ERROR_ON(!insns, "calloc");
idx = 0;
@@ -548,7 +548,7 @@ static void init_pv_ops(struct objtool_file *file)
return;
nr = sym->len / sizeof(unsigned long);
- file->pv_ops = calloc(sizeof(struct pv_state), nr);
+ file->pv_ops = calloc(nr, sizeof(struct pv_state));
ERROR_ON(!file->pv_ops, "calloc");
for (idx = 0; idx < nr; idx++)
-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
and now a happy build of objtool.
The top-level `make` moves onto building all the kernel objects, but
then objtool vmlinux.o crashes:
$ gdb --args ./tools/objtool/objtool --sym-checksum --hacks=jump_label --hacks=noinstr --hacks=skylake --ibt --orc --retpoline --rethunk --static-call --uaccess --prefix=16 --link vmlinux.o
Program received signal SIGSEGV, Segmentation fault.
ignore_unreachable_insn (file=0x435ea0 <file>, insn=0x1cd928c0) at check.c:3980
3980 if (prev_insn->dead_end &&
(gdb) bt
#0 ignore_unreachable_insn (file=0x435ea0 <file>, insn=0x1cd928c0) at check.c:3980
#1 validate_reachable_instructions (file=0x435ea0 <file>) at check.c:4452
#2 check (file=file@entry=0x435ea0 <file>) at check.c:4610
#3 0x0000000000412d4f in objtool_run (argc=<optimized out>, argc@entry=14, argv=argv@entry=0x7fffffffdd78) at builtin-check.c:206
#4 0x0000000000417f9b in main (argc=14, argv=0x7fffffffdd78) at objtool.c:131
(gdb) p prev_insn
$1 = (struct instruction *) 0x0
which I worked around by copying a similar conditional check on
prev_insn after calling prev_insn_same_sec():
-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 63c2d6c06..c6f192859 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3977,7 +3977,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
* It may also insert a UD2 after calling a __noreturn function.
*/
prev_insn = prev_insn_same_sec(file, insn);
- if (prev_insn->dead_end &&
+ if (prev_insn && prev_insn->dead_end &&
(insn->type == INSN_BUG ||
(insn->type == INSN_JUMP_UNCONDITIONAL &&
insn->jump_dest && insn->jump_dest->type == INSN_BUG)))
-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
and now a happy kernel build and boot.
A klp-build of the usual cmdline.patch succeeds, however it generates
some strange relocations:
Relocation section '.rela.text' at offset 0x238 contains 6 entries:
Offset Info Type Symbol's Value Symbol's Name + Addend
0000000000000016 0000004600000004 R_X86_64_PLT32 0000000000000000 __kmalloc_noprof - 4
0000000000000035 0000004e00000004 R_X86_64_PLT32 0000000000000000 __fentry__ - 4
000000000000003c 0000000000000000 R_X86_64_NONE -4
Relocation section '.rela.klp.relocs' at offset 0x1168 contains 2 entries:
Offset Info Type Symbol's Value Symbol's Name + Addend
0000000000000000 0000000700000001 R_X86_64_64 0000000000000000 .text + 3c
0000000000000008 0000000000000001 R_X86_64_64 -4
Relocation section '.klp.rela.h..text' at offset 0x53f18 contains 1 entry:
Offset Info Type Symbol's Value Symbol's Name + Addend
000000000000003c 0000000000000002 R_X86_64_PC32 -4
No bueno. FWIW, Song's 0001-test-klp.patch does seem to build w/o odd
relocations and it loads fine.
--
Joe
