linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/2] livepatch: patch creation tooling proposal
@ 2016-10-27 14:35 Josh Poimboeuf
  2016-10-27 14:35 ` [PATCH RFC 1/2] livepatch: add klp-convert tool and integrate with module build Josh Poimboeuf
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2016-10-27 14:35 UTC (permalink / raw)
  To: live-patching; +Cc: linux-kernel

Here's what I've been working on in preparation for my talk about
"livepatch module creation tooling" at next week's Linux Plumbers
Conference.  I'm posting this code now as a sneak preview.  Hopefully we
can get the conversation started here, as the microconference agenda is
packed and there might not be that much time for discussion.

Other than the consistency model, our last big missing piece for live
patching is the tooling for creating patch modules.

In kpatch-land, we have kpatch-build:

  https://github.com/dynup/kpatch/tree/master/kpatch-build

I wrote the original version of kpatch-build almost three years ago.  It
has been made much better since then, thanks to many improvements (and
rewrites) from Seth, Jessica, and others.  It's rather "magical".  It
takes a source code patch and spits out a patch module.  It works by
building the kernel twice: once without the patch and once with it.  It
then does a binary diff of the two kernels, and extracts that difference
into the patch module, at a function granularity.  It's a very powerful
tool, and it's very easy to use, when it works.

But maintenance of kpatch-build can be a nightmare.  It needs to have
intimate knowledge of many gcc and kernel quirks.  When one of those
quirks change, the tool breaks.  Not to mention all the bugs we already
know about today (for example, see the issues on github).  And porting
it to other arches is going to be really hard, since each arch has its
own set of quirks.

So while kpatch-build is powerful and easy to use, I get the feeling
that it's just too complex to be reasonably sustainable.  We need
something simpler.

Today we *do* have something simpler upstream, but it's *too* simple.
The existing kernel build infrastructure can be used to build livepatch
modules pretty much like any other out-of-tree kernel module.  See the
sample in samples/livepatch.  But it can't deal with relocations of
local or unexported symbols.

So here's my proposal: use the existing kernel build infrastructure.  If
klp relocations are needed, manually specify them with a new
klp_module_reloc struct and corresponding KLP_MODULE_RELOC macro.  Then
run a post-processing tool called klp-convert which converts those
klp_module_reloc structs into the sections, relocations, and symbols
needed by the klp runtime code.

See patch 2/2 to see what a patch's module source code would look like.

Note this is somewhat harder to use than kpatch-build, because you have
to copy/paste the replacement function into the source file for your
livepatch module.  And then you have to determine if the new function
needs any local or unexported symbols, and then specify those missing
symbols with the KLP_MODULE_RELOC macro.

With this patch set, figuring out the missing symbols is relatively
easy: build the patch without the livepatch modinfo, and the linker
warnings will tell you what's missing.  Or alternatively, try to load
the module and it should tell you the unresolved symbols and then fail
to load.

We can probably add more tooling to make that easier.  But anyway I
think it won't be too bad if we document the process.  The patch
creation process doesn't need to be *too* easy: the patch author already
needs to be very careful regardless.

For kGraft, what tooling do you guys use for patch creation?  I seem to
remember several approaches being mentioned -- one of them similar to
what I'm proposing here, I believe -- but I don't know what you ended up
with.  Maybe you're already doing something like this?

In any case it would be good to hear everyone's opinions about this
approach.  This code is functional, but it's a work in progress, so
don't review it *too* closely.  Just wondering what people think about
the general idea.


NOTE: There's still a big missing piece here: how to support the new
.klp.arch.objname..altinstructions/.parainstructions sections.
Specifically, when parsing the entries in an .altinstructions section,
how can klp-convert determine which object a given entry belongs to?

One solution would be to replace the explicit klp_register_patch()
interface with a more implicit special section based interface.  For
example, the klp_funcs/objects could be placed in a special section
which is recognized by the module loader.

Then klp-convert would then be able to tell which object a given
function belongs to, and could use that information to determine which
object a given .altinstruction entry belongs to, and then move the entry
to the appropriate .klp.arch section.


TODO:
- support .klp.arch.objname..altinstructions/parainstructions
- split up the patches better
- patch creation documentation
- more tooling support for detecting missing relocations?  or
  automatically converting them if a sympos isn't needed?

Josh Poimboeuf (2):
  livepatch: add klp-convert tool and integrate with module build
  livepatch: update sample module to use klp relocations

 Makefile                             |   2 +-
 include/linux/livepatch.h            |   1 +
 include/uapi/linux/Kbuild            |   1 +
 include/uapi/linux/livepatch.h       |  37 ++
 kernel/livepatch/core.c              |   4 +-
 samples/livepatch/livepatch-sample.c |  15 +-
 scripts/Makefile                     |   1 +
 scripts/Makefile.modpost             |  18 +-
 scripts/livepatch/.gitignore         |   1 +
 scripts/livepatch/Makefile           |   7 +
 scripts/livepatch/elf.c              | 717 +++++++++++++++++++++++++++++++++++
 scripts/livepatch/elf.h              |  88 +++++
 scripts/livepatch/klp-convert.c      | 181 +++++++++
 scripts/livepatch/list.h             | 381 +++++++++++++++++++
 scripts/mod/modpost.c                |   9 +-
 scripts/mod/modpost.h                |   1 +
 16 files changed, 1454 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/linux/livepatch.h
 create mode 100644 scripts/livepatch/.gitignore
 create mode 100644 scripts/livepatch/Makefile
 create mode 100644 scripts/livepatch/elf.c
 create mode 100644 scripts/livepatch/elf.h
 create mode 100644 scripts/livepatch/klp-convert.c
 create mode 100644 scripts/livepatch/list.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH RFC 1/2] livepatch: add klp-convert tool and integrate with module build
  2016-10-27 14:35 [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
@ 2016-10-27 14:35 ` Josh Poimboeuf
  2016-12-17 13:08   ` [PATCH] livepatch: fixup klp-convert tool integration Konstantin Khlebnikov
  2016-10-27 14:35 ` [PATCH RFC 2/2] livepatch: update sample module to use klp relocations Josh Poimboeuf
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Josh Poimboeuf @ 2016-10-27 14:35 UTC (permalink / raw)
  To: live-patching; +Cc: linux-kernel

This still needs to be split up into several patches.  It does several
things:

- Create a klp_module_reloc struct and KLP_MODULE_RELOC macro which
  gives livepatch modules a way to specify which local and unexported
  symbols they need.

- Create a klp-convert tool which reads the klp_module_reloc structs
  from the ".klp.module_relocs.*" sections and converts the relevant
  symbols and relocations to their klp counterparts.

- Integrate klp-convert with the module build so that it automatically
  converts any kernel modules with the 'livepatch' modinfo to real
  livepatch modules.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Makefile                        |   2 +-
 include/linux/livepatch.h       |   1 +
 include/uapi/linux/Kbuild       |   1 +
 include/uapi/linux/livepatch.h  |  37 +++
 kernel/livepatch/core.c         |   4 +-
 scripts/Makefile                |   1 +
 scripts/Makefile.modpost        |  18 +-
 scripts/livepatch/.gitignore    |   1 +
 scripts/livepatch/Makefile      |   7 +
 scripts/livepatch/elf.c         | 717 ++++++++++++++++++++++++++++++++++++++++
 scripts/livepatch/elf.h         |  88 +++++
 scripts/livepatch/klp-convert.c | 181 ++++++++++
 scripts/livepatch/list.h        | 381 +++++++++++++++++++++
 scripts/mod/modpost.c           |   9 +-
 scripts/mod/modpost.h           |   1 +
 15 files changed, 1443 insertions(+), 6 deletions(-)
 create mode 100644 include/uapi/linux/livepatch.h
 create mode 100644 scripts/livepatch/.gitignore
 create mode 100644 scripts/livepatch/Makefile
 create mode 100644 scripts/livepatch/elf.c
 create mode 100644 scripts/livepatch/elf.h
 create mode 100644 scripts/livepatch/klp-convert.c
 create mode 100644 scripts/livepatch/list.h

diff --git a/Makefile b/Makefile
index 93beca4..5114e99 100644
--- a/Makefile
+++ b/Makefile
@@ -555,7 +555,7 @@ ifeq ($(KBUILD_EXTMOD),)
 # in parallel
 PHONY += scripts
 scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
-	 asm-generic gcc-plugins
+	 asm-generic gcc-plugins headers_install
 	$(Q)$(MAKE) $(build)=$(@)
 
 # Objects we will link into vmlinux / subdirs we need to visit
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 9072f04..0537eb9 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/ftrace.h>
+#include <uapi/linux/livepatch.h>
 
 #if IS_ENABLED(CONFIG_LIVEPATCH)
 
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index bc2ef9f..5500312 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -250,6 +250,7 @@ header-y += l2tp.h
 header-y += libc-compat.h
 header-y += lirc.h
 header-y += limits.h
+header-y += livepatch.h
 header-y += llc.h
 header-y += loop.h
 header-y += lp.h
