Re: [PATCH v4 06/10] modpost: Add modinfo flag to livepatch modules

Masahiro Yamada
2019-07-31T05:59:21Z

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

Hi Joe,

On Thu, May 9, 2019 at 11:39 PM Joe Lawrence <joe.lawrence@redhat.com> wrote:
>
> From: Miroslav Benes <mbenes@suse.cz>
>
> Currently, livepatch infrastructure in the kernel relies on
> MODULE_INFO(livepatch, "Y") statement in a livepatch module. Then the
> kernel module loader knows a module is indeed livepatch module and can
> behave accordingly.
>
> klp-convert, on the other hand relies on LIVEPATCH_* statement in the
> module's Makefile for exactly the same reason.
>
> Remove dependency on modinfo and generate MODULE_INFO flag
> automatically in modpost when LIVEPATCH_* is defined in the module's
> Makefile. Generate a list of all built livepatch modules based on
> the .livepatch file and store it in (MODVERDIR)/livepatchmods. Give
> this list as an argument for modpost which will use it to identify
> livepatch modules.
>
> As MODULE_INFO is no longer needed, remove it.

I do not understand this patch.
This makes the implementation so complicated.

I think MODULE_INFO(livepatch, "Y") is cleaner than
LIVEPATCH_* in Makefile.

How about this approach?

[1] Make modpost generate the list of livepatch modules.
(livepatch-modules)

[2] Generate Symbols.list in scripts/Makefile.modpost
(vmlinux + modules excluding livepatch-modules)

[3] Run klp-convert for modules in livepatch-modules.

If you do this, you can remove most of the build system hacks
can't you?

I attached an example implementation for [1].

Please check whether this works.

Thanks.

--
Best Regards
Masahiro Yamada

[-- Attachment #2: 0001-livepatch-make-modpost-generate-the-list-of-livepatc.patch --]
[-- Type: text/x-patch, Size: 3983 bytes --]

From 85571430aa12cd19a75cbc856da1092199876e6a Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <yamada.masahiro@socionext.com>
Date: Wed, 31 Jul 2019 14:51:29 +0900
Subject: [PATCH] livepatch: make modpost generate the list of livepatch
modules

Reverse the livepatch-modules direction.

The modpost generates the livepatch-modules file instead of
Makefile feeding it to modpost.

The implementation just mimics write_dump().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/Makefile.modpost | 3 ++-
scripts/mod/modpost.c | 28 ++++++++++++++++++++++++++--
scripts/mod/modpost.h | 1 +
3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 92ed02d7cd5e..c884b7b709ca 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -56,7 +56,8 @@ MODPOST = scripts/mod/modpost \
$(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS))) \
$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
- $(if $(KBUILD_MODPOST_WARN),-w)
+ $(if $(KBUILD_MODPOST_WARN),-w) \
+ $(if $(CONFIG_LIVEPATCH),-l livepatch-modules)

ifdef MODPOST_VMLINUX

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 3e6d36ddfcdf..e3f637f225e4 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1976,6 +1976,10 @@ static void read_symbols(const char *modname)
license = get_next_modinfo(&info, "license", license);
}

+ /* Livepatch modules have unresolved symbols resolved by klp-convert */
+ if (get_modinfo(&info, "livepatch"))
+ mod->livepatch = 1;
+
for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
symname = remove_dot(info.strtab + sym->st_name);

@@ -2118,7 +2122,7 @@ static int check_exports(struct module *mod)
const char *basename;
exp = find_symbol(s->name);
if (!exp || exp->module == mod) {
- if (have_vmlinux && !s->weak) {
+ if (have_vmlinux && !s->weak && !mod->livepatch) {
if (warn_unresolved) {
warn("\"%s\" [%s.ko] undefined!\n",
s->name, mod->name);
@@ -2429,6 +2433,20 @@ static void write_dump(const char *fname)
free(buf.p);
}

+static void write_livepatch_modules(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+
+ for (mod = modules; mod; mod = mod->next) {
+ if (mod->livepatch)
+ buf_printf(&buf, "%s\n", mod->name);
+ }
+
+ write_if_changed(&buf, fname);
+ free(buf.p);
+}
+
struct ext_sym_list {
struct ext_sym_list *next;
const char *file;
@@ -2440,13 +2458,14 @@ int main(int argc, char **argv)
struct buffer buf = { };
char *kernel_read = NULL, *module_read = NULL;
char *dump_write = NULL, *files_source = NULL;
+ char *livepatch_modules = NULL;
int opt;
int err;
int n;
struct ext_sym_list *extsym_iter;
struct ext_sym_list *extsym_start = NULL;

- while ((opt = getopt(argc, argv, "i:I:e:mnsT:o:awE")) != -1) {
+ while ((opt = getopt(argc, argv, "i:I:e:l:mnsT:o:awE")) != -1) {
switch (opt) {
case 'i':
kernel_read = optarg;
@@ -2463,6 +2482,9 @@ int main(int argc, char **argv)
extsym_iter->file = optarg;
extsym_start = extsym_iter;
break;
+ case 'l':
+ livepatch_modules = optarg;
+ break;
case 'm':
modversions = 1;
break;
@@ -2535,6 +2557,8 @@ int main(int argc, char **argv)
}
if (dump_write)
write_dump(dump_write);
+ if (livepatch_modules)
+ write_livepatch_modules(livepatch_modules);
if (sec_mismatch_count && sec_mismatch_fatal)
fatal("modpost: Section mismatches detected.\n"
"Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 8453d6ac2f77..2acfaae064ec 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -118,6 +118,7 @@ struct module {
int skip;
int has_init;
int has_cleanup;
+ int livepatch;
struct buffer dev_table_buf;
char srcversion[25];
int is_dot_o;
--
2.17.1