On Wed 2025-09-17 09:03:59, Josh Poimboeuf wrote:
> Add a new klp diff subcommand which performs a binary diff between two
> object files and extracts changed functions into a new object which can
> then be linked into a livepatch module.
>
> This builds on concepts from the longstanding out-of-tree kpatch [1]
> project which began in 2012 and has been used for many years to generate
> livepatch modules for production kernels. However, this is a complete
> rewrite which incorporates hard-earned lessons from 12+ years of
> maintaining kpatch.
>
> --- /dev/null
> +++ b/tools/objtool/klp-diff.c
> +static int read_exports(void)
> +{
> + const char *symvers = "Module.symvers";
> + char line[1024], *path = NULL;
> + unsigned int line_num = 1;
> + FILE *file;
> +
> + file = fopen(symvers, "r");
> + if (!file) {
> + path = top_level_dir(symvers);
> + if (!path) {
> + ERROR("can't open '%s', \"objtool diff\" should be run
> from the kernel tree", symvers);
> + return -1;
> + }
> +
> + file = fopen(path, "r");
> + if (!file) {
> + ERROR_GLIBC("fopen");
> + return -1;
> + }
> + }
> +
> + while (fgets(line, 1024, file)) {
Nit: It might be more safe to replace 1024 with sizeof(line).
It might be fixed later in a separate patch.
> + char *sym, *mod, *type;
> + struct export *export;
> +
> + sym = strchr(line, '\t');
> + if (!sym) {
> + ERROR("malformed Module.symvers (sym) at line %d",
> line_num);
> + return -1;
> + }
> +
[...]
> +/*
> + * Klp relocations aren't allowed for __jump_table and .static_call_sites if
> + * the referenced symbol lives in a kernel module, because such klp relocs
> may
> + * be applied after static branch/call init, resulting in code corruption.
> + *
> + * Validate a special section entry to avoid that. Note that an inert
> + * tracepoint is harmless enough, in that case just skip the entry and print
> a
> + * warning. Otherwise, return an error.
> + *
> + * This is only a temporary limitation which will be fixed when livepatch
> adds
> + * support for submodules: fully self-contained modules which are embedded in
> + * the top-level livepatch module's data and which can be loaded on demand
> when
> + * their corresponding to-be-patched module gets loaded. Then klp relocs can
> + * be retired.
I wonder how temporary this is ;-) The comment looks optimistic. I am
just curious. Do you have any plans to implement the support for
the submodules... ?
> + * Return:
> + * -1: error: validation failed
> + * 1: warning: tracepoint skipped
> + * 0: success
> + */
> +static int validate_special_section_klp_reloc(struct elfs *e, struct symbol
> *sym)
> +{
> + bool static_branch = !strcmp(sym->sec->name, "__jump_table");
> + bool static_call = !strcmp(sym->sec->name, ".static_call_sites");
> + struct symbol *code_sym = NULL;
> + unsigned long code_offset = 0;
> + struct reloc *reloc;
> + int ret = 0;
> +
> + if (!static_branch && !static_call)
> + return 0;
> +
Best Regards,
Petr
PS: To make some expectations. I am not doing a deep review.
I am just looking at the patchset to see how far and mature
it is. And I just comment what catches my eye.
My first impression is that it is already in a pretty good state.
And I do not see any big problem there. Well, some documentation
would be fine ;-)
What are your plans, please?