diff --git a/include/uapi/linux/livepatch.h b/include/uapi/linux/livepatch.h
new file mode 100644
index 0000000..41184f9
--- /dev/null
+++ b/include/uapi/linux/livepatch.h
@@ -0,0 +1,37 @@
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _UAPI_LIVEPATCH_H
+#define _UAPI_LIVEPATCH_H
+
+#include <linux/types.h>
+
+#define KLP_RELA_PREFIX		".klp.rela."
+#define KLP_SYM_PREFIX		".klp.sym."
+
+struct klp_module_reloc {
+	void *sym;
+	unsigned int sympos;
+} __attribute__((packed));
+
+#define KLP_MODULE_RELOC(obj) \
+	klp_module_reloc \
+	__attribute__((__section__(".klp.module_relocs." #obj)))
+
+#endif /* _UAPI_LIVEPATCH_H */
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index af46438..e451345 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -240,7 +240,7 @@ static int klp_resolve_symbols(Elf_Shdr *relasec, struct module *pmod)
 
 		/* Format: .klp.sym.objname.symname,sympos */
 		cnt = sscanf(strtab + sym->st_name,
-			     ".klp.sym.%55[^.].%127[^,],%lu",
+			     KLP_SYM_PREFIX "%55[^.].%127[^,],%lu",
 			     objname, symname, &sympos);
 		if (cnt != 3) {
 			pr_err("symbol %s has an incorrectly formatted name",
@@ -286,7 +286,7 @@ static int klp_write_object_relocations(struct module *pmod,
 		 * See comment in klp_resolve_symbols() for an explanation
 		 * of the selected field width value.
 		 */
-		cnt = sscanf(secname, ".klp.rela.%55[^.]", sec_objname);
+		cnt = sscanf(secname, KLP_RELA_PREFIX "%55[^.]", sec_objname);
 		if (cnt != 1) {
 			pr_err("section %s has an incorrectly formatted name",
 			       secname);
diff --git a/scripts/Makefile b/scripts/Makefile
index 1d80897..2cc2b8a 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -45,6 +45,7 @@ subdir-y                     += mod
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_DTC)         += dtc
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
+subdir-$(CONFIG_LIVEPATCH)   += livepatch
 
 # Let clean descend into subdirs
 subdir-	+= basic kconfig package gcc-plugins
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 16923ba..916dd34 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -123,10 +123,24 @@ quiet_cmd_ld_ko_o = LD [M]  $@
 	$(LD) -r $(LDFLAGS)                                             \
                  $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)             \
                  -o $@ $(filter-out FORCE,$^) ;                         \
-	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
+	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) ;
+
+ifdef CONFIG_LIVEPATCH
+KLP_CONVERT = scripts/livepatch/klp-convert
+      cmd_klp_convert =							\
+	if [[ -n "`modinfo -F livepatch $@`" ]]; then			\
+		mv $@ $(@:.ko=.klp.o);					\
+		$(KLP_CONVERT) $(@:.ko=.klp.o) $@;			\
+	fi ;
+endif
+
+define rule_link_module
+	$(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o)				\
+	$(cmd_klp_convert)
+endef
 
 $(modules): %.ko :%.o %.mod.o FORCE
-	+$(call if_changed,ld_ko_o)
+	+$(call if_changed_rule,link_module)
 
 targets += $(modules)
 
diff --git a/scripts/livepatch/.gitignore b/scripts/livepatch/.gitignore
new file mode 100644
index 0000000..dc22fe4
--- /dev/null
+++ b/scripts/livepatch/.gitignore
@@ -0,0 +1 @@
+klp-convert
diff --git a/scripts/livepatch/Makefile b/scripts/livepatch/Makefile
new file mode 100644
index 0000000..221829b
--- /dev/null
+++ b/scripts/livepatch/Makefile
@@ -0,0 +1,7 @@
+hostprogs-y		:= klp-convert
+always			:= $(hostprogs-y)
+
+klp-convert-objs	:= klp-convert.o elf.o
+
+HOSTCFLAGS		:= -g -I$(INSTALL_HDR_PATH)/include -Wall
+HOSTLDFLAGS		:= -lelf
diff --git a/scripts/livepatch/elf.c b/scripts/livepatch/elf.c
new file mode 100644
index 0000000..bbf1b76
--- /dev/null
+++ b/scripts/livepatch/elf.c
@@ -0,0 +1,717 @@
+/*
+ * elf.c - ELF access library
+ *
+ * Adapted from kpatch (https://github.com/dynup/kpatch):
+ * Copyright (C) 2013-2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ * Copyright (C) 2014 Seth Jennings <sjenning@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "elf.h"
+
+#define WARN(format, ...) \
+	fprintf(stderr, "%s: " format "\n", elf->name, ##__VA_ARGS__)
+
+/*
+ * Fallback for systems without this "read, mmaping if possible" cmd.
+ */
+#ifndef ELF_C_READ_MMAP
+#define ELF_C_READ_MMAP ELF_C_READ
+#endif
+
+bool is_rela_section(struct section *sec)
+{
+	return (sec->sh.sh_type == SHT_RELA);
+}
+
+struct section *find_section_by_name(struct elf *elf, const char *name)
+{
+	struct section *sec;
+
+	list_for_each_entry(sec, &elf->sections, list)
+		if (!strcmp(sec->name, name))
+			return sec;
+
+	return NULL;
+}
+
+static struct section *find_section_by_index(struct elf *elf,
+					     unsigned int idx)
+{
+	struct section *sec;
+
+	list_for_each_entry(sec, &elf->sections, list)
+		if (sec->idx == idx)
+			return sec;
+
+	return NULL;
+}
+
+static struct symbol *find_symbol_by_index(struct elf *elf, unsigned int idx)
+{
+	struct symbol *sym;
+
+	list_for_each_entry(sym, &elf->symbols, list)
+		if (sym->idx == idx)
+			return sym;
+
+	return NULL;
+}
+
+struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
+				     unsigned int len)
+{
+	struct rela *rela;
+	unsigned long o;
+
+	if (!sec->rela)
+		return NULL;
+
+	for (o = offset; o < offset + len; o++)
+		list_for_each_entry(rela, &sec->rela->relas, list)
+			if (rela->offset == o)
+				return rela;
+
+	return NULL;
+}
+
+struct rela *find_rela_by_dest(struct section *sec, unsigned long offset)
+{
+	return find_rela_by_dest_range(sec, offset, 1);
+}
+
+static int read_sections(struct elf *elf)
+{
+	Elf_Scn *s = NULL;
+	struct section *sec;
+	size_t shstrndx, sections_nr;
+	int i;
+
+	if (elf_getshdrnum(elf->elf, &sections_nr)) {
+		perror("elf_getshdrnum");
+		return -1;
+	}
+
+	if (elf_getshdrstrndx(elf->elf, &shstrndx)) {
+		perror("elf_getshdrstrndx");
+		return -1;
+	}
+
+	for (i = 0; i < sections_nr; i++) {
+		sec = malloc(sizeof(*sec));
+		if (!sec) {
+			perror("malloc");
+			return -1;
+		}
+		memset(sec, 0, sizeof(*sec));
+
+		INIT_LIST_HEAD(&sec->relas);
+
+		list_add_tail(&sec->list, &elf->sections);
+
+		s = elf_getscn(elf->elf, i);
+		if (!s) {
+			perror("elf_getscn");
+			return -1;
+		}
+
+		sec->idx = elf_ndxscn(s);
+
+		if (!gelf_getshdr(s, &sec->sh)) {
+			perror("gelf_getshdr");
+			return -1;
+		}
+
+		sec->name = elf_strptr(elf->elf, shstrndx, sec->sh.sh_name);
+		if (!sec->name) {
+			perror("elf_strptr");
+			return -1;
+		}
+
+		sec->elf_data = elf_getdata(s, NULL);
+		if (!sec->elf_data) {
+			perror("elf_getdata");
+			return -1;
+		}
+
+		if (sec->elf_data->d_off != 0 ||
+		    sec->elf_data->d_size != sec->sh.sh_size) {
+			WARN("unexpected data attributes for %s", sec->name);
+			return -1;
+		}
+
+		sec->data = sec->elf_data->d_buf;
+		sec->size = sec->elf_data->d_size;
+	}
+
+	/* sanity check, one more call to elf_nextscn() should return NULL */
+	if (elf_nextscn(elf->elf, s)) {
+		WARN("section entry mismatch");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int read_symbols(struct elf *elf)
+{
+	struct section *symtab;
+	struct symbol *sym;
+	int symbols_nr, i;
+
+	symtab = find_section_by_name(elf, ".symtab");
+	if (!symtab) {
+		WARN("missing symbol table");
+		return -1;
+	}
+
+	symbols_nr = symtab->sh.sh_size / symtab->sh.sh_entsize;
+
+	for (i = 0; i < symbols_nr; i++) {
+		sym = malloc(sizeof(*sym));
+		if (!sym) {
+			perror("malloc");
+			return -1;
+		}
+		memset(sym, 0, sizeof(*sym));
+
+		sym->idx = i;
+
+		if (!gelf_getsym(symtab->elf_data, i, &sym->sym)) {
+			perror("gelf_getsym");
+			goto err;
+		}
+
+		sym->name = elf_strptr(elf->elf, symtab->sh.sh_link,
+				       sym->sym.st_name);
+		if (!sym->name) {
+			perror("elf_strptr");
+			goto err;
+		}
+
+		sym->type = GELF_ST_TYPE(sym->sym.st_info);
+		sym->bind = GELF_ST_BIND(sym->sym.st_info);
+
+		if (sym->sym.st_shndx > SHN_UNDEF &&
+		    sym->sym.st_shndx < SHN_LORESERVE) {
+			sym->sec = find_section_by_index(elf,
+							 sym->sym.st_shndx);
+			if (!sym->sec) {
+				WARN("couldn't find section for symbol %s",
+				     sym->name);
+				goto err;
+			}
+			if (sym->type == STT_SECTION) {
+				sym->name = sym->sec->name;
+				sym->sec->sym = sym;
+			}
+		}
+
+		sym->offset = sym->sym.st_value;
+		sym->size = sym->sym.st_size;
+
+		list_add_tail(&sym->list, &elf->symbols);
+	}
+
+	return 0;
+
+err:
+	free(sym);
+	return -1;
+}
+
+static int read_relas(struct elf *elf)
+{
+	struct section *sec;
+	struct rela *rela;
+	int i;
+	unsigned int symndx;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (sec->sh.sh_type != SHT_RELA)
+			continue;
+
+		sec->base = find_section_by_name(elf, sec->name + 5);
+		if (!sec->base) {
+			WARN("can't find base section for rela section %s",
+			     sec->name);
+			return -1;
+		}
+
+		sec->base->rela = sec;
+
+		for (i = 0; i < sec->sh.sh_size / sec->sh.sh_entsize; i++) {
+			rela = malloc(sizeof(*rela));
+			if (!rela) {
+				perror("malloc");
+				return -1;
+			}
+			memset(rela, 0, sizeof(*rela));
+
+			if (!gelf_getrela(sec->elf_data, i, &rela->rela)) {
+				perror("gelf_getrela");
+				return -1;
+			}
+
+			rela->type = GELF_R_TYPE(rela->rela.r_info);
+			rela->addend = rela->rela.r_addend;
+			rela->offset = rela->rela.r_offset;
+			symndx = GELF_R_SYM(rela->rela.r_info);
+			rela->sym = find_symbol_by_index(elf, symndx);
+			if (!rela->sym) {
+				WARN("can't find rela entry symbol %d for %s",
+				     symndx, sec->name);
+				return -1;
+			}
+
+			list_add_tail(&rela->list, &sec->relas);
+		}
+	}
+
+	return 0;
+}
+
+struct section *create_rela_section(struct elf *elf, const char *name,
+				    struct section *base)
+{
+	struct section *sec;
+
+	sec = malloc(sizeof(*sec));
+	if (!sec) {
+		WARN("malloc failed");
+		return NULL;
+	}
+	memset(sec, 0, sizeof(*sec));
+	INIT_LIST_HEAD(&sec->relas);
+
+	sec->base = base;
+	sec->name = strdup(name);
+	sec->sh.sh_name = -1;
+	sec->sh.sh_type = SHT_RELA;
+	sec->sh.sh_entsize = sizeof(GElf_Rela);
+	sec->sh.sh_addralign = 8;
+	sec->sh.sh_flags = SHF_ALLOC;
+
+	sec->elf_data = malloc(sizeof(*sec->elf_data));
+	if (!sec->elf_data) {
+		WARN("malloc failed");
+		return NULL;
+	}
+	memset(sec->elf_data, 0, sizeof(*sec->elf_data));
+	sec->elf_data->d_type = ELF_T_RELA;
+
+	list_add_tail(&sec->list, &elf->sections);
+
+	return sec;
+}
+
+static int update_shstrtab(struct elf *elf)
+{
+	struct section *shstrtab, *sec;
+	size_t orig_size, new_size = 0, offset, len;
+	char *buf;
+
+	shstrtab = find_section_by_name(elf, ".shstrtab");
+	if (!shstrtab) {
+		WARN("can't find .shstrtab");
+		return -1;
+	}
+
+	orig_size = new_size = shstrtab->size;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (sec->sh.sh_name != -1)
+			continue;
+		new_size += strlen(sec->name) + 1;
+	}
+
+	if (new_size == orig_size)
+		return 0;
+
+	buf = malloc(new_size);
+	if (!buf) {
+		WARN("malloc failed");
+		return -1;
+	}
+	memcpy(buf, (void *)shstrtab->data, orig_size);
+
+	offset = orig_size;
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (sec->sh.sh_name != -1)
+			continue;
+		sec->sh.sh_name = offset;
+		len = strlen(sec->name) + 1;
+		memcpy(buf + offset, sec->name, len);
+		offset += len;
+	}
+
+	shstrtab->elf_data->d_buf = shstrtab->data = buf;
+	shstrtab->elf_data->d_size = shstrtab->size = new_size;
+	shstrtab->sh.sh_size = new_size;
+
+	return 0;
+}
+
+static int update_strtab(struct elf *elf)
+{
+	struct section *strtab;
+	struct symbol *sym;
+	size_t orig_size, new_size = 0, offset, len;
+	char *buf;
+
+	strtab = find_section_by_name(elf, ".strtab");
+	if (!strtab) {
+		WARN("can't find .strtab");
+		return -1;
+	}
+
+	orig_size = new_size = strtab->size;
+
+	list_for_each_entry(sym, &elf->symbols, list) {
+		if (sym->sym.st_name != -1)
+			continue;
+		new_size += strlen(sym->name) + 1;
+	}
+
+	if (new_size == orig_size)
+		return 0;
+
+	buf = malloc(new_size);
+	if (!buf) {
+		WARN("malloc failed");
+		return -1;
+	}
+	memcpy(buf, (void *)strtab->data, orig_size);
+
+	offset = orig_size;
+	list_for_each_entry(sym, &elf->symbols, list) {
+		if (sym->sym.st_name != -1)
+			continue;
+		sym->sym.st_name = offset;
+		len = strlen(sym->name) + 1;
+		memcpy(buf + offset, sym->name, len);
+		offset += len;
+	}
+
+	strtab->elf_data->d_buf = strtab->data = buf;
+	strtab->elf_data->d_size = strtab->size = new_size;
+	strtab->sh.sh_size = new_size;
+
+	return 0;
+}
+
+static int update_symtab(struct elf *elf)
+{
+	struct section *symtab, *sec;
+	struct symbol *sym;
+	char *buf;
+	size_t size;
+	int nr_syms = 0, offset = 0, nr_locals = 0, idx;
+
+	idx = 0;
+	list_for_each_entry(sec, &elf->sections, list)
+		sec->idx = idx++;
+
+	idx = 0;
+	list_for_each_entry(sym, &elf->symbols, list) {
+		sym->idx = idx++;
+		if (sym->sec)
+			sym->sym.st_shndx = sym->sec->idx;
+	}
+
+	symtab = find_section_by_name(elf, ".symtab");
+	if (!symtab) {
+		WARN("can't find symtab");
+		return -1;
+	}
+
+	symtab->sh.sh_link = find_section_by_name(elf, ".strtab")->idx;
+
+	/* count symbols */
+	list_for_each_entry(sym, &elf->symbols, list)
+		nr_syms++;
+
+	/* create new symtab buffer */
+	size = nr_syms * symtab->sh.sh_entsize;
+	buf = malloc(size);
+	if (!buf) {
+		WARN("malloc failed");
+		return -1;
+	}
+	memset(buf, 0, size);
+
+	offset = 0;
+	list_for_each_entry(sym, &elf->symbols, list) {
+		memcpy(buf + offset, &sym->sym, symtab->sh.sh_entsize);
+		offset += symtab->sh.sh_entsize;
+
+		if (sym->bind == STB_LOCAL)
+			nr_locals++;
+	}
+
+	symtab->elf_data->d_buf = symtab->data = buf;
+	symtab->elf_data->d_size = symtab->size = size;
+	symtab->sh.sh_size = size;
+
+	/* update symtab section header */
+	symtab->sh.sh_info = nr_locals;
+
+	return 0;
+}
+
+static int update_relas(struct elf *elf)
+{
+	struct section *sec, *symtab;
+	struct rela *rela;
+	int nr_relas, idx, size;
+	GElf_Rela *relas;
+
+	symtab = find_section_by_name(elf, ".symtab");
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (!is_rela_section(sec))
+			continue;
+
+		sec->sh.sh_link = symtab->idx;
+		if (sec->base)
+			sec->sh.sh_info = sec->base->idx;
+
+		nr_relas = 0;
+		list_for_each_entry(rela, &sec->relas, list)
+			nr_relas++;
+
+		size = nr_relas * sizeof(*relas);
+		relas = malloc(size);
+		if (!relas) {
+			WARN("malloc failed");
+			return -1;
+		}
+
+		sec->elf_data->d_buf = sec->data = relas;
+		sec->elf_data->d_size = sec->size = size;
+		sec->sh.sh_size = size;
+
+		idx = 0;
+		list_for_each_entry(rela, &sec->relas, list) {
+			relas[idx].r_offset = rela->offset;
+			relas[idx].r_addend = rela->addend;
+			relas[idx].r_info = GELF_R_INFO(rela->sym->idx,
+							rela->type);
+			idx++;
+		}
+	}
+
+	return 0;
+}
+
+static int write_file(struct elf *elf, const char *file)
+{
+	int fd;
+	Elf *e;
+	GElf_Ehdr eh, ehout;
+	Elf_Scn *scn;
+	Elf_Data *data;
+	GElf_Shdr sh;
+	struct section *sec;
+
+	fd = creat(file, 0664);
+	if (fd == -1) {
+		WARN("couldn't create %s", file);
+		return -1;
+	}
+
+	e = elf_begin(fd, ELF_C_WRITE, NULL);
+	if (!e) {
+		WARN("elf_begin failed");
+		return -1;
+	}
+
+	if (!gelf_newehdr(e, gelf_getclass(elf->elf))) {
+		WARN("gelf_newehdr failed");
+		return -1;
+	}
+
+	if (!gelf_getehdr(e, &ehout)) {
+		WARN("gelf_getehdr failed");
+		return -1;
+	}
+
+	if (!gelf_getehdr(elf->elf, &eh)) {
+		WARN("gelf_getehdr failed");
+		return -1;
+	}
+
+	memset(&ehout, 0, sizeof(ehout));
+	ehout.e_ident[EI_DATA] = eh.e_ident[EI_DATA];
+	ehout.e_machine = eh.e_machine;
+	ehout.e_type = eh.e_type;
+	ehout.e_version = EV_CURRENT;
+	ehout.e_shstrndx = find_section_by_name(elf, ".shstrtab")->idx;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (!sec->idx)
+			continue;
+		scn = elf_newscn(e);
+		if (!scn) {
+			WARN("elf_newscn failed");
+			return -1;
+		}
+
+		data = elf_newdata(scn);
+		if (!data) {
+			WARN("elf_newdata failed");
+			return -1;
+		}
+
+		if (!elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY)) {
+			WARN("elf_flagdata failed");
+			return -1;
+		}
+
+		data->d_type = sec->elf_data->d_type;
+		data->d_buf = sec->elf_data->d_buf;
+		data->d_size = sec->elf_data->d_size;
+
+		if(!gelf_getshdr(scn, &sh)) {
+			WARN("gelf_getshdr failed");
+			return -1;
+		}
+
+		sh = sec->sh;
+
+		if (!gelf_update_shdr(scn, &sh)) {
+			WARN("gelf_update_shdr failed");
+			return -1;
+		}
+	}
+
+	if (!gelf_update_ehdr(e, &ehout)) {
+		WARN("gelf_update_ehdr failed");
+		return -1;
+	}
+
+	if (elf_update(e, ELF_C_WRITE) < 0) {
+		fprintf(stderr, "%s\n",elf_errmsg(-1));
+		WARN("elf_update failed");
+		return -1;
+	}
+
+	return 0;
+}
+
+int elf_write_file(struct elf *elf, const char *file)
+{
+	int ret;
+
+	ret = update_shstrtab(elf);
+	if (ret)
+		return ret;
+
+	ret = update_strtab(elf);
+	if (ret)
+		return ret;
+
+	ret = update_symtab(elf);
+	if (ret)
+		return ret;
+
+	ret = update_relas(elf);
+	if (ret)
+		return ret;
+
+	return write_file(elf, file);
+}
+
+struct elf *elf_open(const char *name)
+{
+	struct elf *elf;
+
+	elf_version(EV_CURRENT);
+
+	elf = malloc(sizeof(*elf));
+	if (!elf) {
+		perror("malloc");
+		return NULL;
+	}
+	memset(elf, 0, sizeof(*elf));
+
+	INIT_LIST_HEAD(&elf->sections);
+	INIT_LIST_HEAD(&elf->symbols);
+
+	elf->fd = open(name, O_RDONLY);
+	if (elf->fd == -1) {
+		perror("open");
+		goto err;
+	}
+
+	elf->elf = elf_begin(elf->fd, ELF_C_READ_MMAP, NULL);
+	if (!elf->elf) {
+		perror("elf_begin");
+		goto err;
+	}
+
+	if (!gelf_getehdr(elf->elf, &elf->ehdr)) {
+		perror("gelf_getehdr");
+		goto err;
+	}
+
+	if (read_sections(elf))
+		goto err;
+
+	if (read_symbols(elf))
+		goto err;
+
+	if (read_relas(elf))
+		goto err;
+
+	return elf;
+
+err:
+	elf_close(elf);
+	return NULL;
+}
+
+void elf_close(struct elf *elf)
+{
+	struct section *sec, *tmpsec;
+	struct symbol *sym, *tmpsym;
+	struct rela *rela, *tmprela;
+
+	list_for_each_entry_safe(sym, tmpsym, &elf->symbols, list) {
+		list_del(&sym->list);
+		free(sym);
+	}
+	list_for_each_entry_safe(sec, tmpsec, &elf->sections, list) {
+		list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+			list_del(&rela->list);
+			free(rela);
+		}
+		list_del(&sec->list);
+		free(sec);
+	}
+	if (elf->fd > 0)
+		close(elf->fd);
+	if (elf->elf)
+		elf_end(elf->elf);
+	free(elf);
+}
diff --git a/scripts/livepatch/elf.h b/scripts/livepatch/elf.h
new file mode 100644
index 0000000..f7bc676
--- /dev/null
+++ b/scripts/livepatch/elf.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2015-2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _KLP_MODPOST_ELF_H
+#define _KLP_MODPOST_ELF_H
+
+#include <stdio.h>
+#include <stdbool.h>
+#include <gelf.h>
+#include "list.h"
+
+#ifdef LIBELF_USE_DEPRECATED
+# define elf_getshdrnum    elf_getshnum
+# define elf_getshdrstrndx elf_getshstrndx
+#endif
+
+struct section {
+	struct list_head list;
+	GElf_Shdr sh;
+	struct section *base, *rela;
+	struct list_head relas;
+	struct symbol *sym;
+	Elf_Data *elf_data;
+	char *name;
+	int idx;
+	void *data;
+	unsigned int size;
+};
+
+struct symbol {
+	struct list_head list;
+	GElf_Sym sym;
+	struct section *sec;
+	char *name;
+	unsigned int idx;
+	unsigned char bind, type;
+	unsigned long offset;
+	unsigned int size;
+};
+
+struct rela {
+	struct list_head list;
+	GElf_Rela rela;
+	struct symbol *sym;
+	unsigned int type;
+	unsigned long offset;
+	int addend;
+};
+
+struct elf {
+	Elf *elf;
+	GElf_Ehdr ehdr;
+	int fd;
+	char *name;
+	struct list_head sections;
+	struct list_head symbols;
+};
+
+
+struct elf *elf_open(const char *name);
+bool is_rela_section(struct section *sec);
+struct section *find_section_by_name(struct elf *elf, const char *name);
+struct rela *find_rela_by_dest(struct section *sec, unsigned long offset);
+struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
+				     unsigned int len);
+
+struct section *create_rela_section(struct elf *elf, const char *name,
+				    struct section *base);
+
+void elf_close(struct elf *elf);
+int elf_write_file(struct elf *elf, const char *file);
+
+
+#endif /* _KLP_MODPOST_ELF_H */
diff --git a/scripts/livepatch/klp-convert.c b/scripts/livepatch/klp-convert.c
new file mode 100644
index 0000000..28a7282
--- /dev/null
+++ b/scripts/livepatch/klp-convert.c
@@ -0,0 +1,181 @@
+/*
+ * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include "elf.h"
+#include <linux/livepatch.h>
+
+#define WARN(format, ...) \
+	fprintf(stderr, "%s: " format "\n", elf->name, ##__VA_ARGS__)
+
+#define MODULE_NAME_LEN (64 - sizeof(GElf_Addr))
+
+#define SHN_LIVEPATCH		0xff20
+#define SHF_RELA_LIVEPATCH	0x00100000
+
+static const char usage_string[] =
+	"klp-convert <input.ko> <output.ko>";
+
+struct elf *elf;
+
+static struct section *find_or_create_klp_rela_section(char *objname,
+						       struct section *oldsec)
+{
+	char name[256];
+	struct section *sec;
+
+	if (snprintf(name, 256, KLP_RELA_PREFIX "%s.%s", objname,
+		     oldsec->base->name) >= 256) {
+		WARN("section name too long (%s)", oldsec->base->name);
+		return NULL;
+	}
+
+	sec = find_section_by_name(elf, name);
+	if (sec)
+		return sec;
+
+	sec = create_rela_section(elf, name, oldsec->base);
+	if (!sec)
+		return NULL;
+
+	sec->sh.sh_flags |= SHF_RELA_LIVEPATCH;
+	return sec;
+}
+
+static int rename_klp_symbols(struct section *sec, char *objname)
+{
+	struct section *relasec;
+	struct rela *rela, *tmprela;
+	struct klp_module_reloc *reloc;
+	int nr_entries, i;
+	char name[256];
+
+	relasec = sec->rela;
+	if (!relasec) {
+		WARN("section %s doesn't have a corresponding rela section",
+		     sec->name);
+		return -1;
+	}
+
+	if (list_empty(&relasec->relas)) {
+		WARN("section %s is empty", relasec->name);
+		return -1;
+	}
+
+	reloc = sec->data;
+	nr_entries = sec->size / sizeof(*reloc);
+
+	i = 0;
+	list_for_each_entry_safe(rela, tmprela, &relasec->relas, list) {
+
+		if (snprintf(name, 256, KLP_SYM_PREFIX "%s.%s,%d", objname,
+			     rela->sym->name, reloc[i].sympos) >= 256) {
+			WARN("symbol name too long (%s)", rela->sym->name);
+			return -1;
+		}
+
+		rela->sym->name = strdup(name);
+		rela->sym->sym.st_name = -1;
+		rela->sym->sec = NULL;
+		rela->sym->sym.st_shndx = SHN_LIVEPATCH;
+
+		list_del(&rela->list);
+		i++;
+	}
+
+	if (i != nr_entries) {
+		WARN("nr_entries mismatch (%d != %d) for %s\n",
+		     i, nr_entries, relasec->name);
+		return -1;
+	}
+
+	list_del(&relasec->list);
+	list_del(&sec->list);
+	list_del(&sec->sym->list);
+
+	return 0;
+}
+
+static int migrate_klp_rela(struct section *oldsec, struct rela *rela)
+{
+	char objname[MODULE_NAME_LEN];
+	struct section *newsec;
+
+	if (sscanf(rela->sym->name, KLP_SYM_PREFIX "%55[^.]", objname) != 1) {
+		WARN("bad format for klp rela %s", rela->sym->name);
+		return -1;
+	}
+
+	newsec = find_or_create_klp_rela_section(objname, oldsec);
+	if (!newsec)
+		return -1;
+
+	list_del(&rela->list);
+	list_add_tail(&rela->list, &newsec->relas);
+
+	return 0;
+}
+
+int main(int argc, const char **argv)
+{
+	const char *in_name, *out_name;
+	struct section *sec, *tmpsec;
+	char objname[MODULE_NAME_LEN];
+	struct rela *rela, *tmprela;
+
+	if (argc != 3) {
+		fprintf(stderr, "Usage: %s\n", usage_string);
+		return 1;
+	}
+
+	in_name = argv[1];
+	out_name = argv[2];
+
+	elf = elf_open(in_name);
+	if (!elf) {
+		fprintf(stderr, "error reading elf file %s\b", in_name);
+		return 1;
+	}
+
+	list_for_each_entry_safe(sec, tmpsec, &elf->sections, list) {
+		if (sscanf(sec->name, ".klp.module_relocs.%55s", objname) != 1)
+			continue;
+		if (rename_klp_symbols(sec, objname))
+			return 1;
+	}
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (!is_rela_section(sec))
+			continue;
+		if (!strncmp(sec->name, KLP_RELA_PREFIX,
+			     strlen(KLP_RELA_PREFIX)))
+			continue;
+		list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+			if (strncmp(rela->sym->name, KLP_SYM_PREFIX,
+				    strlen(KLP_SYM_PREFIX)))
+				continue;
+			if (migrate_klp_rela(sec, rela))
+				return 1;
+		}
+	}
+
+	if (elf_write_file(elf, out_name))
+		return 1;
+
+	return 0;
+}
diff --git a/scripts/livepatch/list.h b/scripts/livepatch/list.h
new file mode 100644
index 0000000..7f27e64
--- /dev/null
+++ b/scripts/livepatch/list.h
@@ -0,0 +1,381 @@
+#ifndef _LINUX_LIST_H
+#define _LINUX_LIST_H
+
+/*
+ * Simple doubly linked list implementation.
+ *
+ * Some of the internal functions ("__xxx") are useful when
+ * manipulating whole lists rather than single entries, as
+ * sometimes we already know the next/prev entries and we can
+ * generate better code by using them directly rather than
+ * using the generic single-entry routines.
+ */
+
+#define WRITE_ONCE(a, b) a = b
+#define READ_ONCE(a) a
+
+#undef offsetof
+#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+
+/**
+ * container_of - cast a member of a structure out to the containing structure
+ * @ptr:        the pointer to the member.
+ * @type:       the type of the container struct this is embedded in.
+ * @member:     the name of the member within the struct.
+ *
+ */
+#define container_of(ptr, type, member) ({                      \
+	const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
+	(type *)( (char *)__mptr - offsetof(type,member) );})
+
+struct list_head {
+	struct list_head *next, *prev;
+};
+
+#define LIST_HEAD_INIT(name) { &(name), &(name) }
+
+#define LIST_HEAD(name) \
+	struct list_head name = LIST_HEAD_INIT(name)
+
+static inline void INIT_LIST_HEAD(struct list_head *list)
+{
+	WRITE_ONCE(list->next, list);
+	list->prev = list;
+}
+
+/*
+ * Insert a new entry between two known consecutive entries.
+ *
+ * This is only for internal list manipulation where we know
+ * the prev/next entries already!
+ */
+static inline void __list_add(struct list_head *new,
+			      struct list_head *prev,
+			      struct list_head *next)
+{
+	next->prev = new;
+	new->next = next;
+	new->prev = prev;
+	WRITE_ONCE(prev->next, new);
+}
+
+/**
+ * list_add - add a new entry
+ * @new: new entry to be added
+ * @head: list head to add it after
+ *
+ * Insert a new entry after the specified head.
+ * This is good for implementing stacks.
+ */
+static inline void list_add(struct list_head *new, struct list_head *head)
+{
+	__list_add(new, head, head->next);
+}
+
+
+/**
+ * list_add_tail - add a new entry
+ * @new: new entry to be added
+ * @head: list head to add it before
+ *
+ * Insert a new entry before the specified head.
+ * This is useful for implementing queues.
+ */
+static inline void list_add_tail(struct list_head *new, struct list_head *head)
+{
+	__list_add(new, head->prev, head);
+}
+
+/*
+ * Delete a list entry by making the prev/next entries
+ * point to each other.
+ *
+ * This is only for internal list manipulation where we know
+ * the prev/next entries already!
+ */
+static inline void __list_del(struct list_head * prev, struct list_head * next)
+{
+	next->prev = prev;
+	WRITE_ONCE(prev->next, next);
+}
+
+/**
+ * list_del - deletes entry from list.
+ * @entry: the element to delete from the list.
+ * Note: list_empty() on entry does not return true after this, the entry is
+ * in an undefined state.
+ */
+static inline void __list_del_entry(struct list_head *entry)
+{
+	__list_del(entry->prev, entry->next);
+}
+
+static inline void list_del(struct list_head *entry)
+{
+	__list_del(entry->prev, entry->next);
+}
+
+/**
+ * list_is_last - tests whether @list is the last entry in list @head
+ * @list: the entry to test
+ * @head: the head of the list
+ */
+static inline int list_is_last(const struct list_head *list,
+				const struct list_head *head)
+{
+	return list->next == head;
+}
+
+/**
+ * list_empty - tests whether a list is empty
+ * @head: the list to test.
+ */
+static inline int list_empty(const struct list_head *head)
+{
+	return READ_ONCE(head->next) == head;
+}
+
+/**
+ * list_entry - get the struct for this entry
+ * @ptr:	the &struct list_head pointer.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_entry(ptr, type, member) \
+	container_of(ptr, type, member)
+
+/**
+ * list_first_entry - get the first element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note, that list is expected to be not empty.
+ */
+#define list_first_entry(ptr, type, member) \
+	list_entry((ptr)->next, type, member)
+
+/**
+ * list_last_entry - get the last element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note, that list is expected to be not empty.
+ */
+#define list_last_entry(ptr, type, member) \
+	list_entry((ptr)->prev, type, member)
+
+/**
+ * list_first_entry_or_null - get the first element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note that if the list is empty, it returns NULL.
+ */
+#define list_first_entry_or_null(ptr, type, member) \
+	(!list_empty(ptr) ? list_first_entry(ptr, type, member) : NULL)
+
+/**
+ * list_next_entry - get the next element in list
+ * @pos:	the type * to cursor
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_next_entry(pos, member) \
+	list_entry((pos)->member.next, typeof(*(pos)), member)
+
+/**
+ * list_prev_entry - get the prev element in list
+ * @pos:	the type * to cursor
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_prev_entry(pos, member) \
+	list_entry((pos)->member.prev, typeof(*(pos)), member)
+
+/**
+ * list_for_each	-	iterate over a list
+ * @pos:	the &struct list_head to use as a loop cursor.
+ * @head:	the head for your list.
+ */
+#define list_for_each(pos, head) \
+	for (pos = (head)->next; pos != (head); pos = pos->next)
+
+/**
+ * list_for_each_prev	-	iterate over a list backwards
+ * @pos:	the &struct list_head to use as a loop cursor.
+ * @head:	the head for your list.
+ */
+#define list_for_each_prev(pos, head) \
+	for (pos = (head)->prev; pos != (head); pos = pos->prev)
+
+/**
+ * list_for_each_safe - iterate over a list safe against removal of list entry
+ * @pos:	the &struct list_head to use as a loop cursor.
+ * @n:		another &struct list_head to use as temporary storage
+ * @head:	the head for your list.
+ */
+#define list_for_each_safe(pos, n, head) \
+	for (pos = (head)->next, n = pos->next; pos != (head); \
+		pos = n, n = pos->next)
+
+/**
+ * list_for_each_prev_safe - iterate over a list backwards safe against removal of list entry
+ * @pos:	the &struct list_head to use as a loop cursor.
+ * @n:		another &struct list_head to use as temporary storage
+ * @head:	the head for your list.
+ */
+#define list_for_each_prev_safe(pos, n, head) \
+	for (pos = (head)->prev, n = pos->prev; \
+	     pos != (head); \
+	     pos = n, n = pos->prev)
+
+/**
+ * list_for_each_entry	-	iterate over list of given type
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_for_each_entry(pos, head, member)				\
+	for (pos = list_first_entry(head, typeof(*pos), member);	\
+	     &pos->member != (head);					\
+	     pos = list_next_entry(pos, member))
+
+/**
+ * list_for_each_entry_reverse - iterate backwards over list of given type.
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_for_each_entry_reverse(pos, head, member)			\
+	for (pos = list_last_entry(head, typeof(*pos), member);		\
+	     &pos->member != (head); 					\
+	     pos = list_prev_entry(pos, member))
+
+/**
+ * list_prepare_entry - prepare a pos entry for use in list_for_each_entry_continue()
+ * @pos:	the type * to use as a start point
+ * @head:	the head of the list
+ * @member:	the name of the list_head within the struct.
+ *
+ * Prepares a pos entry for use as a start point in list_for_each_entry_continue().
+ */
+#define list_prepare_entry(pos, head, member) \
+	((pos) ? : list_entry(head, typeof(*pos), member))
+
+/**
+ * list_for_each_entry_continue - continue iteration over list of given type
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Continue to iterate over list of given type, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue(pos, head, member) 		\
+	for (pos = list_next_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = list_next_entry(pos, member))
+
+/**
+ * list_for_each_entry_continue_reverse - iterate backwards from the given point
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Start to iterate over list of given type backwards, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_reverse(pos, head, member)		\
+	for (pos = list_prev_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = list_prev_entry(pos, member))
+
+/**
+ * list_for_each_entry_from - iterate over list of given type from the current point
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Iterate over list of given type, continuing from current position.
+ */
+#define list_for_each_entry_from(pos, head, member) 			\
+	for (; &pos->member != (head);					\
+	     pos = list_next_entry(pos, member))
+
+/**
+ * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry
+ * @pos:	the type * to use as a loop cursor.
+ * @n:		another type * to use as temporary storage
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_for_each_entry_safe(pos, n, head, member)			\
+	for (pos = list_first_entry(head, typeof(*pos), member),	\
+		n = list_next_entry(pos, member);			\
+	     &pos->member != (head); 					\
+	     pos = n, n = list_next_entry(n, member))
+
+/**
+ * list_for_each_entry_safe_continue - continue list iteration safe against removal
+ * @pos:	the type * to use as a loop cursor.
+ * @n:		another type * to use as temporary storage
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Iterate over list of given type, continuing after current point,
+ * safe against removal of list entry.
+ */
+#define list_for_each_entry_safe_continue(pos, n, head, member) 		\
+	for (pos = list_next_entry(pos, member), 				\
+		n = list_next_entry(pos, member);				\
+	     &pos->member != (head);						\
+	     pos = n, n = list_next_entry(n, member))
+
+/**
+ * list_for_each_entry_safe_from - iterate over list from current point safe against removal
+ * @pos:	the type * to use as a loop cursor.
+ * @n:		another type * to use as temporary storage
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Iterate over list of given type from current point, safe against
+ * removal of list entry.
+ */
+#define list_for_each_entry_safe_from(pos, n, head, member) 			\
+	for (n = list_next_entry(pos, member);					\
+	     &pos->member != (head);						\
+	     pos = n, n = list_next_entry(n, member))
+
+/**
+ * list_for_each_entry_safe_reverse - iterate backwards over list safe against removal
+ * @pos:	the type * to use as a loop cursor.
+ * @n:		another type * to use as temporary storage
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Iterate backwards over list of given type, safe against removal
+ * of list entry.
+ */
+#define list_for_each_entry_safe_reverse(pos, n, head, member)		\
+	for (pos = list_last_entry(head, typeof(*pos), member),		\
+		n = list_prev_entry(pos, member);			\
+	     &pos->member != (head); 					\
+	     pos = n, n = list_prev_entry(n, member))
+
+/**
+ * list_safe_reset_next - reset a stale list_for_each_entry_safe loop
+ * @pos:	the loop cursor used in the list_for_each_entry_safe loop
+ * @n:		temporary storage used in list_for_each_entry_safe
+ * @member:	the name of the list_head within the struct.
+ *
+ * list_safe_reset_next is not safe to use in general if the list may be
+ * modified concurrently (eg. the lock is dropped in the loop body). An
+ * exception to this is if the cursor element (pos) is pinned in the list,
+ * and list_safe_reset_next is called after re-taking the lock and before
+ * completing the current iteration of the loop body.
+ */
+#define list_safe_reset_next(pos, n, member)				\
+	n = list_next_entry(pos, member)
+
+#endif
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index bd83497..f1a9bbc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1960,6 +1960,13 @@ static void read_symbols(char *modname)
 					   "license", license);
 	}
 
