Re: [PATCH v2 54/62] objtool/klp: Add post-link subcommand to finalize livepatch modules

2025-06-26 Thread Josh Poimboeuf
On Fri, May 09, 2025 at 01:17:18PM -0700, Josh Poimboeuf wrote:
> Livepatch needs some ELF magic which linkers don't like:
> 
>   - Two relocation sections (.rela*, .klp.rela*) for the same text
> section.
> 
>   - Use of SHN_LIVEPATCH to mark livepatch symbols.
> 
> Unfortunately linkers tend to mangle such things.  To work around that,
> klp diff generates a linker-compliant intermediate binary which encodes
> the relevant KLP section/reloc/symbol metadata.
> 
> After module linking, the .ko then needs to be converted to an actual
> livepatch module.  Introduce a new klp post-link subcommand to do so.
> 
> Signed-off-by: Josh Poimboeuf 

FWIW, I have plans to get rid of this post-link step by saying goodbye
to "klp relocs" altogether.

I have a working PoC which implements livepatch "submodules" which are
specific to their target object (vmlinux or module).  The top-level
livepatch module keeps its submodule .ko binaries in memory (embedded in
its private data) and loads/unloads them on demand.

The end result looks a lot cleaner.  It also removes the restrictions we
have today which don't allow references to static call/branch keys which
live in modules.

That will have to be another patch set though, 63 patches is plenty long
enough already.

-- 
Josh



[PATCH v2 54/62] objtool/klp: Add post-link subcommand to finalize livepatch modules

2025-05-09 Thread Josh Poimboeuf
Livepatch needs some ELF magic which linkers don't like:

  - Two relocation sections (.rela*, .klp.rela*) for the same text
section.

  - Use of SHN_LIVEPATCH to mark livepatch symbols.

Unfortunately linkers tend to mangle such things.  To work around that,
klp diff generates a linker-compliant intermediate binary which encodes
the relevant KLP section/reloc/symbol metadata.

After module linking, the .ko then needs to be converted to an actual
livepatch module.  Introduce a new klp post-link subcommand to do so.

Signed-off-by: Josh Poimboeuf 
---
 tools/objtool/Build |   2 +-
 tools/objtool/builtin-klp.c |   1 +
 tools/objtool/include/objtool/klp.h |   4 +
 tools/objtool/klp-post-link.c   | 165 
 4 files changed, 171 insertions(+), 1 deletion(-)
 create mode 100644 tools/objtool/klp-post-link.c

diff --git a/tools/objtool/Build b/tools/objtool/Build
index 0b01657671d7..8cd71b9a5eef 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -9,7 +9,7 @@ objtool-y += elf.o
 objtool-y += objtool.o
 
 objtool-$(BUILD_ORC) += orc_gen.o orc_dump.o
-objtool-$(BUILD_KLP) += builtin-klp.o klp-diff.o
+objtool-$(BUILD_KLP) += builtin-klp.o klp-diff.o klp-post-link.o
 
 objtool-y += libstring.o
 objtool-y += libctype.o
diff --git a/tools/objtool/builtin-klp.c b/tools/objtool/builtin-klp.c
index 9b13dd1182af..56d5a5b92f72 100644
--- a/tools/objtool/builtin-klp.c
+++ b/tools/objtool/builtin-klp.c
@@ -14,6 +14,7 @@ struct subcmd {
 
 static struct subcmd subcmds[] = {
{ "diff",   "Generate binary diff of two object files", 
cmd_klp_diff, },
+   { "post-link",  "Finalize klp symbols/relocs after module 
linking", cmd_klp_post_link, },
 };
 
 static void cmd_klp_usage(void)
diff --git a/tools/objtool/include/objtool/klp.h 
b/tools/objtool/include/objtool/klp.h
index 07928fac059b..ad830a7ce55b 100644
--- a/tools/objtool/include/objtool/klp.h
+++ b/tools/objtool/include/objtool/klp.h
@@ -2,6 +2,9 @@
 #ifndef _OBJTOOL_KLP_H
 #define _OBJTOOL_KLP_H
 
+#define SHF_RELA_LIVEPATCH 0x0010
+#define SHN_LIVEPATCH  0xff20
+
 /*
  * __klp_objects and __klp_funcs are created by klp diff and used by the patch
  * module init code to build the klp_patch, klp_object and klp_func structs
@@ -27,5 +30,6 @@ struct klp_reloc {
 };
 
 int cmd_klp_diff(int argc, const char **argv);
+int cmd_klp_post_link(int argc, const char **argv);
 
 #endif /* _OBJTOOL_KLP_H */
diff --git a/tools/objtool/klp-post-link.c b/tools/objtool/klp-post-link.c
new file mode 100644
index ..05be6251e35f
--- /dev/null
+++ b/tools/objtool/klp-post-link.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Read the intermediate KLP reloc/symbol representations created by klp diff
+ * and convert them to the proper format required by livepatch.  This needs to
+ * run last to avoid linker wreckage.  Linkers don't tend to handle the "two
+ * rela sections for a single base section" case very well, nor do they like
+ * SHN_LIVEPATCH.
+ *
+ * This is the final tool in the livepatch module generation pipeline:
+ *
+ *   kernel builds -> objtool klp diff -> module link -> objtool klp post-link
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int fix_klp_relocs(struct elf *elf)
+{
+   struct section *symtab, *klp_relocs;
+
+   klp_relocs = find_section_by_name(elf, KLP_RELOCS_SEC);
+   if (!klp_relocs)
+   return 0;
+
+   symtab = find_section_by_name(elf, ".symtab");
+   if (!symtab) {
+   ERROR("missing .symtab");
+   return -1;
+   }
+
+   for (int i = 0; i < sec_size(klp_relocs) / sizeof(struct klp_reloc); 
i++) {
+   struct klp_reloc *klp_reloc;
+   unsigned long klp_reloc_off;
+   struct section *sec, *tmp, *klp_rsec;
+   unsigned long offset;
+   struct reloc *reloc;
+   char sym_modname[64];
+   char rsec_name[SEC_NAME_LEN];
+   u64 addend;
+   struct symbol *sym, *klp_sym;
+
+   klp_reloc_off = i * sizeof(*klp_reloc);
+   klp_reloc = klp_relocs->data->d_buf + klp_reloc_off;
+
+   /*
+* Read __klp_relocs[i]:
+*/
+
+   /* klp_reloc.sec_offset */
+   reloc = find_reloc_by_dest(elf, klp_relocs,
+  klp_reloc_off + offsetof(struct 
klp_reloc, offset));
+   if (!reloc) {
+   ERROR("malformed " KLP_RELOCS_SEC " section");
+   return -1;
+   }
+
+   sec = reloc->sym->sec;
+   offset = reloc_addend(reloc);
+
+   /* klp_reloc.sym */
+   reloc = find_reloc_by_dest(elf, klp_relocs,
+  klp_reloc_off + offsetof(struct 
klp_reloc, sym));
+