Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	live-patching@vger.kernel.org,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>
Subject: Re: [PATCH v4 06/10] modpost: Add modinfo flag to livepatch modules
Date: Wed, 31 Jul 2019 14:58:27 +0900
Message-ID: <CAK7LNAQuS-YcXecfJ21BGzc0CimzWxQcYST5-1xRgnCQGtcL4A@mail.gmail.com> (raw)
In-Reply-To: <20190509143859.9050-7-joe.lawrence@redhat.com>

[-- 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


  parent reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190509143859.9050-1-joe.lawrence@redhat.com>
     [not found] ` <alpine.LSU.2.21.1906131451560.22698@pobox.suse.cz>
     [not found]   ` <b1a627a4-3702-9689-6c03-0c2123c06a2d@redhat.com>
     [not found]     ` <c9021573-11c6-b576-0aa6-97754c98a06e@redhat.com>
     [not found]       ` <20190614083435.uq3mk6mprbatysol@pathway.suse.cz>
2019-06-25 11:36         ` [PATCH v4 00/10] klp-convert livepatch build tooling Miroslav Benes
2019-06-25 13:24           ` Joe Lawrence
2019-06-25 19:08           ` Joe Lawrence
2019-06-26 10:27             ` Miroslav Benes
     [not found] ` <20190509143859.9050-4-joe.lawrence@redhat.com>
2019-07-31  2:50   ` [PATCH v4 03/10] livepatch: Add klp-convert tool Masahiro Yamada
2019-07-31  3:36     ` Masahiro Yamada
2019-08-09 18:42       ` Joe Lawrence
2019-08-13  1:15         ` Masahiro Yamada
     [not found] ` <20190509143859.9050-7-joe.lawrence@redhat.com>
2019-07-31  5:58   ` Masahiro Yamada [this message]
2019-08-12 15:56     ` [PATCH v4 06/10] modpost: Add modinfo flag to livepatch modules Joe Lawrence
2019-08-15 15:05       ` Masahiro Yamada
2019-08-16  8:19         ` Miroslav Benes
2019-08-16 12:43           ` Joe Lawrence
2019-08-16 19:01             ` Joe Lawrence
2019-08-19  3:50               ` Masahiro Yamada
2019-08-19 15:55                 ` Joe Lawrence
2019-08-20  7:54                   ` Miroslav Benes
2019-08-19  3:49             ` Masahiro Yamada
2019-08-19  7:31               ` Miroslav Benes
2019-08-19 16:02                 ` Joe Lawrence
2019-08-22  3:35                   ` Masahiro Yamada
2019-08-13 10:26     ` Miroslav Benes
     [not found] ` <20190509143859.9050-8-joe.lawrence@redhat.com>
2019-08-16 11:35   ` [PATCH v4 07/10] livepatch: Add sample livepatch module Masahiro Yamada
2019-08-16 12:47     ` Joe Lawrence

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK7LNAQuS-YcXecfJ21BGzc0CimzWxQcYST5-1xRgnCQGtcL4A@mail.gmail.com \
    --to=yamada.masahiro@socionext.com \
    --cc=joe.lawrence@redhat.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git