+	/*
+	 * For livepatch modules, any unresolved symbols will get resolved
+	 * after linking by klp-convert.
+	 */
+	if (get_modinfo(info.modinfo, info.modinfo_len, "livepatch"))
+		mod->livepatch = 1;
+
 	for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
 		symname = remove_dot(info.strtab + sym->st_name);
 
@@ -2153,7 +2160,7 @@ static int add_versions(struct buffer *b, struct module *mod)
 	for (s = mod->unres; s; s = s->next) {
 		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);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 6a5e151..dcb32a5 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -117,6 +117,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.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFC 2/2] livepatch: update sample module to use klp relocations
  2016-10-27 14:35 [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
  2016-10-27 14:35 ` [PATCH RFC 1/2] livepatch: add klp-convert tool and integrate with module build Josh Poimboeuf
@ 2016-10-27 14:35 ` Josh Poimboeuf
  2016-11-10 15:36 ` [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
  2017-03-01 10:53 ` Miroslav Benes
  3 siblings, 0 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2016-10-27 14:35 UTC (permalink / raw)
  To: live-patching; +Cc: linux-kernel

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 samples/livepatch/livepatch-sample.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
index e34f871..478d102 100644
--- a/samples/livepatch/livepatch-sample.c
+++ b/samples/livepatch/livepatch-sample.c
@@ -32,7 +32,7 @@
  *
  * $ insmod livepatch-sample.ko
  * $ cat /proc/cmdline
- * this has been live patched
+ * <your cmdline> livepatch=1
  *
  * $ echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled
  * $ cat /proc/cmdline
@@ -42,11 +42,18 @@
 #include <linux/seq_file.h>
 static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
 {
-	seq_printf(m, "%s\n", "this has been live patched");
+	seq_printf(m, "%s livepatch=1\n", saved_command_line);
 	return 0;
 }
 
-static struct klp_func funcs[] = {
+struct KLP_MODULE_RELOC(vmlinux) vmlinux_relocs[] = {
+	{
+		.sym = &saved_command_line,
+		.sympos = 0,
+	},
+};
+
+static struct klp_func vmlinux_funcs[] = {
 	{
 		.old_name = "cmdline_proc_show",
 		.new_func = livepatch_cmdline_proc_show,
@@ -56,7 +63,7 @@ static struct klp_func funcs[] = {
 static struct klp_object objs[] = {
 	{
 		/* name being NULL means vmlinux */
-		.funcs = funcs,
+		.funcs = vmlinux_funcs,
 	}, { }
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 0/2] livepatch: patch creation tooling proposal
  2016-10-27 14:35 [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
  2016-10-27 14:35 ` [PATCH RFC 1/2] livepatch: add klp-convert tool and integrate with module build Josh Poimboeuf
  2016-10-27 14:35 ` [PATCH RFC 2/2] livepatch: update sample module to use klp relocations Josh Poimboeuf
@ 2016-11-10 15:36 ` Josh Poimboeuf
  2016-11-10 15:48   ` Miroslav Benes
  2017-03-01 10:53 ` Miroslav Benes
  3 siblings, 1 reply; 10+ messages in thread
From: Josh Poimboeuf @ 2016-11-10 15:36 UTC (permalink / raw)
  To: live-patching; +Cc: linux-kernel

On Thu, Oct 27, 2016 at 09:35:48AM -0500, Josh Poimboeuf wrote:
> So here's my proposal: use the existing kernel build infrastructure.  If
> klp relocations are needed, manually specify them with a new
> klp_module_reloc struct and corresponding KLP_MODULE_RELOC macro.  Then
> run a post-processing tool called klp-convert which converts those
> klp_module_reloc structs into the sections, relocations, and symbols
> needed by the klp runtime code.

I think the biggest blocker for this approach is detecting gcc
optimizations which break function ABI, i.e. Miroslav's presentation:

  http://www.linuxplumbersconf.org/2016/ocw//system/presentations/3573/original/pres_gcc.pdf

Right now we have no way of finding all such cases.

I think our options are:

1) Find a way for gcc to report when function ABI has been broken;

2) Disable all gcc optimizations which can break function ABI.  Not sure
   if this is even possible, but if so, we'd need to quantify the
   performance impact.  (Note we might be able to leave some options
   enabled if they result in a function name change (e.g.,
   -fpartial-inlining, -fipa-sra, -fipa-cp)); or

3) Stay with the status quo (kpatch-build?), since it has detection of
   such optimizations "built in".

Does anybody want to take ownership of this patch set and/or try to
explore the options further?  I don't have any more bandwidth right now
(mainly due to the consistency model and porting objtool to DWARF).

-- 
Josh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 0/2] livepatch: patch creation tooling proposal
  2016-11-10 15:36 ` [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
@ 2016-11-10 15:48   ` Miroslav Benes
  2016-11-10 15:54     ` Miroslav Benes
  2016-11-10 16:10     ` Josh Poimboeuf
  0 siblings, 2 replies; 10+ messages in thread
From: Miroslav Benes @ 2016-11-10 15:48 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: live-patching, Linux Kernel Mailing List, Joerg Roedel, jkosina,
	jeyu, pmladek

On Thu, 10 Nov 2016, Josh Poimboeuf wrote:

> On Thu, Oct 27, 2016 at 09:35:48AM -0500, Josh Poimboeuf wrote:
> > So here's my proposal: use the existing kernel build infrastructure.  If
> > klp relocations are needed, manually specify them with a new
> > klp_module_reloc struct and corresponding KLP_MODULE_RELOC macro.  Then
> > run a post-processing tool called klp-convert which converts those
> > klp_module_reloc structs into the sections, relocations, and symbols
> > needed by the klp runtime code.
> 
> I think the biggest blocker for this approach is detecting gcc
> optimizations which break function ABI, i.e. Miroslav's presentation:
> 
>   http://www.linuxplumbersconf.org/2016/ocw//system/presentations/3573/original/pres_gcc.pdf
> 
> Right now we have no way of finding all such cases.
> 
> I think our options are:
> 
> 1) Find a way for gcc to report when function ABI has been broken;

This is the one I'd like to pursue in parallel to 3). But it is 
going to be long way I imagine.

> 2) Disable all gcc optimizations which can break function ABI.  Not sure
>    if this is even possible, but if so, we'd need to quantify the
>    performance impact.  (Note we might be able to leave some options
>    enabled if they result in a function name change (e.g.,
>    -fpartial-inlining, -fipa-sra, -fipa-cp)); or

I don't think this is possible. I mean technically possible, because 
I'm almost sure some optimizations cannot be disabled easily. And also 
performance-wise. It could have a serious impact on the kernel with 
CONFIG_LIVEPATCH enabled.

I consider this option a last resort.
 
> 3) Stay with the status quo (kpatch-build?), since it has detection of
>    such optimizations "built in".

Also possible. We could explore the usability of Joerg's asmtool for the 
purpose.

https://github.com/joergroedel/asmtool

It could be useful even if for the detection of changed functions.

> Does anybody want to take ownership of this patch set and/or try to
> explore the options further?  I don't have any more bandwidth right now
> (mainly due to the consistency model and porting objtool to DWARF).

Sure. I can take it. I tried to write a similar tool, I saw kpatch-build 
sources and have a clue how it all works. On the other hand, no promises 
about a timeline.

Miroslav

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 0/2] livepatch: patch creation tooling proposal
  2016-11-10 15:48   ` Miroslav Benes
@ 2016-11-10 15:54     ` Miroslav Benes
  2016-11-10 16:10     ` Josh Poimboeuf
  1 sibling, 0 replies; 10+ messages in thread
From: Miroslav Benes @ 2016-11-10 15:54 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: live-patching, Linux Kernel Mailing List, Joerg Roedel, jkosina,
	jeyu, pmladek

On Thu, 10 Nov 2016, Miroslav Benes wrote:

> On Thu, 10 Nov 2016, Josh Poimboeuf wrote:
> 
> > Does anybody want to take ownership of this patch set and/or try to
> > explore the options further?  I don't have any more bandwidth right now
> > (mainly due to the consistency model and porting objtool to DWARF).
> 
> Sure. I can take it. I tried to write a similar tool, I saw kpatch-build 
> sources and have a clue how it all works. On the other hand, no promises 
> about a timeline.

To be precise... I'm really happy to take it. It is the top of my 
priority list for upstream livepatch. Only I get stuck with things as part 
of my day job once in a while :)

Miroslav

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 0/2] livepatch: patch creation tooling proposal
  2016-11-10 15:48   ` Miroslav Benes
  2016-11-10 15:54     ` Miroslav Benes
@ 2016-11-10 16:10     ` Josh Poimboeuf
  1 sibling, 0 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2016-11-10 16:10 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: live-patching, Linux Kernel Mailing List, Joerg Roedel, jkosina,
	jeyu, pmladek

On Thu, Nov 10, 2016 at 04:48:33PM +0100, Miroslav Benes wrote:
> On Thu, 10 Nov 2016, Josh Poimboeuf wrote:
> 
> > On Thu, Oct 27, 2016 at 09:35:48AM -0500, Josh Poimboeuf wrote:
> > > So here's my proposal: use the existing kernel build infrastructure.  If
> > > klp relocations are needed, manually specify them with a new
> > > klp_module_reloc struct and corresponding KLP_MODULE_RELOC macro.  Then
> > > run a post-processing tool called klp-convert which converts those
> > > klp_module_reloc structs into the sections, relocations, and symbols
> > > needed by the klp runtime code.
> > 
> > I think the biggest blocker for this approach is detecting gcc
> > optimizations which break function ABI, i.e. Miroslav's presentation:
> > 
> >   http://www.linuxplumbersconf.org/2016/ocw//system/presentations/3573/original/pres_gcc.pdf
> > 
> > Right now we have no way of finding all such cases.
> > 
> > I think our options are:
> > 
> > 1) Find a way for gcc to report when function ABI has been broken;
> 
> This is the one I'd like to pursue in parallel to 3). But it is 
> going to be long way I imagine.

Yes, the gcc folks I've talked to seem to agree.

> > 2) Disable all gcc optimizations which can break function ABI.  Not sure
> >    if this is even possible, but if so, we'd need to quantify the
> >    performance impact.  (Note we might be able to leave some options
> >    enabled if they result in a function name change (e.g.,
> >    -fpartial-inlining, -fipa-sra, -fipa-cp)); or
> 
> I don't think this is possible. I mean technically possible, because 
> I'm almost sure some optimizations cannot be disabled easily. And also 
> performance-wise. It could have a serious impact on the kernel with 
> CONFIG_LIVEPATCH enabled.
> 
> I consider this option a last resort.

I have some doubts about whether it would noticeably impact performance.
As far as I can tell these optimizations are quite rare.

FWIW, I've asked some gcc folks about the feasibility of something like
a '-fpreserve-function-abi' option.  But I'm not holding my breath.  It
seems like a hard problem.

> > 3) Stay with the status quo (kpatch-build?), since it has detection of
> >    such optimizations "built in".
> 
> Also possible. We could explore the usability of Joerg's asmtool for the 
> purpose.
> 
> https://github.com/joergroedel/asmtool
> 
> It could be useful even if for the detection of changed functions.
> 
> > Does anybody want to take ownership of this patch set and/or try to
> > explore the options further?  I don't have any more bandwidth right now
> > (mainly due to the consistency model and porting objtool to DWARF).
> 
> Sure. I can take it. I tried to write a similar tool, I saw kpatch-build 
> sources and have a clue how it all works. On the other hand, no promises 
> about a timeline.

Great, thanks!  I think your experiences with the gcc optimizations and
with the various patch building tools make you a great candidate for
this.

-- 
Josh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] livepatch: fixup klp-convert tool integration
  2016-10-27 14:35 ` [PATCH RFC 1/2] livepatch: add klp-convert tool and integrate with module build Josh Poimboeuf
@ 2016-12-17 13:08   ` Konstantin Khlebnikov
  2016-12-19 14:29     ` Miroslav Benes
  0 siblings, 1 reply; 10+ messages in thread
From: Konstantin Khlebnikov @ 2016-12-17 13:08 UTC (permalink / raw)
  To: live-patching, Josh Poimboeuf; +Cc: linux-kernel

I've found some minor problems, this patch fixes:

* save cmd_ld_ko_o into .module.cmd, if_changed_rule doesn't do that
* fix bashisms for debian where /bin/sh is a symlink to /bin/dash
* rename rule_link_module to rule_ld_ko_o, otherwise arg-check inside
  if_changed_rule compares cmd_link_module and cmd_ld_ko_o
* use HOSTLOADLIBES_$module instead of HOSTLDFLAGS: -lelf must be at the end
* check modinfo -F livepatch only if CONFIG_LIVEPATCH is true

I think "modinfo -F" could be replaced with explicit mark in makefile,
for example: LIVEPATCH_module.ko := y (like KASAN_SANITIZE_obj.o := n).

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 scripts/Kbuild.include     |    4 +++-
 scripts/Makefile.modpost   |   24 +++++++++++-------------
 scripts/livepatch/Makefile |    2 +-
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 179219845dfc..e299fde3423b 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -247,6 +247,8 @@ endif
 # (needed for the shell)
 make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
 
+save-cmd = printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
+
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.
 any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
@@ -256,7 +258,7 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
 if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
 	@set -e;                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
-	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+	$(save-cmd), @:)
 
 # Execute the command and also postprocess generated .d dependencies file.
 if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 916dd347e8f6..5d149d0b05c2 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -123,24 +123,22 @@ quiet_cmd_ld_ko_o = LD [M]  $@
 	$(LD) -r $(LDFLAGS)                                             \
                  $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)             \
                  -o $@ $(filter-out FORCE,$^) ;                         \
-	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) ;
+	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
-ifdef CONFIG_LIVEPATCH
 KLP_CONVERT = scripts/livepatch/klp-convert
-      cmd_klp_convert =							\
-	if [[ -n "`modinfo -F livepatch $@`" ]]; then			\
-		mv $@ $(@:.ko=.klp.o);					\
-		$(KLP_CONVERT) $(@:.ko=.klp.o) $@;			\
-	fi ;
-endif
-
-define rule_link_module
-	$(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o)				\
-	$(cmd_klp_convert)
+quiet_cmd_klp_convert = LIVEPATCH $@
+      cmd_klp_convert = mv $@ $(@:.ko=.klp.o); $(KLP_CONVERT) $(@:.ko=.klp.o) $@
+
+define rule_ld_ko_o
+       $(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o) ;                        \
+       $(call save-cmd,ld_ko_o) ;                                       \
+       $(if $(CONFIG_LIVEPATCH),                                        \
+         if [ -n "`modinfo -F livepatch $@`" ] ; then                   \
+           $(call echo-cmd,klp_convert) $(cmd_klp_convert) ; fi)
 endef
 
 $(modules): %.ko :%.o %.mod.o FORCE
-	+$(call if_changed_rule,link_module)
+	+$(call if_changed_rule,ld_ko_o)
 
 targets += $(modules)
 
diff --git a/scripts/livepatch/Makefile b/scripts/livepatch/Makefile
index 221829bb34c7..bd5c1ae553ab 100644
--- a/scripts/livepatch/Makefile
+++ b/scripts/livepatch/Makefile
@@ -4,4 +4,4 @@ always			:= $(hostprogs-y)
 klp-convert-objs	:= klp-convert.o elf.o
 
 HOSTCFLAGS		:= -g -I$(INSTALL_HDR_PATH)/include -Wall
-HOSTLDFLAGS		:= -lelf
+HOSTLOADLIBES_klp-convert	:= -lelf

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] livepatch: fixup klp-convert tool integration
  2016-12-17 13:08   ` [PATCH] livepatch: fixup klp-convert tool integration Konstantin Khlebnikov
@ 2016-12-19 14:29     ` Miroslav Benes
  0 siblings, 0 replies; 10+ messages in thread
From: Miroslav Benes @ 2016-12-19 14:29 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: live-patching, Josh Poimboeuf, Linux Kernel Mailing List

On Sat, 17 Dec 2016, Konstantin Khlebnikov wrote:

> I've found some minor problems, this patch fixes:
> 
> * save cmd_ld_ko_o into .module.cmd, if_changed_rule doesn't do that
> * fix bashisms for debian where /bin/sh is a symlink to /bin/dash
> * rename rule_link_module to rule_ld_ko_o, otherwise arg-check inside
>   if_changed_rule compares cmd_link_module and cmd_ld_ko_o
> * use HOSTLOADLIBES_$module instead of HOSTLDFLAGS: -lelf must be at the end

Ok, why not. I'll incorporate these changes to the patch set, if you're ok 
with it.

> * check modinfo -F livepatch only if CONFIG_LIVEPATCH is true

Right, this was the case with Josh's changes too if I understand modpost 
correctly (still causing me headaches).
 
> I think "modinfo -F" could be replaced with explicit mark in makefile,
> for example: LIVEPATCH_module.ko := y (like KASAN_SANITIZE_obj.o := n).

Yes, replacing modinfo is definitely something I plan to do and explicit 
mark in Makefile is one option how to deal with it. I also though about a 
mark somewhere in a source code, but it does not matter much, I guess.

Thanks,
Miroslav

> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
>  scripts/Kbuild.include     |    4 +++-
>  scripts/Makefile.modpost   |   24 +++++++++++-------------
>  scripts/livepatch/Makefile |    2 +-
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 179219845dfc..e299fde3423b 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -247,6 +247,8 @@ endif
>  # (needed for the shell)
>  make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
>  
> +save-cmd = printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
> +
>  # Find any prerequisites that is newer than target or that does not exist.
>  # PHONY targets skipped in both cases.
>  any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
> @@ -256,7 +258,7 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
>  if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
>  	@set -e;                                                             \
>  	$(echo-cmd) $(cmd_$(1));                                             \
> -	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> +	$(save-cmd), @:)
>  
>  # Execute the command and also postprocess generated .d dependencies file.
>  if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 916dd347e8f6..5d149d0b05c2 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -123,24 +123,22 @@ quiet_cmd_ld_ko_o = LD [M]  $@
>  	$(LD) -r $(LDFLAGS)                                             \
>                   $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)             \
>                   -o $@ $(filter-out FORCE,$^) ;                         \
> -	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) ;
> +	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>  
> -ifdef CONFIG_LIVEPATCH
>  KLP_CONVERT = scripts/livepatch/klp-convert
> -      cmd_klp_convert =							\
> -	if [[ -n "`modinfo -F livepatch $@`" ]]; then			\
> -		mv $@ $(@:.ko=.klp.o);					\
> -		$(KLP_CONVERT) $(@:.ko=.klp.o) $@;			\
> -	fi ;
> -endif
> -
> -define rule_link_module
> -	$(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o)				\
> -	$(cmd_klp_convert)
> +quiet_cmd_klp_convert = LIVEPATCH $@
> +      cmd_klp_convert = mv $@ $(@:.ko=.klp.o); $(KLP_CONVERT) $(@:.ko=.klp.o) $@
> +
> +define rule_ld_ko_o
> +       $(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o) ;                        \
> +       $(call save-cmd,ld_ko_o) ;                                       \
> +       $(if $(CONFIG_LIVEPATCH),                                        \
> +         if [ -n "`modinfo -F livepatch $@`" ] ; then                   \
> +           $(call echo-cmd,klp_convert) $(cmd_klp_convert) ; fi)
>  endef
>  
>  $(modules): %.ko :%.o %.mod.o FORCE
> -	+$(call if_changed_rule,link_module)
> +	+$(call if_changed_rule,ld_ko_o)
>  
>  targets += $(modules)
>  
> diff --git a/scripts/livepatch/Makefile b/scripts/livepatch/Makefile
> index 221829bb34c7..bd5c1ae553ab 100644
> --- a/scripts/livepatch/Makefile
> +++ b/scripts/livepatch/Makefile
> @@ -4,4 +4,4 @@ always			:= $(hostprogs-y)
>  klp-convert-objs	:= klp-convert.o elf.o
>  
>  HOSTCFLAGS		:= -g -I$(INSTALL_HDR_PATH)/include -Wall
> -HOSTLDFLAGS		:= -lelf
> +HOSTLOADLIBES_klp-convert	:= -lelf
> 
> --
> To unsubscribe from this list: send the line "unsubscribe live-patching" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 0/2] livepatch: patch creation tooling proposal
  2016-10-27 14:35 [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
                   ` (2 preceding siblings ...)
  2016-11-10 15:36 ` [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
@ 2017-03-01 10:53 ` Miroslav Benes
  3 siblings, 0 replies; 10+ messages in thread
From: Miroslav Benes @ 2017-03-01 10:53 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: live-patching, Linux Kernel Mailing List, jmoreira

On Thu, 27 Oct 2016, Josh Poimboeuf wrote:

> TODO:
> - support .klp.arch.objname..altinstructions/parainstructions
> - split up the patches better
> - patch creation documentation
> - more tooling support for detecting missing relocations?  or
>   automatically converting them if a sympos isn't needed?

Hi Josh,

I owe you (and others) update...

- I split up the patch set (but it is still not nice enough)
- Konstantin's changes are merged
- I removed a dependency on modinfo and made some changes

But much more importantly. Joao (CC'd) took it over from now on. He can 
help us with userspace tooling in the future too. I am sure there is 
going to be faster progress with it now, because something has always 
distracted me recently :/ and it is important to finish it.

Regards,
Miroslav

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-03-01 10:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 14:35 [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
2016-10-27 14:35 ` [PATCH RFC 1/2] livepatch: add klp-convert tool and integrate with module build Josh Poimboeuf
2016-12-17 13:08   ` [PATCH] livepatch: fixup klp-convert tool integration Konstantin Khlebnikov
2016-12-19 14:29     ` Miroslav Benes
2016-10-27 14:35 ` [PATCH RFC 2/2] livepatch: update sample module to use klp relocations Josh Poimboeuf
2016-11-10 15:36 ` [PATCH RFC 0/2] livepatch: patch creation tooling proposal Josh Poimboeuf
2016-11-10 15:48   ` Miroslav Benes
2016-11-10 15:54     ` Miroslav Benes
2016-11-10 16:10     ` Josh Poimboeuf
2017-03-01 10:53 ` Miroslav Benes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).