[PATCH 0/8] livepatch: klp-convert tool

[PATCH 0/8] livepatch: klp-convert tool

[Joao Moreira]

Livepatches may use symbols which are not contained in its own scope,
and, because of that, may end up compiled with relocations that will
only be resolved during module load. Yet, when the referenced symbols are
not exported, solving this relocation requires information on the object
that holds the symbol (either vmlinux or modules) and its position inside
the object, as an object may contain multiple symbols with the same name.
Providing such information must be done accordingly to what is specified
in Documentation/livepatch/module-elf-format.txt.

Currently, there is no trivial way to embed the required information as
requested in the final livepatch elf object. klp-convert solves this
problem in two different forms: (i) by relying on a symbol map, which is
built during kernel compilation, to automatically infers the relocation
targeted symbol, and, when such inference is not possible (ii) by using
annotations in the elf object to convert the relocation accordingly to
the specification, enabling it to be handled by the livepatch loader.

Given the above, add support for symbol mapping in the form of
Symbols.list file; add klp-convert tool; integrate klp-convert tool into
kbuild; make livepatch modules discernible during kernel compilation
pipeline; add data-structure and macros to enable users to annotate
livepatch source code; make modpost stage compatible with livepatches;
update livepatch-sample and update documentation.

The patch was tested under three use-cases:

use-case 1: There is a relocation in the lp that can be automatically
resolved by klp-convert (tested by removing the annotations from
samples/livepatch/livepatch-annotated-sample.c)

use-case 2: There is a relocation in the lp that cannot be automatically
resolved, as the name of the respective symbol appears in multiple
objects. The livepatch contains an annotation to enable a correct
relocation - reproducible with this livepatch sample:
www.livewire.com.br/suse/klp/livepatch-sample.1.c

use-case 3: There is a relocation in the lp that cannot be automatically
resolved similarly as 2, but no annotation was provided in the livepatch,
triggering an error during compilation - reproducible with this livepatch
sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c

Joao Moreira (2):
  kbuild: Support for Symbols.list creation
  documentation: Update on livepatch elf format

Josh Poimboeuf (5):
  livepatch: Create and include UAPI headers
  livepatch: Add klp-convert tool
  livepatch: Add klp-convert annotation helpers
  modpost: Integrate klp-convert
  livepatch: Add sample livepatch module

Miroslav Benes (1):
  modpost: Add modinfo flag to livepatch modules

 .gitignore                                     |   1 +
 Documentation/livepatch/module-elf-format.txt  |  47 +-
 MAINTAINERS                                    |   2 +
 Makefile                                       |  29 +-
 include/linux/livepatch.h                      |  12 +
 include/uapi/linux/livepatch.h                 |  33 ++
 kernel/livepatch/core.c                        |   4 +-
 samples/livepatch/Makefile                     |   5 +-
 samples/livepatch/livepatch-annotated-sample.c | 128 +++++
 samples/livepatch/livepatch-sample.c           |   1 -
 scripts/Kbuild.include                         |   4 +-
 scripts/Makefile                               |   1 +
 scripts/Makefile.build                         |   6 +
 scripts/Makefile.modpost                       |  24 +-
 scripts/livepatch/.gitignore                   |   1 +
 scripts/livepatch/Makefile                     |   7 +
 scripts/livepatch/elf.c                        | 696 +++++++++++++++++++++++++
 scripts/livepatch/elf.h                        |  84 +++
 scripts/livepatch/klp-convert.c                | 567 ++++++++++++++++++++
 scripts/livepatch/list.h                       | 389 ++++++++++++++
 scripts/mod/modpost.c                          |  80 ++-
 scripts/mod/modpost.h                          |   1 +
 22 files changed, 2104 insertions(+), 18 deletions(-)
 create mode 100644 include/uapi/linux/livepatch.h
 create mode 100644 samples/livepatch/livepatch-annotated-sample.c
 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.12.0

[PATCH 1/8] livepatch: Create and include UAPI headers

[Joao Moreira]

From: Josh Poimboeuf <jpoimboe@redhat.com>

Define klp prefixes in include/uapi/linux/livepatch.h, and use them for
replacing hard-coded values in kernel/livepatch/core.c.

Update MAINTAINERS.

Note: Add defines to uapi as these are also to be used by klp-convert, a
userspace tool that will be added in patch 03/07 of this same patch set.

[jmoreira: split up and changelog]

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 MAINTAINERS                    |  1 +
 include/linux/livepatch.h      |  1 +
 include/uapi/linux/livepatch.h | 28 ++++++++++++++++++++++++++++
 kernel/livepatch/core.c        |  4 ++--
 4 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 include/uapi/linux/livepatch.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 09b5ab6a8a5c..1d8ea36e1173 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7713,6 +7713,7 @@ R:	Petr Mladek <pmladek@suse.com>
 S:	Maintained
 F:	kernel/livepatch/
 F:	include/linux/livepatch.h
+F:	include/uapi/linux/livepatch.h
 F:	arch/x86/include/asm/livepatch.h
 F:	arch/x86/kernel/livepatch.c
 F:	Documentation/livepatch/
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 194991ef9347..96a75be7ef50 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <linux/ftrace.h>
 #include <linux/completion.h>
+#include <uapi/linux/livepatch.h>
 
 #if IS_ENABLED(CONFIG_LIVEPATCH)
 
diff --git a/include/uapi/linux/livepatch.h b/include/uapi/linux/livepatch.h
new file mode 100644
index 000000000000..bc35f85fd859
--- /dev/null
+++ b/include/uapi/linux/livepatch.h
@@ -0,0 +1,28 @@
+/*
+ * 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."
+
+#endif /* _UAPI_LIVEPATCH_H */
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index b9628e43c78f..a9b172d2c83f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -212,7 +212,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\n",
@@ -258,7 +258,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\n",
 			       secname);
-- 
2.12.0

[PATCH 2/8] kbuild: Support for Symbols.list creation

[Joao Moreira]

For automatic resolution of livepatch relocations, a file called
Symbols.list is used. This file maps symbols within every compiled
kernel object allowing the identification of symbols whose name is
unique, thus relocation can be automatically inferred, or providing
information that helps developers when code annotation is required for
solving the matter.

Add support for creating Symbols.list in the main Makefile. First,
ensure that built-in is compiled when CONFIG_LIVEPATCH is enabled (as
required to achieve a complete Symbols.list file). Define the command to
build Symbols.list (cmd_klp_map) and hook it in the modules rule.

As it is undesirable to have symbols from livepatch objects inside
Symbols.list, make livepatches discernible by modifying
scripts/Makefile.build to create a .livepatch file for each livepatch
in $(MODVERDIR). This file is then used by cmd_klp_map to identify and
bypass livepatches.

For identifying livepatches during the build process, a flag variable
LIVEPATCH_$(basetarget).o is considered in scripts/Makefile.build. This
way, set this flag for the livepatch sample Makefile in
samples/livepatch/Makefile.

Finally, Add a clean rule to ensure that Symbols.list is removed during
clean.

Notes:

To achieve a correct Symbols.list file, all kernel objects must be
considered, thus, its construction require these objects to be priorly
built. On the other hand, invoking scripts/Makefile.modpost without
having a complete Symbols.list in place would occasionally lead to
in-tree livepatches being post-processed incorrectly. To prevent this
from becoming a circular dependency, the construction of Symbols.list
uses non-post-processed kernel objects and such does not cause harm as
the symbols normally referenced from within livepatches are visible at
this stage. Also due to these requirements, the spot in-between modules
compilation and the invocation of scripts/Makefile.modpost was picked
for hooking cmd_klp_map.

The approach based on .livepatch files was proposed as an alternative
to using MODULE_INFO statementes. This approach was originally
proposed by Miroslav Benes as a workaround for identifying livepathces
without depending on modinfo during the modpost stage. It was moved to
this patch as the approach also shown to be useful while building
Symbols.list.

Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 .gitignore                 |  1 +
 Makefile                   | 27 ++++++++++++++++++++++++---
 samples/livepatch/Makefile |  1 +
 scripts/Makefile.build     |  6 ++++++
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0c39aa20b6ba..e6a67517a3bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -39,6 +39,7 @@ Module.symvers
 *.dwo
 *.su
 *.c.[012]*.*
+Symbols.list
 
 #
 # Top-level generic files
diff --git a/Makefile b/Makefile
index e40c471abe29..e1d315e585d8 100644
--- a/Makefile
+++ b/Makefile
@@ -320,10 +320,13 @@ KBUILD_BUILTIN := 1
 # If we have only "make modules", don't compile built-in objects.
 # When we're building modules with modversions, we need to consider
 # the built-in objects during the descend as well, in order to
-# make sure the checksums are up to date before we record them.
+# make sure the checksums are up to date before we record them. The
+# same applies for building livepatches, as built-in objects may hold
+# symbols which are referenced from livepatches and are required by
+# klp-convert post-processing tool for resolving these cases.
 
 ifeq ($(MAKECMDGOALS),modules)
-  KBUILD_BUILTIN := $(if $(CONFIG_MODVERSIONS),1)
+  KBUILD_BUILTIN := $(if $(or $(CONFIG_MODVERSIONS), $(CONFIG_LIVEPATCH)),1)
 endif
 
 # If we have "make <whatever> modules", compile modules
@@ -1207,9 +1210,24 @@ all: modules
 # duplicate lines in modules.order files.  Those are removed
 # using awk while concatenating to the final file.
 
+quiet_cmd_klp_map = LIVEPATCH	Symbols.list
+SLIST = $(objtree)/Symbols.list
+
+define cmd_klp_map
+	$(shell echo "*vmlinux" > $(SLIST))						\
+	$(shell nm -f posix $(objtree)/vmlinux | cut -d\  -f1 >> $(SLIST))		\
+	$(foreach m, $(wildcard $(MODVERDIR)/*.mod),					\
+		$(eval mod = $(patsubst %.ko,%.o,$(shell head -n1 $(m))))		\
+		$(if $(wildcard $(MODVERDIR)/$(shell basename -s .o $(mod)).livepatch),,\
+			$(eval fmod = $(subst $(quote),_,$(subst -,_,$(mod))))		\
+			$(shell echo "*$(shell basename -s .o $(fmod))" >> $(SLIST))	\
+			$(shell nm -f posix $(mod) | cut -d\  -f1 >> $(SLIST))))
+endef
+
 PHONY += modules
 modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) modules.builtin
 	$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
+	$(if $(CONFIG_LIVEPATCH), $(call cmd,klp_map))
 	@$(kecho) '  Building modules, stage 2.';
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.fwinst obj=firmware __fw_modbuild
@@ -1306,7 +1324,10 @@ vmlinuxclean:
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
 	$(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+klpclean:
+	$(Q) rm -f $(objtree)/Symbols.list
+
+clean: archclean vmlinuxclean klpclean
 
 # mrproper - Delete all generated files, including .config
 #
diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
index 10319d7ea0b1..955e7ac12d2b 100644
--- a/samples/livepatch/Makefile
+++ b/samples/livepatch/Makefile
@@ -1 +1,2 @@
+LIVEPATCH_livepatch-sample.o := y
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 733e044fff8b..d0c32a50cce7 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -276,6 +276,11 @@ objtool_obj = $(if $(patsubst y%,, \
 endif # SKIP_STACK_VALIDATION
 endif # CONFIG_STACK_VALIDATION
 
+ifdef CONFIG_LIVEPATCH
+cmd_livepatch = $(if $(LIVEPATCH_$(basetarget).o),			\
+	$(shell touch $(MODVERDIR)/$(basetarget).livepatch))
+endif
+
 define rule_cc_o_c
 	$(call echo-cmd,checksrc) $(cmd_checksrc)			  \
 	$(call cmd_and_fixdep,cc_o_c)					  \
@@ -309,6 +314,7 @@ $(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_obj) F
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; \
 	   $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
+	$(call cmd_livepatch)
 
 quiet_cmd_cc_lst_c = MKLST   $@
       cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
-- 
2.12.0

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

[Joe Lawrence]

Hi Joao,

A few nitpicks in-line below...

On Tue, Aug 29, 2017 at 04:01:34PM -0300, Joao Moreira wrote:
> For automatic resolution of livepatch relocations, a file called
> Symbols.list is used. This file maps symbols within every compiled
> kernel object allowing the identification of symbols whose name is
> unique, thus relocation can be automatically inferred, or providing
> information that helps developers when code annotation is required for
> solving the matter.
> 
> Add support for creating Symbols.list in the main Makefile. First,
> ensure that built-in is compiled when CONFIG_LIVEPATCH is enabled (as
> required to achieve a complete Symbols.list file). Define the command to
> build Symbols.list (cmd_klp_map) and hook it in the modules rule.
> 
> As it is undesirable to have symbols from livepatch objects inside
> Symbols.list, make livepatches discernible by modifying
> scripts/Makefile.build to create a .livepatch file for each livepatch
> in $(MODVERDIR). This file is then used by cmd_klp_map to identify and
> bypass livepatches.
> 
> For identifying livepatches during the build process, a flag variable
> LIVEPATCH_$(basetarget).o is considered in scripts/Makefile.build. This
> way, set this flag for the livepatch sample Makefile in
> samples/livepatch/Makefile.

I've never tried building a livepatch out of tree (is that even
possible?) but does this make it any more/less harder?
 
> Finally, Add a clean rule to ensure that Symbols.list is removed during
> clean.
> 
> Notes:
> 
> To achieve a correct Symbols.list file, all kernel objects must be
> considered, thus, its construction require these objects to be priorly
> built. On the other hand, invoking scripts/Makefile.modpost without
> having a complete Symbols.list in place would occasionally lead to
> in-tree livepatches being post-processed incorrectly. To prevent this
> from becoming a circular dependency, the construction of Symbols.list
> uses non-post-processed kernel objects and such does not cause harm as
> the symbols normally referenced from within livepatches are visible at
> this stage. Also due to these requirements, the spot in-between modules
> compilation and the invocation of scripts/Makefile.modpost was picked
> for hooking cmd_klp_map.
> 
> The approach based on .livepatch files was proposed as an alternative
> to using MODULE_INFO statementes. This approach was originally
                       ^^^^^^^^^^^
nit: s/statementes/statements

> proposed by Miroslav Benes as a workaround for identifying livepathces
                                                             ^^^^^^^^^^^
nit: s/livepathces/livepatches

> without depending on modinfo during the modpost stage. It was moved to
> this patch as the approach also shown to be useful while building
> Symbols.list.
> 
> Signed-off-by: Joao Moreira <jmoreira@suse.de>
> ---
>  .gitignore                 |  1 +
>  Makefile                   | 27 ++++++++++++++++++++++++---
>  samples/livepatch/Makefile |  1 +
>  scripts/Makefile.build     |  6 ++++++
>  4 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 0c39aa20b6ba..e6a67517a3bc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -39,6 +39,7 @@ Module.symvers
>  *.dwo
>  *.su
>  *.c.[012]*.*
> +Symbols.list
>  
>  #
>  # Top-level generic files
> diff --git a/Makefile b/Makefile
> index e40c471abe29..e1d315e585d8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -320,10 +320,13 @@ KBUILD_BUILTIN := 1
>  # If we have only "make modules", don't compile built-in objects.
>  # When we're building modules with modversions, we need to consider
>  # the built-in objects during the descend as well, in order to
> -# make sure the checksums are up to date before we record them.
> +# make sure the checksums are up to date before we record them. The
> +# same applies for building livepatches, as built-in objects may hold
> +# symbols which are referenced from livepatches and are required by
> +# klp-convert post-processing tool for resolving these cases.
>  
>  ifeq ($(MAKECMDGOALS),modules)
> -  KBUILD_BUILTIN := $(if $(CONFIG_MODVERSIONS),1)
> +  KBUILD_BUILTIN := $(if $(or $(CONFIG_MODVERSIONS), $(CONFIG_LIVEPATCH)),1)
>  endif
>  
>  # If we have "make <whatever> modules", compile modules
> @@ -1207,9 +1210,24 @@ all: modules
>  # duplicate lines in modules.order files.  Those are removed
>  # using awk while concatenating to the final file.
>  
> +quiet_cmd_klp_map = LIVEPATCH	Symbols.list

nit: I don't think any other quiet_cmd invocations put a tab between the
label and file list.  That said, "LIVEPATCH" is > 8 chars, so it's not
going to line up anyway.

> +SLIST = $(objtree)/Symbols.list
> +
> +define cmd_klp_map
> +	$(shell echo "*vmlinux" > $(SLIST))						\
> +	$(shell nm -f posix $(objtree)/vmlinux | cut -d\  -f1 >> $(SLIST))		\
> +	$(foreach m, $(wildcard $(MODVERDIR)/*.mod),					\
> +		$(eval mod = $(patsubst %.ko,%.o,$(shell head -n1 $(m))))		\
> +		$(if $(wildcard $(MODVERDIR)/$(shell basename -s .o $(mod)).livepatch),,\
> +			$(eval fmod = $(subst $(quote),_,$(subst -,_,$(mod))))		\
> +			$(shell echo "*$(shell basename -s .o $(fmod))" >> $(SLIST))	\
> +			$(shell nm -f posix $(mod) | cut -d\  -f1 >> $(SLIST))))
> +endef
> +
>  PHONY += modules
>  modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) modules.builtin
>  	$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
> +	$(if $(CONFIG_LIVEPATCH), $(call cmd,klp_map))
>  	@$(kecho) '  Building modules, stage 2.';
>  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
>  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.fwinst obj=firmware __fw_modbuild
> @@ -1306,7 +1324,10 @@ vmlinuxclean:
>  	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
>  	$(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
>  
> -clean: archclean vmlinuxclean
> +klpclean:
> +	$(Q) rm -f $(objtree)/Symbols.list
> +
> +clean: archclean vmlinuxclean klpclean

Does the klpclean target need to be added to the PHONY variable?

  % touch klpclean
  % make klpclean
  make: 'klpclean' is up to date.
  % ls -l Symbols.list 
  -rw-r--r--. 1 jolawren jolawren 2323179 Aug 29 16:55 Symbols.list

>  
>  # mrproper - Delete all generated files, including .config
>  #
> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
> index 10319d7ea0b1..955e7ac12d2b 100644
> --- a/samples/livepatch/Makefile
> +++ b/samples/livepatch/Makefile
> @@ -1 +1,2 @@
> +LIVEPATCH_livepatch-sample.o := y
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 733e044fff8b..d0c32a50cce7 100644<F2>
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -276,6 +276,11 @@ objtool_obj = $(if $(patsubst y%,, \
>  endif # SKIP_STACK_VALIDATION
>  endif # CONFIG_STACK_VALIDATION
>  
> +ifdef CONFIG_LIVEPATCH
> +cmd_livepatch = $(if $(LIVEPATCH_$(basetarget).o),			\
> +	$(shell touch $(MODVERDIR)/$(basetarget).livepatch))
> +endif
> +
>  define rule_cc_o_c
>  	$(call echo-cmd,checksrc) $(cmd_checksrc)			  \
>  	$(call cmd_and_fixdep,cc_o_c)					  \
> @@ -309,6 +314,7 @@ $(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_obj) F
>  	$(call if_changed_rule,cc_o_c)
>  	@{ echo $(@:.o=.ko); echo $@; \
>  	   $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
> +	$(call cmd_livepatch)
>  
>  quiet_cmd_cc_lst_c = MKLST   $@
>        cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
> -- 
> 2.12.0
> 

I'll try to dig in deeper and tinker with the patchset next week.

Regards,

-- Joe

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

[Josh Poimboeuf]

On Thu, Aug 31, 2017 at 11:24:39AM -0400, Joe Lawrence wrote:
> > +quiet_cmd_klp_map = LIVEPATCH	Symbols.list
> 
> nit: I don't think any other quiet_cmd invocations put a tab between the
> label and file list.  That said, "LIVEPATCH" is > 8 chars, so it's not
> going to line up anyway.

Maybe "SYMBOLS" would be more appropriate (and would fit).

-- 
Josh

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

[Joao Moreira]

On 08/31/2017 12:24 PM, Joe Lawrence wrote:
> Hi Joao,
> 
Hi Joe and Josh,

Thanks for the quick feedback, I'll be looking forward for your comments 
once you have the change to dig deeper :). I'll apply all typo-fixes, 
add klpclean to the PHONY targets and change the quiet_cmd invocation as 
suggested in v2.

>> For identifying livepatches during the build process, a flag variable
>> LIVEPATCH_$(basetarget).o is considered in scripts/Makefile.build. This
>> way, set this flag for the livepatch sample Makefile in
>> samples/livepatch/Makefile.
> 
> I've never tried building a livepatch out of tree (is that even
> possible?) but does this make it any more/less harder?

Tbh, I never had tried it before, so I just emulated the process and it 
was simple and successful. All it required in addition to what is 
regular, was the introduction of the "LIVEPATCH_($bastarget).o := y" 
into the Makefile. All else was implicitly handled by Kbuild.

Best,
Joao

[PATCH 3/8] livepatch: Add klp-convert tool

[Joao Moreira]

From: Josh Poimboeuf <jpoimboe@redhat.com>

Livepatches may use symbols which are not contained in its own scope,
and, because of that, may end up compiled with relocations that will
only be resolved during module load. Yet, when the referenced symbols
are not exported, solving this relocation requires information on the
object that holds the symbol (either vmlinux or modules) and its
position inside the object, as an object may contain multiple symbols
with the same name. Providing such information must be done
accordingly to what is specified in
Documentation/livepatch/module-elf-format.txt.

Currently, there is no trivial way to embed the required information
as requested in the final livepatch elf object. klp-convert solves
this problem in two different forms: (i) by relying on Symbols.list,
which is built during kernel compilation, to automatically infer the
relocation targeted symbol, and, when such inference is not possible
(ii) by using annotations in the elf object to convert the relocation
accordingly to the specification, enabling it to be handled by the
livepatch loader.

Given the above, create scripts/livepatch to hold tools developed for
livepatches and add source files for klp-convert there.

The core file of klp-convert is scripts/livepatch/klp-convert.c, which
implements the heuristics used to solve the relocations and the
conversion of unresolved symbols into the expected format, as defined
in [1].

klp-convert receives as arguments the Symbols.list file, an input
livepatch module to be converted and the output name for the converted
livepatch. When it starts running, klp-convert parses Symbols.list and
builds two internal lists of symbols, one containing the exported and
another containing the non-exported symbols. Then, by parsing the rela
sections in the elf object, klp-convert identifies which symbols must
be converted, which are those unresolved and that do not have a
corresponding exported symbol, and attempts to convert them
accordingly to the specification.

By using Symbols.list, klp-convert identifies which symbols have names
that only appear in a single kernel object, thus being capable of
resolving these cases without the intervention of the developer. When
various homonymous symbols exist through kernel objects, it is not
possible to infer the right one, thus klp-convert falls back into
using developer annotations. If these were not provided, then the tool
will print a list with all acceptable targets for the symbol being
processed.

Annotations in the context of klp-convert are accessible as struct
klp_module_reloc entries in sections named
.klp.module_relocs.<objname>. These entries are pairs of symbol
references and positions which are to be resolved against definitions
in <objname>.

Define the structure klp_module_reloc in
include/linux/uapi/livepatch.h allowing developers to annotate the
livepatch source code with it.

klp-convert relies on libelf and on a list implementation. Add files
scripts/livepatch/elf.c and scripts/livepatch/elf.h, which are a
libelf interfacing layer and scripts/livepatch/list.h, which is a
list implementation.

Update Makefiles to correctly support the compilation of the new tool,
update MAINTAINERS file and add a .gitignore file.

[1] - Documentation/livepatch/module-elf-format.txt

[khlebnikov: use HOSTLOADLIBES_ instead of HOSTLDFLAGS: -lelf must be
at the end]
[jmoreira:
* add support to automatic relocation conversion in klp-convert.c
* changelog]

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 MAINTAINERS                     |   1 +
 Makefile                        |   2 +-
 include/uapi/linux/livepatch.h  |   5 +
 scripts/Makefile                |   1 +
 scripts/livepatch/.gitignore    |   1 +
 scripts/livepatch/Makefile      |   7 +
 scripts/livepatch/elf.c         | 696 ++++++++++++++++++++++++++++++++++++++++
 scripts/livepatch/elf.h         |  84 +++++
 scripts/livepatch/klp-convert.c | 567 ++++++++++++++++++++++++++++++++
 scripts/livepatch/list.h        | 389 ++++++++++++++++++++++
 10 files changed, 1752 insertions(+), 1 deletion(-)
 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/MAINTAINERS b/MAINTAINERS
index 1d8ea36e1173..3e0a576a5639 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7719,6 +7719,7 @@ F:	arch/x86/kernel/livepatch.c
 F:	Documentation/livepatch/
 F:	Documentation/ABI/testing/sysfs-kernel-livepatch
 F:	samples/livepatch/
+F:	scripts/livepatch/
 L:	live-patching@vger.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jikos/livepatching.git
 
diff --git a/Makefile b/Makefile
index e1d315e585d8..5265cb5a3d89 100644
--- a/Makefile
+++ b/Makefile
@@ -561,7 +561,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/uapi/linux/livepatch.h b/include/uapi/linux/livepatch.h
index bc35f85fd859..3de6f7fcea3a 100644
--- a/include/uapi/linux/livepatch.h
+++ b/include/uapi/linux/livepatch.h
@@ -25,4 +25,9 @@
 #define KLP_RELA_PREFIX		".klp.rela."
 #define KLP_SYM_PREFIX		".klp.sym."
 
+struct klp_module_reloc {
+	void *sym;
+	unsigned int sympos;
+} __attribute__((packed));
+
 #endif /* _UAPI_LIVEPATCH_H */
diff --git a/scripts/Makefile b/scripts/Makefile
index 1d80897a9644..2cc2b8a52b3b 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/livepatch/.gitignore b/scripts/livepatch/.gitignore
new file mode 100644
index 000000000000..dc22fe4b6a5b
--- /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 000000000000..05bae6a849e4
--- /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
+HOSTLOADLIBES_klp-convert	:= -lelf
diff --git a/scripts/livepatch/elf.c b/scripts/livepatch/elf.c
new file mode 100644
index 000000000000..f279dd3be1b7
--- /dev/null
+++ b/scripts/livepatch/elf.c
@@ -0,0 +1,696 @@
+/*
+ * 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;
+}
+
+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);
+	if (!sec->name) {
+		WARN("strdup failed");
+		return NULL;
+	}
+	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 offset = 0, nr_locals = 0, idx, nr_syms;
+
+	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;
+	}
+	nr_syms = 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;
+
+	/* 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 000000000000..e8aa8f5fb3bc
--- /dev/null
+++ b/scripts/livepatch/elf.h
@@ -0,0 +1,84 @@
+/*
+ * 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_POST_ELF_H
+#define _KLP_POST_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 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_POST_ELF_H */
diff --git a/scripts/livepatch/klp-convert.c b/scripts/livepatch/klp-convert.c
new file mode 100644
index 000000000000..baf6c83f8eb0
--- /dev/null
+++ b/scripts/livepatch/klp-convert.c
@@ -0,0 +1,567 @@
+/*
+ * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ * Copyright (C) 2017 Joao Moreira   <jmoreira@suse.de>
+ *
+ * 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 <unistd.h>
+#include <string.h>
+#include <linux/livepatch.h>
+#include "elf.h"
+#include "list.h"
+
+#define SHN_LIVEPATCH		0xff20
+#define SHF_RELA_LIVEPATCH	0x00100000
+#define MODULE_NAME_LEN		(64 - sizeof(GElf_Addr))
+#define WARN(format, ...) \
+	fprintf(stderr, "klp-convert: " format "\n", ##__VA_ARGS__)
+
+struct symbol_entry {
+	struct list_head list;
+	char *symbol_name;
+	char *object_name;
+};
+
+struct sympos {
+	struct list_head list;
+	char *symbol_name;
+	char *object_name;
+	int pos;
+};
+
+/*
+ * Symbols parsed from Symbols.list are kept in two lists:
+ * - symbols: keeps non-exported symbols
+ * - exp_symbols: keeps exported symbols (__ksymtab_prefixed)
+ */
+static LIST_HEAD(symbols);
+static LIST_HEAD(exp_symbols);
+
+/* In-livepatch user-provided symbol positions are kept in list usr_symbols */
+static LIST_HEAD(usr_symbols);
+
+void free_syms_lists(void)
+{
+	struct symbol_entry *entry, *aux;
+	struct sympos *sp, *sp_aux;
+
+	list_for_each_entry_safe(entry, aux, &symbols, list) {
+		free(entry->object_name);
+		free(entry->symbol_name);
+		list_del(&entry->list);
+		free(entry);
+	}
+
+	list_for_each_entry_safe(entry, aux, &exp_symbols, list) {
+		free(entry->object_name);
+		free(entry->symbol_name);
+		list_del(&entry->list);
+		free(entry);
+	}
+
+	list_for_each_entry_safe(sp, sp_aux, &usr_symbols, list) {
+		free(sp->object_name);
+		free(sp->symbol_name);
+		list_del(&sp->list);
+		free(sp);
+	}
+}
+
+/* Parses file and fill symbols and exp_symbols list */
+bool load_syms_lists(const char *symbols_list)
+{
+	FILE *fsyms;
+	struct symbol_entry *entry;
+	size_t len = 0;
+	ssize_t n;
+	char *obj = NULL, *sym = NULL;
+
+	fsyms = fopen(symbols_list, "r");
+	if (!fsyms) {
+		WARN("Unable to open Symbol list: %s", symbols_list);
+		return false;
+	}
+
+	n = getline(&sym, &len, fsyms);
+	while (n > 0) {
+		if (sym[n-1] == '\n')
+			sym[n-1] = '\0';
+
+		/* Objects in Symbols.list are flagged with '*' */
+		if (sym[0] == '*') {
+			if (obj)
+				free(obj);
+			obj = strdup(sym+1);
+			if (!obj) {
+				WARN("Unable to allocate object name\n");
+				return false;
+			}
+			free(sym);
+		} else {
+			entry = calloc(1, sizeof(struct symbol_entry));
+			if (!entry) {
+				WARN("Unable to allocate Symbol entry\n");
+				return false;
+			}
+
+			entry->object_name = strdup(obj);
+			if (!entry->object_name) {
+				WARN("Unable to allocate entry object name\n");
+				return false;
+			}
+
+			entry->symbol_name = sym;
+
+			if (strncmp(entry->symbol_name, "__ksymtab_", 10) == 0)
+				list_add(&entry->list, &exp_symbols);
+			else
+				list_add(&entry->list, &symbols);
+		}
+		len = 0;
+		sym = NULL;
+		n = getline(&sym, &len, fsyms);
+	}
+	free(sym);
+	free(obj);
+	fclose(fsyms);
+	return true;
+}
+
+/* Dump symbols list for debugging purposes */
+void dump_symbols(void)
+{
+	struct symbol_entry *entry;
+
+	fprintf(stderr, "BEGIN OF SYMBOLS DUMP\n");
+	list_for_each_entry(entry, &symbols, list)
+		printf("%s %s\n", entry->object_name, entry->symbol_name);
+	fprintf(stderr, "END OF SYMBOLS DUMP\n");
+}
+
+
+/* Searches for sympos of specific symbol in usr_symbols list */
+bool get_usr_sympos(struct symbol *s, struct sympos *sp)
+{
+	struct sympos *aux;
+
+	list_for_each_entry(aux, &usr_symbols, list) {
+		if (strcmp(aux->symbol_name, s->name) == 0) {
+			sp->symbol_name = aux->symbol_name;
+			sp->object_name = aux->object_name;
+			sp->pos = aux->pos;
+			return true;
+		}
+	}
+	return false;
+}
+
+/* Removes symbols used for sympos annotation from livepatch elf object */
+void clear_sympos_symbols(struct section *sec, struct elf *klp_elf)
+{
+	struct symbol *sym, *aux;
+
+	list_for_each_entry_safe(sym, aux, &klp_elf->symbols, list) {
+		if (sym->sec == sec) {
+			list_del(&sym->list);
+			free(sym);
+		}
+	}
+}
+
+/* Removes annotation from livepatch elf object */
+void clear_sympos_annontations(struct elf *klp_elf)
+{
+	struct section *sec, *aux;
+
+	list_for_each_entry_safe(sec, aux, &klp_elf->sections, list) {
+		if (strncmp(sec->name, ".klp.module_relocs.", 19) == 0) {
+			clear_sympos_symbols(sec, klp_elf);
+			list_del(&sec->list);
+			free(sec);
+			continue;
+		}
+		if (strncmp(sec->name, ".rela.klp.module_relocs.", 24) == 0) {
+			list_del(&sec->list);
+			free(sec);
+			continue;
+		}
+	}
+}
+
+/* Checks if two or more elements in usr_symbols have the same name */
+bool sympos_sanity_check(void)
+{
+	bool sane = true;
+	struct sympos *sp, *aux;
+
+	list_for_each_entry(sp, &usr_symbols, list) {
+		aux = list_next_entry(sp, list);
+		list_for_each_entry_from(aux, &usr_symbols, list) {
+			if (strcmp(sp->symbol_name, aux->symbol_name) == 0) {
+				WARN("KLP_SYMPOS Collision: %s in %s and %s.",
+						sp->symbol_name,
+						sp->object_name,
+						aux->object_name);
+				sane = false;
+			}
+		}
+	}
+	return sane;
+}
+
+/* Parses the livepatch elf object and fills usr_symbols */
+bool load_usr_symbols(struct elf *klp_elf)
+{
+	char objname[MODULE_NAME_LEN];
+	struct sympos *sp;
+	struct section *sec, *aux, *relasec;
+	struct rela *rela;
+	struct klp_module_reloc *reloc;
+	int i, nr_entries;
+
+	list_for_each_entry_safe(sec, aux, &klp_elf->sections, list) {
+		if (sscanf(sec->name, ".klp.module_relocs.%55s", objname) != 1)
+			continue;
+
+		relasec = sec->rela;
+		reloc = sec->data;
+		i = 0;
+		nr_entries = sec->size / sizeof(*reloc);
+		list_for_each_entry(rela, &relasec->relas, list) {
+			sp = calloc(1, sizeof(struct sympos));
+			if (!sp) {
+				WARN("Unable to allocate sympos memory\n");
+				return false;
+			}
+			sp->object_name = strdup(objname);
+			if (!sp->object_name) {
+				WARN("Unable to allocate object name\n");
+				return false;
+			}
+			sp->symbol_name = strdup(rela->sym->name);
+			sp->pos = reloc[i].sympos;
+			list_add(&sp->list, &usr_symbols);
+			i++;
+		}
+		if (i != nr_entries) {
+			WARN("nr_entries mismatch (%d != %d) for %s\n",
+			     i, nr_entries, relasec->name);
+			return false;
+		}
+	}
+	clear_sympos_annontations(klp_elf);
+	return sympos_sanity_check();
+}
+
+/* Dumps sympos list (useful for debugging purposes) */
+void dump_sympos(void)
+{
+	struct sympos *sp;
+
+	fprintf(stderr, "BEGIN OF SYMPOS DUMP\n");
+	list_for_each_entry(sp, &usr_symbols, list) {
+		fprintf(stderr, "%s %s %d\n", sp->symbol_name, sp->object_name,
+				sp->pos);
+	}
+	fprintf(stderr, "END OF SYMPOS DUMP\n");
+}
+
+/* prints list of valid sympos for symbol with provided name */
+void print_valid_module_relocs(char *name)
+{
+	struct symbol_entry *e;
+	char *cur_obj = "";
+	int counter;
+	bool first = true;
+
+	/* Symbols from the same object are locally gathered in the list */
+	fprintf(stderr, "Valid KLP_SYMPOS for symbol %s:\n", name);
+	fprintf(stderr, "-------------------------------------------------\n");
+	list_for_each_entry(e, &symbols, list) {
+		if (strcmp(e->object_name, cur_obj) != 0) {
+			cur_obj = e->object_name;
+			counter = 0;
+		}
+		if (strcmp(e->symbol_name, name) == 0) {
+			if (counter == 0) {
+				if (!first)
+					fprintf(stderr, "}\n");
+
+				fprintf(stderr, "KLP_MODULE_RELOC(%s){\n",
+						cur_obj);
+				first = false;
+			}
+			fprintf(stderr, "\tKLP_SYMPOS(%s,%d)\n", name, counter);
+			counter++;
+		}
+	}
+	fprintf(stderr, "-------------------------------------------------\n");
+}
+
+/* Searches for symbol in symbols list and returns its sympos if it is unique,
+ * otherwise prints a list with all considered valid sympos
+ */
+struct symbol_entry *find_sym_entry_by_name(char *name)
+{
+	struct symbol_entry *found = NULL;
+	struct symbol_entry *e;
+
+	list_for_each_entry(e, &symbols, list) {
+		if (strcmp(e->symbol_name, name) == 0) {
+
+			/* If there exist multiple symbols with the same
+			 * name then user-provided sympos is required
+			 */
+			if (found) {
+				print_valid_module_relocs(name);
+				return NULL;
+			}
+			found = e;
+		}
+	}
+	if (found)
+		return found;
+
+	return NULL;
+}
+
+/* Checks if sympos is valid, otherwise prints valid sympos list */
+bool valid_sympos(struct sympos *sp)
+{
+	struct symbol_entry *e;
+	int counter = 0;
+
+	list_for_each_entry(e, &symbols, list) {
+		if ((strcmp(e->symbol_name, sp->symbol_name) == 0) &&
+		    (strcmp(e->object_name, sp->object_name) == 0)) {
+			if (counter == sp->pos)
+				return true;
+			counter++;
+		}
+	}
+
+	WARN("Provided KLP_SYMPOS does not match a symbol (%s.%s)",
+			sp->symbol_name, sp->object_name);
+	print_valid_module_relocs(sp->symbol_name);
+
+	return false;
+}
+
+/* Returns the right sympos respective to a symbol to be relocated */
+bool find_missing_position(struct symbol *s, struct sympos *sp)
+{
+	struct symbol_entry *entry;
+
+	if (get_usr_sympos(s, sp)) {
+		if (valid_sympos(sp))
+			return true;
+		return false;
+	}
+
+	/* if no user-provided sympos, search symbol in symbols list */
+	entry = find_sym_entry_by_name(s->name);
+	if (entry) {
+		sp->symbol_name = entry->symbol_name;
+		sp->object_name = entry->object_name;
+		sp->pos = 0;
+		return true;
+	}
+	return false;
+}
+
+/* Finds or creates a klp rela section based on another given section (@oldsec)
+ * and sympos (@*sp), then returns it
+ */
+struct section *get_or_create_klp_rela_section(struct section *oldsec,
+		struct sympos *sp, struct elf *klp_elf)
+{
+	char *name;
+	struct section *sec;
+	unsigned int length;
+
+	length = strlen(KLP_RELA_PREFIX) + strlen(sp->object_name)
+		 + strlen(oldsec->base->name) + 2;
+
+	name = calloc(1, length);
+	if (!name) {
+		WARN("Memory allocation failed (%s%s.%s)\n", KLP_RELA_PREFIX,
+				sp->object_name, oldsec->base->name);
+		return NULL;
+	}
+
+	if (snprintf(name, length, KLP_RELA_PREFIX "%s.%s", sp->object_name,
+				oldsec->base->name) >= length) {
+		WARN("Length error (%s)", name);
+		free(name);
+		return false;
+	}
+
+	sec = find_section_by_name(klp_elf, name);
+	if (!sec)
+		sec = create_rela_section(klp_elf, name, oldsec->base);
+
+	if (sec)
+		sec->sh.sh_flags |= SHF_RELA_LIVEPATCH;
+
+	free(name);
+	return sec;
+}
+
+/* Create klp rela, add it to the right klp rela sec and del reference rela */
+bool convert_rela(struct section *oldsec, struct rela *r, struct sympos *sp,
+		struct elf *klp_elf)
+{
+	char *name;
+	char pos[4];	/* assume that pos will never be > 999 */
+	unsigned int length;
+	struct section *sec;
+	struct symbol *s = r->sym;
+
+	sec = get_or_create_klp_rela_section(oldsec, sp, klp_elf);
+	if (!sec) {
+		WARN("Can't create or access klp.rela section (%s.%s)\n",
+		    sp->object_name, sp->symbol_name);
+		return false;
+	}
+
+	if (snprintf(pos, sizeof(pos), "%d", sp->pos) > 3) {
+		WARN("Insuficient buffer for expanding sympos (%s.%s,%d)\n",
+			sp->object_name, sp->symbol_name, sp->pos);
+		return false;
+	}
+
+	length = strlen(KLP_SYM_PREFIX) + strlen(sp->object_name)
+		 + strlen(sp->symbol_name) + 4;
+
+	name = calloc(1, length);
+	if (!name) {
+		WARN("Memory allocation failed (%s%s.%s,%s)\n", KLP_SYM_PREFIX,
+				sp->object_name, sp->symbol_name, pos);
+		return false;
+	}
+
+	if (snprintf(name, length, KLP_SYM_PREFIX "%s.%s,%s", sp->object_name,
+				sp->symbol_name, pos) >= length) {
+		WARN("Length error (%s.%s)", sp->object_name, sp->symbol_name);
+		return false;
+	}
+
+	s->name = name;
+	s->sec = NULL;
+	s->sym.st_name = -1;
+	s->sym.st_shndx = SHN_LIVEPATCH;
+
+	list_del(&r->list);
+	list_add(&r->list, &sec->relas);
+	return true;
+}
+
+/* Checks if given symbol name matches a symbol in exp_symbols */
+bool is_exported(char *sname)
+{
+	struct symbol_entry *e;
+
+	/* exp_symbols itens are prefixed with __ksymtab_ - comparisons must
+	 * skip prefix and check if both are properly null-terminated
+	 */
+	list_for_each_entry(e, &exp_symbols, list) {
+		if (strcmp(e->symbol_name + 10, sname) == 0)
+			return true;
+	}
+	return false;
+}
+
+/* Checks if a symbol was previously klp-converted based on its name */
+bool is_converted(char *sname)
+{
+	int len = strlen(KLP_SYM_PREFIX);
+
+	if (strncmp(sname, KLP_SYM_PREFIX, len) == 0)
+		return true;
+	return false;
+}
+
+/* Checks if symbol must be klp-converted - must not be already resolved, have
+ * been already klp-converted nor be an exported symbol
+ */
+bool must_convert(struct symbol *sym)
+{
+	if (sym->sec)
+		return false;
+	return (!(is_converted(sym->name) || is_exported(sym->name)));
+}
+
+int main(int argc, const char **argv)
+{
+	const char *klp_in_module, *klp_out_module, *symbols_list;
+	struct rela *rela, *tmprela;
+	struct section *sec, *aux;
+	struct sympos *sp;
+	struct elf *klp_elf;
+
+	if (argc != 4) {
+		WARN("Usage: %s <Symbols.list> <input.ko> <out.ko>", argv[0]);
+		return -1;
+	}
+
+	symbols_list = argv[1];
+	klp_in_module = argv[2];
+	klp_out_module = argv[3];
+
+	klp_elf = elf_open(klp_in_module);
+	if (!klp_elf) {
+		WARN("Unable to read elf file %s\n", klp_in_module);
+		return -1;
+	}
+
+	if (!load_syms_lists(symbols_list))
+		return -1;
+
+	if (!load_usr_symbols(klp_elf)) {
+		WARN("Unable to load user-provided sympos");
+		return -1;
+	}
+
+	list_for_each_entry_safe(sec, aux, &klp_elf->sections, list) {
+		if (!is_rela_section(sec))
+			continue;
+		list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+			if (!must_convert(rela->sym))
+				continue;
+
+			sp = calloc(1, sizeof(struct sympos));
+			if (!sp) {
+				WARN("Unable to allocate memory for sympos");
+				return -1;
+			}
+			if (!find_missing_position(rela->sym, sp)) {
+				WARN("Unable to find missing symbol");
+				return -1;
+			}
+			if (!convert_rela(sec, rela, sp, klp_elf)) {
+				WARN("Unable to convert relocation");
+				return -1;
+			}
+			free(sp);
+		}
+	}
+
+	free_syms_lists();
+	if (elf_write_file(klp_elf, klp_out_module))
+		return -1;
+
+	return 0;
+}
diff --git a/scripts/livepatch/list.h b/scripts/livepatch/list.h
new file mode 100644
index 000000000000..b324eb29c3b6
--- /dev/null
+++ b/scripts/livepatch/list.h
@@ -0,0 +1,389 @@
+#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
-- 
2.12.0

Re: [PATCH 3/8] livepatch: Add klp-convert tool

[Joao Moreira]

Hi, I just would like to pinpoint a bug I just noticed while doing some 
extra experiments on klp-convert.

In this current version multiple relas respective to a same symbol are 
not being correctly moved into the .klp.rela section, just the first 
one. This way, if the live-patch algorithm reuses the same symbol 
multiple times, only the first one will be correctly resolved.

I have already worked out a solution for the bug and I'll integrate it 
into v2, along with another use-case that unveils this situation.

Also, despite the bug, the live-patch was not prevented from being 
loaded. Perhaps we should care about it in the future.

Tks,
Joao.

On 08/29/2017 04:01 PM, Joao Moreira wrote:
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> Livepatches may use symbols which are not contained in its own scope,
> and, because of that, may end up compiled with relocations that will
> only be resolved during module load. Yet, when the referenced symbols
> are not exported, solving this relocation requires information on the
> object that holds the symbol (either vmlinux or modules) and its
> position inside the object, as an object may contain multiple symbols
> with the same name. Providing such information must be done
> accordingly to what is specified in
> Documentation/livepatch/module-elf-format.txt.
> 
> Currently, there is no trivial way to embed the required information
> as requested in the final livepatch elf object. klp-convert solves
> this problem in two different forms: (i) by relying on Symbols.list,
> which is built during kernel compilation, to automatically infer the
> relocation targeted symbol, and, when such inference is not possible
> (ii) by using annotations in the elf object to convert the relocation
> accordingly to the specification, enabling it to be handled by the
> livepatch loader.
> 
> Given the above, create scripts/livepatch to hold tools developed for
> livepatches and add source files for klp-convert there.
> 
> The core file of klp-convert is scripts/livepatch/klp-convert.c, which
> implements the heuristics used to solve the relocations and the
> conversion of unresolved symbols into the expected format, as defined
> in [1].
> 
> klp-convert receives as arguments the Symbols.list file, an input
> livepatch module to be converted and the output name for the converted
> livepatch. When it starts running, klp-convert parses Symbols.list and
> builds two internal lists of symbols, one containing the exported and
> another containing the non-exported symbols. Then, by parsing the rela
> sections in the elf object, klp-convert identifies which symbols must
> be converted, which are those unresolved and that do not have a
> corresponding exported symbol, and attempts to convert them
> accordingly to the specification.
> 
> By using Symbols.list, klp-convert identifies which symbols have names
> that only appear in a single kernel object, thus being capable of
> resolving these cases without the intervention of the developer. When
> various homonymous symbols exist through kernel objects, it is not
> possible to infer the right one, thus klp-convert falls back into
> using developer annotations. If these were not provided, then the tool
> will print a list with all acceptable targets for the symbol being
> processed.
> 
> Annotations in the context of klp-convert are accessible as struct
> klp_module_reloc entries in sections named
> .klp.module_relocs.<objname>. These entries are pairs of symbol
> references and positions which are to be resolved against definitions
> in <objname>.
> 
> Define the structure klp_module_reloc in
> include/linux/uapi/livepatch.h allowing developers to annotate the
> livepatch source code with it.
> 
> klp-convert relies on libelf and on a list implementation. Add files
> scripts/livepatch/elf.c and scripts/livepatch/elf.h, which are a
> libelf interfacing layer and scripts/livepatch/list.h, which is a
> list implementation.
> 
> Update Makefiles to correctly support the compilation of the new tool,
> update MAINTAINERS file and add a .gitignore file.
> 
> [1] - Documentation/livepatch/module-elf-format.txt
> 
> [khlebnikov: use HOSTLOADLIBES_ instead of HOSTLDFLAGS: -lelf must be
> at the end]
> [jmoreira:
> * add support to automatic relocation conversion in klp-convert.c
> * changelog]
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Signed-off-by: Joao Moreira <jmoreira@suse.de>
> ---
>   MAINTAINERS                     |   1 +
>   Makefile                        |   2 +-
>   include/uapi/linux/livepatch.h  |   5 +
>   scripts/Makefile                |   1 +
>   scripts/livepatch/.gitignore    |   1 +
>   scripts/livepatch/Makefile      |   7 +
>   scripts/livepatch/elf.c         | 696 ++++++++++++++++++++++++++++++++++++++++
>   scripts/livepatch/elf.h         |  84 +++++
>   scripts/livepatch/klp-convert.c | 567 ++++++++++++++++++++++++++++++++
>   scripts/livepatch/list.h        | 389 ++++++++++++++++++++++
>   10 files changed, 1752 insertions(+), 1 deletion(-)
>   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/MAINTAINERS b/MAINTAINERS
> index 1d8ea36e1173..3e0a576a5639 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7719,6 +7719,7 @@ F:	arch/x86/kernel/livepatch.c
>   F:	Documentation/livepatch/
>   F:	Documentation/ABI/testing/sysfs-kernel-livepatch
>   F:	samples/livepatch/
> +F:	scripts/livepatch/
>   L:	live-patching@vger.kernel.org
>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jikos/livepatching.git
>   
> diff --git a/Makefile b/Makefile
> index e1d315e585d8..5265cb5a3d89 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -561,7 +561,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/uapi/linux/livepatch.h b/include/uapi/linux/livepatch.h
> index bc35f85fd859..3de6f7fcea3a 100644
> --- a/include/uapi/linux/livepatch.h
> +++ b/include/uapi/linux/livepatch.h
> @@ -25,4 +25,9 @@
>   #define KLP_RELA_PREFIX		".klp.rela."
>   #define KLP_SYM_PREFIX		".klp.sym."
>   
> +struct klp_module_reloc {
> +	void *sym;
> +	unsigned int sympos;
> +} __attribute__((packed));
> +
>   #endif /* _UAPI_LIVEPATCH_H */
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 1d80897a9644..2cc2b8a52b3b 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/livepatch/.gitignore b/scripts/livepatch/.gitignore
> new file mode 100644
> index 000000000000..dc22fe4b6a5b
> --- /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 000000000000..05bae6a849e4
> --- /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
> +HOSTLOADLIBES_klp-convert	:= -lelf
> diff --git a/scripts/livepatch/elf.c b/scripts/livepatch/elf.c
> new file mode 100644
> index 000000000000..f279dd3be1b7
> --- /dev/null
> +++ b/scripts/livepatch/elf.c
> @@ -0,0 +1,696 @@
> +/*
> + * 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;
> +}
> +
> +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);
> +	if (!sec->name) {
> +		WARN("strdup failed");
> +		return NULL;
> +	}
> +	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 offset = 0, nr_locals = 0, idx, nr_syms;
> +
> +	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;
> +	}
> +	nr_syms = 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;
> +
> +	/* 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 000000000000..e8aa8f5fb3bc
> --- /dev/null
> +++ b/scripts/livepatch/elf.h
> @@ -0,0 +1,84 @@
> +/*
> + * 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_POST_ELF_H
> +#define _KLP_POST_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 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_POST_ELF_H */
> diff --git a/scripts/livepatch/klp-convert.c b/scripts/livepatch/klp-convert.c
> new file mode 100644
> index 000000000000..baf6c83f8eb0
> --- /dev/null
> +++ b/scripts/livepatch/klp-convert.c
> @@ -0,0 +1,567 @@
> +/*
> + * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
> + * Copyright (C) 2017 Joao Moreira   <jmoreira@suse.de>
> + *
> + * 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 <unistd.h>
> +#include <string.h>
> +#include <linux/livepatch.h>
> +#include "elf.h"
> +#include "list.h"
> +
> +#define SHN_LIVEPATCH		0xff20
> +#define SHF_RELA_LIVEPATCH	0x00100000
> +#define MODULE_NAME_LEN		(64 - sizeof(GElf_Addr))
> +#define WARN(format, ...) \
> +	fprintf(stderr, "klp-convert: " format "\n", ##__VA_ARGS__)
> +
> +struct symbol_entry {
> +	struct list_head list;
> +	char *symbol_name;
> +	char *object_name;
> +};
> +
> +struct sympos {
> +	struct list_head list;
> +	char *symbol_name;
> +	char *object_name;
> +	int pos;
> +};
> +
> +/*
> + * Symbols parsed from Symbols.list are kept in two lists:
> + * - symbols: keeps non-exported symbols
> + * - exp_symbols: keeps exported symbols (__ksymtab_prefixed)
> + */
> +static LIST_HEAD(symbols);
> +static LIST_HEAD(exp_symbols);
> +
> +/* In-livepatch user-provided symbol positions are kept in list usr_symbols */
> +static LIST_HEAD(usr_symbols);
> +
> +void free_syms_lists(void)
> +{
> +	struct symbol_entry *entry, *aux;
> +	struct sympos *sp, *sp_aux;
> +
> +	list_for_each_entry_safe(entry, aux, &symbols, list) {
> +		free(entry->object_name);
> +		free(entry->symbol_name);
> +		list_del(&entry->list);
> +		free(entry);
> +	}
> +
> +	list_for_each_entry_safe(entry, aux, &exp_symbols, list) {
> +		free(entry->object_name);
> +		free(entry->symbol_name);
> +		list_del(&entry->list);
> +		free(entry);
> +	}
> +
> +	list_for_each_entry_safe(sp, sp_aux, &usr_symbols, list) {
> +		free(sp->object_name);
> +		free(sp->symbol_name);
> +		list_del(&sp->list);
> +		free(sp);
> +	}
> +}
> +
> +/* Parses file and fill symbols and exp_symbols list */
> +bool load_syms_lists(const char *symbols_list)
> +{
> +	FILE *fsyms;
> +	struct symbol_entry *entry;
> +	size_t len = 0;
> +	ssize_t n;
> +	char *obj = NULL, *sym = NULL;
> +
> +	fsyms = fopen(symbols_list, "r");
> +	if (!fsyms) {
> +		WARN("Unable to open Symbol list: %s", symbols_list);
> +		return false;
> +	}
> +
> +	n = getline(&sym, &len, fsyms);
> +	while (n > 0) {
> +		if (sym[n-1] == '\n')
> +			sym[n-1] = '\0';
> +
> +		/* Objects in Symbols.list are flagged with '*' */
> +		if (sym[0] == '*') {
> +			if (obj)
> +				free(obj);
> +			obj = strdup(sym+1);
> +			if (!obj) {
> +				WARN("Unable to allocate object name\n");
> +				return false;
> +			}
> +			free(sym);
> +		} else {
> +			entry = calloc(1, sizeof(struct symbol_entry));
> +			if (!entry) {
> +				WARN("Unable to allocate Symbol entry\n");
> +				return false;
> +			}
> +
> +			entry->object_name = strdup(obj);
> +			if (!entry->object_name) {
> +				WARN("Unable to allocate entry object name\n");
> +				return false;
> +			}
> +
> +			entry->symbol_name = sym;
> +
> +			if (strncmp(entry->symbol_name, "__ksymtab_", 10) == 0)
> +				list_add(&entry->list, &exp_symbols);
> +			else
> +				list_add(&entry->list, &symbols);
> +		}
> +		len = 0;
> +		sym = NULL;
> +		n = getline(&sym, &len, fsyms);
> +	}
> +	free(sym);
> +	free(obj);
> +	fclose(fsyms);
> +	return true;
> +}
> +
> +/* Dump symbols list for debugging purposes */
> +void dump_symbols(void)
> +{
> +	struct symbol_entry *entry;
> +
> +	fprintf(stderr, "BEGIN OF SYMBOLS DUMP\n");
> +	list_for_each_entry(entry, &symbols, list)
> +		printf("%s %s\n", entry->object_name, entry->symbol_name);
> +	fprintf(stderr, "END OF SYMBOLS DUMP\n");
> +}
> +
> +
> +/* Searches for sympos of specific symbol in usr_symbols list */
> +bool get_usr_sympos(struct symbol *s, struct sympos *sp)
> +{
> +	struct sympos *aux;
> +
> +	list_for_each_entry(aux, &usr_symbols, list) {
> +		if (strcmp(aux->symbol_name, s->name) == 0) {
> +			sp->symbol_name = aux->symbol_name;
> +			sp->object_name = aux->object_name;
> +			sp->pos = aux->pos;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +/* Removes symbols used for sympos annotation from livepatch elf object */
> +void clear_sympos_symbols(struct section *sec, struct elf *klp_elf)
> +{
> +	struct symbol *sym, *aux;
> +
> +	list_for_each_entry_safe(sym, aux, &klp_elf->symbols, list) {
> +		if (sym->sec == sec) {
> +			list_del(&sym->list);
> +			free(sym);
> +		}
> +	}
> +}
> +
> +/* Removes annotation from livepatch elf object */
> +void clear_sympos_annontations(struct elf *klp_elf)
> +{
> +	struct section *sec, *aux;
> +
> +	list_for_each_entry_safe(sec, aux, &klp_elf->sections, list) {
> +		if (strncmp(sec->name, ".klp.module_relocs.", 19) == 0) {
> +			clear_sympos_symbols(sec, klp_elf);
> +			list_del(&sec->list);
> +			free(sec);
> +			continue;
> +		}
> +		if (strncmp(sec->name, ".rela.klp.module_relocs.", 24) == 0) {
> +			list_del(&sec->list);
> +			free(sec);
> +			continue;
> +		}
> +	}
> +}
> +
> +/* Checks if two or more elements in usr_symbols have the same name */
> +bool sympos_sanity_check(void)
> +{
> +	bool sane = true;
> +	struct sympos *sp, *aux;
> +
> +	list_for_each_entry(sp, &usr_symbols, list) {
> +		aux = list_next_entry(sp, list);
> +		list_for_each_entry_from(aux, &usr_symbols, list) {
> +			if (strcmp(sp->symbol_name, aux->symbol_name) == 0) {
> +				WARN("KLP_SYMPOS Collision: %s in %s and %s.",
> +						sp->symbol_name,
> +						sp->object_name,
> +						aux->object_name);
> +				sane = false;
> +			}
> +		}
> +	}
> +	return sane;
> +}
> +
> +/* Parses the livepatch elf object and fills usr_symbols */
> +bool load_usr_symbols(struct elf *klp_elf)
> +{
> +	char objname[MODULE_NAME_LEN];
> +	struct sympos *sp;
> +	struct section *sec, *aux, *relasec;
> +	struct rela *rela;
> +	struct klp_module_reloc *reloc;
> +	int i, nr_entries;
> +
> +	list_for_each_entry_safe(sec, aux, &klp_elf->sections, list) {
> +		if (sscanf(sec->name, ".klp.module_relocs.%55s", objname) != 1)
> +			continue;
> +
> +		relasec = sec->rela;
> +		reloc = sec->data;
> +		i = 0;
> +		nr_entries = sec->size / sizeof(*reloc);
> +		list_for_each_entry(rela, &relasec->relas, list) {
> +			sp = calloc(1, sizeof(struct sympos));
> +			if (!sp) {
> +				WARN("Unable to allocate sympos memory\n");
> +				return false;
> +			}
> +			sp->object_name = strdup(objname);
> +			if (!sp->object_name) {
> +				WARN("Unable to allocate object name\n");
> +				return false;
> +			}
> +			sp->symbol_name = strdup(rela->sym->name);
> +			sp->pos = reloc[i].sympos;
> +			list_add(&sp->list, &usr_symbols);
> +			i++;
> +		}
> +		if (i != nr_entries) {
> +			WARN("nr_entries mismatch (%d != %d) for %s\n",
> +			     i, nr_entries, relasec->name);
> +			return false;
> +		}
> +	}
> +	clear_sympos_annontations(klp_elf);
> +	return sympos_sanity_check();
> +}
> +
> +/* Dumps sympos list (useful for debugging purposes) */
> +void dump_sympos(void)
> +{
> +	struct sympos *sp;
> +
> +	fprintf(stderr, "BEGIN OF SYMPOS DUMP\n");
> +	list_for_each_entry(sp, &usr_symbols, list) {
> +		fprintf(stderr, "%s %s %d\n", sp->symbol_name, sp->object_name,
> +				sp->pos);
> +	}
> +	fprintf(stderr, "END OF SYMPOS DUMP\n");
> +}
> +
> +/* prints list of valid sympos for symbol with provided name */
> +void print_valid_module_relocs(char *name)
> +{
> +	struct symbol_entry *e;
> +	char *cur_obj = "";
> +	int counter;
> +	bool first = true;
> +
> +	/* Symbols from the same object are locally gathered in the list */
> +	fprintf(stderr, "Valid KLP_SYMPOS for symbol %s:\n", name);
> +	fprintf(stderr, "-------------------------------------------------\n");
> +	list_for_each_entry(e, &symbols, list) {
> +		if (strcmp(e->object_name, cur_obj) != 0) {
> +			cur_obj = e->object_name;
> +			counter = 0;
> +		}
> +		if (strcmp(e->symbol_name, name) == 0) {
> +			if (counter == 0) {
> +				if (!first)
> +					fprintf(stderr, "}\n");
> +
> +				fprintf(stderr, "KLP_MODULE_RELOC(%s){\n",
> +						cur_obj);
> +				first = false;
> +			}
> +			fprintf(stderr, "\tKLP_SYMPOS(%s,%d)\n", name, counter);
> +			counter++;
> +		}
> +	}
> +	fprintf(stderr, "-------------------------------------------------\n");
> +}
> +
> +/* Searches for symbol in symbols list and returns its sympos if it is unique,
> + * otherwise prints a list with all considered valid sympos
> + */
> +struct symbol_entry *find_sym_entry_by_name(char *name)
> +{
> +	struct symbol_entry *found = NULL;
> +	struct symbol_entry *e;
> +
> +	list_for_each_entry(e, &symbols, list) {
> +		if (strcmp(e->symbol_name, name) == 0) {
> +
> +			/* If there exist multiple symbols with the same
> +			 * name then user-provided sympos is required
> +			 */
> +			if (found) {
> +				print_valid_module_relocs(name);
> +				return NULL;
> +			}
> +			found = e;
> +		}
> +	}
> +	if (found)
> +		return found;
> +
> +	return NULL;
> +}
> +
> +/* Checks if sympos is valid, otherwise prints valid sympos list */
> +bool valid_sympos(struct sympos *sp)
> +{
> +	struct symbol_entry *e;
> +	int counter = 0;
> +
> +	list_for_each_entry(e, &symbols, list) {
> +		if ((strcmp(e->symbol_name, sp->symbol_name) == 0) &&
> +		    (strcmp(e->object_name, sp->object_name) == 0)) {
> +			if (counter == sp->pos)
> +				return true;
> +			counter++;
> +		}
> +	}
> +
> +	WARN("Provided KLP_SYMPOS does not match a symbol (%s.%s)",
> +			sp->symbol_name, sp->object_name);
> +	print_valid_module_relocs(sp->symbol_name);
> +
> +	return false;
> +}
> +
> +/* Returns the right sympos respective to a symbol to be relocated */
> +bool find_missing_position(struct symbol *s, struct sympos *sp)
> +{
> +	struct symbol_entry *entry;
> +
> +	if (get_usr_sympos(s, sp)) {
> +		if (valid_sympos(sp))
> +			return true;
> +		return false;
> +	}
> +
> +	/* if no user-provided sympos, search symbol in symbols list */
> +	entry = find_sym_entry_by_name(s->name);
> +	if (entry) {
> +		sp->symbol_name = entry->symbol_name;
> +		sp->object_name = entry->object_name;
> +		sp->pos = 0;
> +		return true;
> +	}
> +	return false;
> +}
> +
> +/* Finds or creates a klp rela section based on another given section (@oldsec)
> + * and sympos (@*sp), then returns it
> + */
> +struct section *get_or_create_klp_rela_section(struct section *oldsec,
> +		struct sympos *sp, struct elf *klp_elf)
> +{
> +	char *name;
> +	struct section *sec;
> +	unsigned int length;
> +
> +	length = strlen(KLP_RELA_PREFIX) + strlen(sp->object_name)
> +		 + strlen(oldsec->base->name) + 2;
> +
> +	name = calloc(1, length);
> +	if (!name) {
> +		WARN("Memory allocation failed (%s%s.%s)\n", KLP_RELA_PREFIX,
> +				sp->object_name, oldsec->base->name);
> +		return NULL;
> +	}
> +
> +	if (snprintf(name, length, KLP_RELA_PREFIX "%s.%s", sp->object_name,
> +				oldsec->base->name) >= length) {
> +		WARN("Length error (%s)", name);
> +		free(name);
> +		return false;
> +	}
> +
> +	sec = find_section_by_name(klp_elf, name);
> +	if (!sec)
> +		sec = create_rela_section(klp_elf, name, oldsec->base);
> +
> +	if (sec)
> +		sec->sh.sh_flags |= SHF_RELA_LIVEPATCH;
> +
> +	free(name);
> +	return sec;
> +}
> +
> +/* Create klp rela, add it to the right klp rela sec and del reference rela */
> +bool convert_rela(struct section *oldsec, struct rela *r, struct sympos *sp,
> +		struct elf *klp_elf)
> +{
> +	char *name;
> +	char pos[4];	/* assume that pos will never be > 999 */
> +	unsigned int length;
> +	struct section *sec;
> +	struct symbol *s = r->sym;
> +
> +	sec = get_or_create_klp_rela_section(oldsec, sp, klp_elf);
> +	if (!sec) {
> +		WARN("Can't create or access klp.rela section (%s.%s)\n",
> +		    sp->object_name, sp->symbol_name);
> +		return false;
> +	}
> +
> +	if (snprintf(pos, sizeof(pos), "%d", sp->pos) > 3) {
> +		WARN("Insuficient buffer for expanding sympos (%s.%s,%d)\n",
> +			sp->object_name, sp->symbol_name, sp->pos);
> +		return false;
> +	}
> +
> +	length = strlen(KLP_SYM_PREFIX) + strlen(sp->object_name)
> +		 + strlen(sp->symbol_name) + 4;
> +
> +	name = calloc(1, length);
> +	if (!name) {
> +		WARN("Memory allocation failed (%s%s.%s,%s)\n", KLP_SYM_PREFIX,
> +				sp->object_name, sp->symbol_name, pos);
> +		return false;
> +	}
> +
> +	if (snprintf(name, length, KLP_SYM_PREFIX "%s.%s,%s", sp->object_name,
> +				sp->symbol_name, pos) >= length) {
> +		WARN("Length error (%s.%s)", sp->object_name, sp->symbol_name);
> +		return false;
> +	}
> +
> +	s->name = name;
> +	s->sec = NULL;
> +	s->sym.st_name = -1;
> +	s->sym.st_shndx = SHN_LIVEPATCH;
> +
> +	list_del(&r->list);
> +	list_add(&r->list, &sec->relas);
> +	return true;
> +}
> +
> +/* Checks if given symbol name matches a symbol in exp_symbols */
> +bool is_exported(char *sname)
> +{
> +	struct symbol_entry *e;
> +
> +	/* exp_symbols itens are prefixed with __ksymtab_ - comparisons must
> +	 * skip prefix and check if both are properly null-terminated
> +	 */
> +	list_for_each_entry(e, &exp_symbols, list) {
> +		if (strcmp(e->symbol_name + 10, sname) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/* Checks if a symbol was previously klp-converted based on its name */
> +bool is_converted(char *sname)
> +{
> +	int len = strlen(KLP_SYM_PREFIX);
> +
> +	if (strncmp(sname, KLP_SYM_PREFIX, len) == 0)
> +		return true;
> +	return false;
> +}
> +
> +/* Checks if symbol must be klp-converted - must not be already resolved, have
> + * been already klp-converted nor be an exported symbol
> + */
> +bool must_convert(struct symbol *sym)
> +{
> +	if (sym->sec)
> +		return false;
> +	return (!(is_converted(sym->name) || is_exported(sym->name)));
> +}
> +
> +int main(int argc, const char **argv)
> +{
> +	const char *klp_in_module, *klp_out_module, *symbols_list;
> +	struct rela *rela, *tmprela;
> +	struct section *sec, *aux;
> +	struct sympos *sp;
> +	struct elf *klp_elf;
> +
> +	if (argc != 4) {
> +		WARN("Usage: %s <Symbols.list> <input.ko> <out.ko>", argv[0]);
> +		return -1;
> +	}
> +
> +	symbols_list = argv[1];
> +	klp_in_module = argv[2];
> +	klp_out_module = argv[3];
> +
> +	klp_elf = elf_open(klp_in_module);
> +	if (!klp_elf) {
> +		WARN("Unable to read elf file %s\n", klp_in_module);
> +		return -1;
> +	}
> +
> +	if (!load_syms_lists(symbols_list))
> +		return -1;
> +
> +	if (!load_usr_symbols(klp_elf)) {
> +		WARN("Unable to load user-provided sympos");
> +		return -1;
> +	}
> +
> +	list_for_each_entry_safe(sec, aux, &klp_elf->sections, list) {
> +		if (!is_rela_section(sec))
> +			continue;
> +		list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
> +			if (!must_convert(rela->sym))
> +				continue;
> +
> +			sp = calloc(1, sizeof(struct sympos));
> +			if (!sp) {
> +				WARN("Unable to allocate memory for sympos");
> +				return -1;
> +			}
> +			if (!find_missing_position(rela->sym, sp)) {
> +				WARN("Unable to find missing symbol");
> +				return -1;
> +			}
> +			if (!convert_rela(sec, rela, sp, klp_elf)) {
> +				WARN("Unable to convert relocation");
> +				return -1;
> +			}
> +			free(sp);
> +		}
> +	}
> +
> +	free_syms_lists();
> +	if (elf_write_file(klp_elf, klp_out_module))
> +		return -1;
> +
> +	return 0;
> +}
> diff --git a/scripts/livepatch/list.h b/scripts/livepatch/list.h
> new file mode 100644
> index 000000000000..b324eb29c3b6
> --- /dev/null
> +++ b/scripts/livepatch/list.h
> @@ -0,0 +1,389 @@
> +#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
> 

[PATCH 4/8] livepatch: Add klp-convert annotation helpers

[Joao Moreira]

From: Josh Poimboeuf <jpoimboe@redhat.com>

Define macros KLP_MODULE_RELOC and KLP_SYMPOS in
include/linux/livepatch.h to improve user-friendliness of the
livepatch annotation process.

[jmoreira:
* split up: move KLP_MODULE_RELOC from previous patch to here
* add KLP_SYMPOS
* move macros from include/uapi/livepatch.h to include/linux/livepatch.h
]

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 include/linux/livepatch.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 96a75be7ef50..3956929f21bc 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -165,6 +165,17 @@ static inline bool klp_have_reliable_stack(void)
 	       IS_ENABLED(CONFIG_HAVE_RELIABLE_STACKTRACE);
 }
 
+/* Used to annotate symbol relocations in livepatches */
+#define KLP_MODULE_RELOC(obj)						\
+	struct klp_module_reloc						\
+	__attribute__((__section__(".klp.module_relocs." #obj)))
+
+#define KLP_SYMPOS(symbol, pos)						\
+	{								\
+		.sym = &symbol,						\
+		.sympos = pos,						\
+	},
+
 #else /* !CONFIG_LIVEPATCH */
 
 static inline int klp_module_coming(struct module *mod) { return 0; }
-- 
2.12.0

[PATCH 5/8] modpost: Integrate klp-convert

[Joao Moreira]

From: Josh Poimboeuf <jpoimboe@redhat.com>

Create cmd_klp_convert and hook it into scripts/Makefile.modpost.
cmd_klp_convert invokes klp-convert with the right arguments for the
conversion of unresolved symbols inside a livepatch.

[khlebnikov:
* 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
* check modinfo -F livepatch only if CONFIG_LIVEPATCH is true
]

[mbenes:
* remove modinfo call. LIVEPATCH_ in Makefile
]

[jmoreira:
* split up: move the .livepatch file-based scheme for identifying
livepatches to a previous patch, as it was required for correctly
building Symbols.list there
* split up: move modinfo call removal by mbenes to the next patch
]

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 scripts/Kbuild.include   |  4 +++-
 scripts/Makefile.modpost | 16 +++++++++++++++-
 scripts/mod/modpost.c    |  6 +++++-
 scripts/mod/modpost.h    |  1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 61f87a99bf0a..9aba82d76659 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -251,6 +251,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 $^),$^)
@@ -260,7 +262,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 16923ba4b5b1..52264ce1b883 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -125,8 +125,22 @@ quiet_cmd_ld_ko_o = LD [M]  $@
                  -o $@ $(filter-out FORCE,$^) ;                         \
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
+SLIST = $(objtree)/Symbols.list
+KLP_CONVERT = scripts/livepatch/klp-convert
+quiet_cmd_klp_convert = LIVEPATCH $@
+      cmd_klp_convert = mv $@ $(@:.ko=.klp.o);				\
+			$(KLP_CONVERT) $(SLIST) $(@:.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 $(wildcard $(MODVERDIR)/$(basetarget).livepatch),	\
+	    $(call echo-cmd,klp_convert) $(cmd_klp_convert) ))
+endef
+
 $(modules): %.ko :%.o %.mod.o FORCE
-	+$(call if_changed,ld_ko_o)
+	+$(call if_changed_rule,ld_ko_o)
 
 targets += $(modules)
 
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 30d752a4a6a6..d9fe972ed92e 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1977,6 +1977,10 @@ static void read_symbols(char *modname)
 					   "license", license);
 	}
 
+	/* Livepatch modules have unresolved symbols resolved 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);
 
@@ -2170,7 +2174,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 6a5e1515123b..dcb32a57be29 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.12.0

[PATCH 6/8] modpost: Add modinfo flag to livepatch modules

[Joao Moreira]

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 in the sample, remove it.

[jmoreira:
* fix modpost.c (add_livepatch_flag) to update module structure with
livepatch flag and prevent modpost from breaking due to unresolved
symbols
* remove MODULE_INFO statement
]

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 samples/livepatch/livepatch-sample.c |  1 -
 scripts/Makefile.modpost             |  8 +++-
 scripts/mod/modpost.c                | 82 +++++++++++++++++++++++++++++++++---
 3 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
index 84795223f15f..8bc6c4f37d77 100644
--- a/samples/livepatch/livepatch-sample.c
+++ b/samples/livepatch/livepatch-sample.c
@@ -105,4 +105,3 @@ static void livepatch_exit(void)
 module_init(livepatch_init);
 module_exit(livepatch_exit);
 MODULE_LICENSE("GPL");
-MODULE_INFO(livepatch, "Y");
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 52264ce1b883..d87464084c44 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -64,6 +64,11 @@ MODLISTCMD := find $(MODVERDIR) -name '*.mod' | xargs -r grep -h '\.ko$$' | sort
 __modules := $(shell $(MODLISTCMD))
 modules   := $(patsubst %.o,%.ko, $(wildcard $(__modules:.ko=.o)))
 
+# find all .livepatch files listed in $(MODVERDIR)/
+ifdef CONFIG_LIVEPATCH
+$(shell find $(MODVERDIR) -name '*.livepatch' | xargs -r -I{} basename {} .livepatch > $(MODVERDIR)/livepatchmods)
+endif
+
 # Stop after building .o files if NOFINAL is set. Makes compile tests quicker
 _modpost: $(if $(KBUILD_MODPOST_NOFINAL), $(modules:.ko:.o),$(modules))
 
@@ -78,7 +83,8 @@ modpost = scripts/mod/modpost                    \
  $(if $(KBUILD_EXTMOD),-o $(modulesymfile))      \
  $(if $(CONFIG_DEBUG_SECTION_MISMATCH),,-S)      \
  $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)  \
- $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
+ $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w) \
+ $(if $(CONFIG_LIVEPATCH),-l $(MODVERDIR)/livepatchmods)
 
 MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
 
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d9fe972ed92e..416737056bc3 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1977,10 +1977,6 @@ static void read_symbols(char *modname)
 					   "license", license);
 	}
 
-	/* Livepatch modules have unresolved symbols resolved 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);
 
@@ -2395,6 +2391,76 @@ static void write_dump(const char *fname)
 	free(buf.p);
 }
 
+struct livepatch_mod_list {
+	struct livepatch_mod_list *next;
+	char *livepatch_mod;
+};
+
+static struct livepatch_mod_list *load_livepatch_mods(const char *fname)
+{
+	struct livepatch_mod_list *list_iter, *list_start = NULL;
+	unsigned long size, pos = 0;
+	void *file = grab_file(fname, &size);
+	char *line;
+
+	if (!file)
+		return NULL;
+
+	while ((line = get_next_line(&pos, file, size))) {
+		list_iter = NOFAIL(malloc(sizeof(*list_iter)));
+		list_iter->next = list_start;
+		list_iter->livepatch_mod = NOFAIL(strdup(line));
+		list_start = list_iter;
+	}
+	release_file(file, size);
+
+	return list_start;
+}
+
+static void free_livepatch_mods(struct livepatch_mod_list *list_start)
+{
+	struct livepatch_mod_list *list_iter;
+
+	while (list_start) {
+		list_iter = list_start->next;
+		free(list_start->livepatch_mod);
+		free(list_start);
+		list_start = list_iter;
+	}
+}
+
+static int is_livepatch_mod(const char *modname,
+		struct livepatch_mod_list *list)
+{
+	const char *myname;
+
+	if (!list)
+		return 0;
+
+	myname = strrchr(modname, '/');
+	if (myname)
+		myname++;
+	else
+		myname = modname;
+
+	while (list) {
+		if (!strcmp(myname, list->livepatch_mod))
+			return 1;
+		list = list->next;
+	}
+	return 0;
+}
+
+static void add_livepatch_flag(struct buffer *b, struct module *mod,
+		struct livepatch_mod_list *list)
+{
+	if (is_livepatch_mod(mod->name, list)) {
+		buf_printf(b, "\nMODULE_INFO(livepatch, \"Y\");\n");
+		mod->livepatch = 1;
+	}
+}
+
+
 struct ext_sym_list {
 	struct ext_sym_list *next;
 	const char *file;
@@ -2410,8 +2476,9 @@ int main(int argc, char **argv)
 	int err;
 	struct ext_sym_list *extsym_iter;
 	struct ext_sym_list *extsym_start = NULL;
+	struct livepatch_mod_list *livepatch_mods = NULL;
 
-	while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awM:K:E")) != -1) {
+	while ((opt = getopt(argc, argv, "i:I:e:l:mnsST:o:awM:K:E")) != -1) {
 		switch (opt) {
 		case 'i':
 			kernel_read = optarg;
@@ -2428,6 +2495,9 @@ int main(int argc, char **argv)
 			extsym_iter->file = optarg;
 			extsym_start = extsym_iter;
 			break;
+		case 'l':
+			livepatch_mods = load_livepatch_mods(optarg);
+			break;
 		case 'm':
 			modversions = 1;
 			break;
@@ -2496,6 +2566,7 @@ int main(int argc, char **argv)
 		add_header(&buf, mod);
 		add_intree_flag(&buf, !external_module);
 		add_staging_flag(&buf, mod->name);
+		add_livepatch_flag(&buf, mod, livepatch_mods);
 		err |= add_versions(&buf, mod);
 		add_depends(&buf, mod, modules);
 		add_moddevtable(&buf, mod);
@@ -2518,6 +2589,7 @@ int main(int argc, char **argv)
 			      "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
 		}
 	}
+	free_livepatch_mods(livepatch_mods);
 	free(buf.p);
 
 	return err;
-- 
2.12.0

[PATCH 7/8] livepatch: Add sample livepatch module

[Joao Moreira]

From: Josh Poimboeuf <jpoimboe@redhat.com>

Add a new livepatch sample in samples/livepatch/ to make use of
symbols that must be post-processed to enable load-time relocation
resolution. As the new sample is to be used as an example, it is
annotated with KLP_MODULE_REOC and with KLP_SYMPOS macros.

The livepatch sample updates the function cmdline_proc_show to
print the string referenced by the symbol saved_command_line
appended by the string "livepatch=1".

[jmoreira:
* Switched from "updating existing" module to "adding a new" module
* Update module to use KLP_SYMPOS
* Comments on symbol resolution scheme
* Update Makefile
* Changelog
]

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 samples/livepatch/Makefile                     |   4 +-
 samples/livepatch/livepatch-annotated-sample.c | 128 +++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 samples/livepatch/livepatch-annotated-sample.c

diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
index 955e7ac12d2b..2690ef439bec 100644
--- a/samples/livepatch/Makefile
+++ b/samples/livepatch/Makefile
@@ -1,2 +1,4 @@
 LIVEPATCH_livepatch-sample.o := y
-obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
+LIVEPATCH_livepatch-annotated-sample.o := y
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o \
+				  livepatch-annotated-sample.o
diff --git a/samples/livepatch/livepatch-annotated-sample.c b/samples/livepatch/livepatch-annotated-sample.c
new file mode 100644
index 000000000000..93da32610076
--- /dev/null
+++ b/samples/livepatch/livepatch-annotated-sample.c
@@ -0,0 +1,128 @@
+/*
+ * livepatch-annotated-sample.c - Kernel Live Patching Sample Module
+ *
+ * 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/>.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include <linux/seq_file.h>
+
+/*
+ * This (dumb) live patch overrides the function that prints the
+ * kernel boot cmdline when /proc/cmdline is read.
+ *
+ * This livepatch uses the symbol saved_command_line whose relocation
+ * must be resolved during load time. To enable that, this module
+ * must be post-processed by a tool called klp-convert, which embeds
+ * information to be used by the loader to solve the relocation.
+ *
+ * The module is annotated with KLP_MODULE_RELOC/KLP_SYMPOS macros.
+ * These annotations are used by klp-convert to infer that the symbol
+ * saved_command_line is in the object vmlinux.
+ *
+ * As saved_command_line has no other homonimous symbol across
+ * kernel objects, this annotation is not a requirement, and can be
+ * suppressed with no harm to klp-convert. Yet, it is kept here as an
+ * example on how to annotate livepatch modules that contain symbols
+ * whose names are used in more than one kernel object.
+ *
+ * Example:
+ *
+ * $ cat /proc/cmdline
+ * <your cmdline>
+ *
+ * $ insmod livepatch-sample.ko
+ * $ cat /proc/cmdline
+ * <your cmdline> livepatch=1
+ *
+ * $ echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled
+ * $ cat /proc/cmdline
+ * <your cmdline>
+ */
+
+extern char *saved_command_line;
+
+static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%s livepatch=1\n", saved_command_line);
+	return 0;
+}
+
+KLP_MODULE_RELOC(vmlinux) vmlinux_relocs[] = {
+	KLP_SYMPOS(saved_command_line, 0)
+};
+
+static struct klp_func vmlinux_funcs[] = {
+	{
+		.old_name = "cmdline_proc_show",
+		.new_func = livepatch_cmdline_proc_show,
+	}, { }
+};
+
+static struct klp_object objs[] = {
+	{
+		/* name being NULL means vmlinux */
+		.funcs = vmlinux_funcs,
+	}, { }
+};
+
+static struct klp_patch patch = {
+	.mod = THIS_MODULE,
+	.objs = objs,
+};
+
+static int livepatch_init(void)
+{
+	int ret;
+
+	if (!klp_have_reliable_stack() && !patch.immediate) {
+		/*
+		 * WARNING: Be very careful when using 'patch.immediate' in
+		 * your patches.  It's ok to use it for simple patches like
+		 * this, but for more complex patches which change function
+		 * semantics, locking semantics, or data structures, it may not
+		 * be safe.  Use of this option will also prevent removal of
+		 * the patch.
+		 *
+		 * See Documentation/livepatch/livepatch.txt for more details.
+		 */
+		patch.immediate = true;
+		pr_notice("The consistency model isn't supported for your architecture.  Bypassing safety mechanisms and applying the patch immediately.\n");
+	}
+
+	ret = klp_register_patch(&patch);
+	if (ret)
+		return ret;
+	ret = klp_enable_patch(&patch);
+	if (ret) {
+		WARN_ON(klp_unregister_patch(&patch));
+		return ret;
+	}
+	return 0;
+}
+
+static void livepatch_exit(void)
+{
+	WARN_ON(klp_unregister_patch(&patch));
+}
+
+module_init(livepatch_init);
+module_exit(livepatch_exit);
+MODULE_LICENSE("GPL");
-- 
2.12.0

[PATCH 8/8] documentation: Update on livepatch elf format

[Joao Moreira]

Add a section to Documentation/livepatch/module-elf-format.txt
describing how klp-convert works for fixing relocations.

Signed-off-by: Joao Moreira <jmoreira@suse.de>
---
 Documentation/livepatch/module-elf-format.txt | 47 ++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/livepatch/module-elf-format.txt b/Documentation/livepatch/module-elf-format.txt
index f21a5289a09c..6b0259dfab49 100644
--- a/Documentation/livepatch/module-elf-format.txt
+++ b/Documentation/livepatch/module-elf-format.txt
@@ -2,7 +2,8 @@
 Livepatch module Elf format
 ===========================
 
-This document outlines the Elf format requirements that livepatch modules must follow.
+This document outlines the Elf format requirements that livepatch modules must
+follow.
 
 -----------------
 Table of Contents
@@ -25,8 +26,9 @@ Table of Contents
        3.3.2 Required name format
        3.3.3 Example livepatch symbol names
        3.3.4 Example `readelf --symbols` output
-4. Architecture-specific sections
-5. Symbol table and Elf section access
+4. Automatic conversion of unresolved relocations
+5. Architecture-specific sections
+6. Symbol table and Elf section access
 
 ----------------------------
 0. Background and motivation
@@ -293,8 +295,43 @@ Symbol table '.symtab' contains 127 entries:
 [*] Note that the 'Ndx' (Section index) for these symbols is SHN_LIVEPATCH (0xff20).
     "OS" means OS-specific.
 
+--------------------------------------------------
+4.  Automatic conversion of unresolved relocations
+--------------------------------------------------
+Sometimes livepatches may operate on symbols which are not self-contained nor
+exported. When this happens, these symbols remain unresolved in the elf object
+and will trigger an error during the livepatch instantiation.
+
+Whenever possible, the kernel building infrastructure solves this problem
+automatically. First, a symbol database containing information on all compiled
+objects is built. Second, this database - a file named Symbols.list, placed in
+the kernel source root directory - is used to identify targets for unresolved
+relocations, converting them in the livepatch elf accordingly to the
+specifications above-described. While the first stage is fully handled by the
+building system, the second is done by a tool called klp-convert, which can be
+found in "scripts/livepatch".
+
+When an unresolved relocation has as target a symbol whose name is also used by
+different symbols throughout the kernel, the relocation cannot be resolved
+automatically. In these cases, the livepatch developer must add annotations to
+the livepatch, making it possible for the system to identify which is the
+correct target amongst multiple homonymous symbols. Such annotations must be
+done through a data structure as follows:
+
+struct KLP_MODULE_RELOC(object) data_structure_name[] = {
+	KLP_SYMPOS(symbol, pos)
+};
+
+In the above example, object refers to the object file which contains the
+symbol, being vmlinux or a module; symbol refers to the symbol name that will
+be relocated and pos is its position in the object.
+
+When a data structure like this is added to the livepatch, the resulting elf
+will hold symbols that will be identified by klp-convert and used to solve name
+ambiguities.
+
 ---------------------------------
-4. Architecture-specific sections
+5. Architecture-specific sections
 ---------------------------------
 Architectures may override arch_klp_init_object_loaded() to perform
 additional arch-specific tasks when a target module loads, such as applying
@@ -305,7 +342,7 @@ be easily identified when iterating through a patch module's Elf sections
 (See arch/x86/kernel/livepatch.c for a complete example).
 
 --------------------------------------
-5. Symbol table and Elf section access
+6. Symbol table and Elf section access
 --------------------------------------
 A livepatch module's symbol table is accessible through module->symtab.
 
-- 
2.12.0

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Tue, Aug 29, 2017 at 04:01:32PM -0300, Joao Moreira wrote:
> Livepatches may use symbols which are not contained in its own scope,
> and, because of that, may end up compiled with relocations that will
> only be resolved during module load. Yet, when the referenced symbols are
> not exported, solving this relocation requires information on the object
> that holds the symbol (either vmlinux or modules) and its position inside
> the object, as an object may contain multiple symbols with the same name.
> Providing such information must be done accordingly to what is specified
> in Documentation/livepatch/module-elf-format.txt.
> 
> Currently, there is no trivial way to embed the required information as
> requested in the final livepatch elf object. klp-convert solves this
> problem in two different forms: (i) by relying on a symbol map, which is
> built during kernel compilation, to automatically infers the relocation
> targeted symbol, and, when such inference is not possible (ii) by using
> annotations in the elf object to convert the relocation accordingly to
> the specification, enabling it to be handled by the livepatch loader.
> 
> Given the above, add support for symbol mapping in the form of
> Symbols.list file; add klp-convert tool; integrate klp-convert tool into
> kbuild; make livepatch modules discernible during kernel compilation
> pipeline; add data-structure and macros to enable users to annotate
> livepatch source code; make modpost stage compatible with livepatches;
> update livepatch-sample and update documentation.
> 
> The patch was tested under three use-cases:
> 
> use-case 1: There is a relocation in the lp that can be automatically
> resolved by klp-convert (tested by removing the annotations from
> samples/livepatch/livepatch-annotated-sample.c)
> 
> use-case 2: There is a relocation in the lp that cannot be automatically
> resolved, as the name of the respective symbol appears in multiple
> objects. The livepatch contains an annotation to enable a correct
> relocation - reproducible with this livepatch sample:
> www.livewire.com.br/suse/klp/livepatch-sample.1.c
> 
> use-case 3: There is a relocation in the lp that cannot be automatically
> resolved similarly as 2, but no annotation was provided in the livepatch,
> triggering an error during compilation - reproducible with this livepatch
> sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c
> 
> Joao Moreira (2):
>   kbuild: Support for Symbols.list creation
>   documentation: Update on livepatch elf format
> 
> Josh Poimboeuf (5):
>   livepatch: Create and include UAPI headers
>   livepatch: Add klp-convert tool
>   livepatch: Add klp-convert annotation helpers
>   modpost: Integrate klp-convert
>   livepatch: Add sample livepatch module
> 
> Miroslav Benes (1):
>   modpost: Add modinfo flag to livepatch modules

Thanks a lot for picking these patches up and improving them.  I've only
glanced at the code, but so far it's looking good.  It may be a few
weeks before a I get a chance to do a proper review.

One quick question, possibly for Miroslav.  Do we have a plan yet for
dealing with GCC optimizations?

  https://lkml.kernel.org/r/20161110161053.heua3abuaekz4yy7@treble

I still like the '-fpreserve-function-abi' idea, but maybe it's not
realistic.

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Miroslav Benes]

On Wed, 30 Aug 2017, Josh Poimboeuf wrote:

> On Tue, Aug 29, 2017 at 04:01:32PM -0300, Joao Moreira wrote:
> > Livepatches may use symbols which are not contained in its own scope,
> > and, because of that, may end up compiled with relocations that will
> > only be resolved during module load. Yet, when the referenced symbols are
> > not exported, solving this relocation requires information on the object
> > that holds the symbol (either vmlinux or modules) and its position inside
> > the object, as an object may contain multiple symbols with the same name.
> > Providing such information must be done accordingly to what is specified
> > in Documentation/livepatch/module-elf-format.txt.
> > 
> > Currently, there is no trivial way to embed the required information as
> > requested in the final livepatch elf object. klp-convert solves this
> > problem in two different forms: (i) by relying on a symbol map, which is
> > built during kernel compilation, to automatically infers the relocation
> > targeted symbol, and, when such inference is not possible (ii) by using
> > annotations in the elf object to convert the relocation accordingly to
> > the specification, enabling it to be handled by the livepatch loader.
> > 
> > Given the above, add support for symbol mapping in the form of
> > Symbols.list file; add klp-convert tool; integrate klp-convert tool into
> > kbuild; make livepatch modules discernible during kernel compilation
> > pipeline; add data-structure and macros to enable users to annotate
> > livepatch source code; make modpost stage compatible with livepatches;
> > update livepatch-sample and update documentation.
> > 
> > The patch was tested under three use-cases:
> > 
> > use-case 1: There is a relocation in the lp that can be automatically
> > resolved by klp-convert (tested by removing the annotations from
> > samples/livepatch/livepatch-annotated-sample.c)
> > 
> > use-case 2: There is a relocation in the lp that cannot be automatically
> > resolved, as the name of the respective symbol appears in multiple
> > objects. The livepatch contains an annotation to enable a correct
> > relocation - reproducible with this livepatch sample:
> > www.livewire.com.br/suse/klp/livepatch-sample.1.c
> > 
> > use-case 3: There is a relocation in the lp that cannot be automatically
> > resolved similarly as 2, but no annotation was provided in the livepatch,
> > triggering an error during compilation - reproducible with this livepatch
> > sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c
> > 
> > Joao Moreira (2):
> >   kbuild: Support for Symbols.list creation
> >   documentation: Update on livepatch elf format
> > 
> > Josh Poimboeuf (5):
> >   livepatch: Create and include UAPI headers
> >   livepatch: Add klp-convert tool
> >   livepatch: Add klp-convert annotation helpers
> >   modpost: Integrate klp-convert
> >   livepatch: Add sample livepatch module
> > 
> > Miroslav Benes (1):
> >   modpost: Add modinfo flag to livepatch modules
> 
> Thanks a lot for picking these patches up and improving them.  I've only
> glanced at the code, but so far it's looking good.  It may be a few
> weeks before a I get a chance to do a proper review.
> 
> One quick question, possibly for Miroslav.  Do we have a plan yet for
> dealing with GCC optimizations?
> 
>   https://lkml.kernel.org/r/20161110161053.heua3abuaekz4yy7@treble
> 
> I still like the '-fpreserve-function-abi' idea, but maybe it's not
> realistic.

I'm sorry for the late response, I failed to reply immediately and then 
completely forgot about it :(

I talked to Martin Jambor from gcc community month ago and he told me it 
could be possible to do. But we need to come up with a good proposal. We 
need a good description of what it should do and provide reasons why we 
need it. I'll talk to him again tomorrow and I'll start to work on the 
proposal.

Ideas are of course more than welcome.

However that might be the easier part. We need to find out what it would 
mean for the whole kernel and its performance.

Regards,
Miroslav

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Tue, Oct 10, 2017 at 04:17:10PM +0200, Miroslav Benes wrote:
> On Wed, 30 Aug 2017, Josh Poimboeuf wrote:
> 
> > On Tue, Aug 29, 2017 at 04:01:32PM -0300, Joao Moreira wrote:
> > > Livepatches may use symbols which are not contained in its own scope,
> > > and, because of that, may end up compiled with relocations that will
> > > only be resolved during module load. Yet, when the referenced symbols are
> > > not exported, solving this relocation requires information on the object
> > > that holds the symbol (either vmlinux or modules) and its position inside
> > > the object, as an object may contain multiple symbols with the same name.
> > > Providing such information must be done accordingly to what is specified
> > > in Documentation/livepatch/module-elf-format.txt.
> > > 
> > > Currently, there is no trivial way to embed the required information as
> > > requested in the final livepatch elf object. klp-convert solves this
> > > problem in two different forms: (i) by relying on a symbol map, which is
> > > built during kernel compilation, to automatically infers the relocation
> > > targeted symbol, and, when such inference is not possible (ii) by using
> > > annotations in the elf object to convert the relocation accordingly to
> > > the specification, enabling it to be handled by the livepatch loader.
> > > 
> > > Given the above, add support for symbol mapping in the form of
> > > Symbols.list file; add klp-convert tool; integrate klp-convert tool into
> > > kbuild; make livepatch modules discernible during kernel compilation
> > > pipeline; add data-structure and macros to enable users to annotate
> > > livepatch source code; make modpost stage compatible with livepatches;
> > > update livepatch-sample and update documentation.
> > > 
> > > The patch was tested under three use-cases:
> > > 
> > > use-case 1: There is a relocation in the lp that can be automatically
> > > resolved by klp-convert (tested by removing the annotations from
> > > samples/livepatch/livepatch-annotated-sample.c)
> > > 
> > > use-case 2: There is a relocation in the lp that cannot be automatically
> > > resolved, as the name of the respective symbol appears in multiple
> > > objects. The livepatch contains an annotation to enable a correct
> > > relocation - reproducible with this livepatch sample:
> > > www.livewire.com.br/suse/klp/livepatch-sample.1.c
> > > 
> > > use-case 3: There is a relocation in the lp that cannot be automatically
> > > resolved similarly as 2, but no annotation was provided in the livepatch,
> > > triggering an error during compilation - reproducible with this livepatch
> > > sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c
> > > 
> > > Joao Moreira (2):
> > >   kbuild: Support for Symbols.list creation
> > >   documentation: Update on livepatch elf format
> > > 
> > > Josh Poimboeuf (5):
> > >   livepatch: Create and include UAPI headers
> > >   livepatch: Add klp-convert tool
> > >   livepatch: Add klp-convert annotation helpers
> > >   modpost: Integrate klp-convert
> > >   livepatch: Add sample livepatch module
> > > 
> > > Miroslav Benes (1):
> > >   modpost: Add modinfo flag to livepatch modules
> > 
> > Thanks a lot for picking these patches up and improving them.  I've only
> > glanced at the code, but so far it's looking good.  It may be a few
> > weeks before a I get a chance to do a proper review.
> > 
> > One quick question, possibly for Miroslav.  Do we have a plan yet for
> > dealing with GCC optimizations?
> > 
> >   https://lkml.kernel.org/r/20161110161053.heua3abuaekz4yy7@treble
> > 
> > I still like the '-fpreserve-function-abi' idea, but maybe it's not
> > realistic.
> 
> I'm sorry for the late response, I failed to reply immediately and then 
> completely forgot about it :(

No worries...

> I talked to Martin Jambor from gcc community month ago and he told me it 
> could be possible to do. But we need to come up with a good proposal. We 
> need a good description of what it should do and provide reasons why we 
> need it. I'll talk to him again tomorrow and I'll start to work on the 
> proposal.

Sounds good!  For klp-convert to be successful, we really need a
strategy for dealing with such optimizations.  I'm thinking that a
'-fpreserve-function-abi' flag would be the cleanest way to handle it.

If we don't have a strategy for dealing with optimizations, then we may
instead need to go with a binary diff-based tool like kpatch-build.

> Ideas are of course more than welcome.
> 
> However that might be the easier part. We need to find out what it would 
> mean for the whole kernel and its performance.

TBH, I'd be surprised if it affected kernel performance in any
measurable way.  The vast majority of non-inlined functions seem to
conform to function ABI.

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Joao Moreira]

On 10/10/2017 11:46 PM, Josh Poimboeuf wrote:
> On Tue, Oct 10, 2017 at 04:17:10PM +0200, Miroslav Benes wrote:
>> On Wed, 30 Aug 2017, Josh Poimboeuf wrote:
>>
>>> On Tue, Aug 29, 2017 at 04:01:32PM -0300, Joao Moreira wrote:
>>>> Livepatches may use symbols which are not contained in its own scope,
>>>> and, because of that, may end up compiled with relocations that will
>>>> only be resolved during module load. Yet, when the referenced symbols are
>>>> not exported, solving this relocation requires information on the object
>>>> that holds the symbol (either vmlinux or modules) and its position inside
>>>> the object, as an object may contain multiple symbols with the same name.
>>>> Providing such information must be done accordingly to what is specified
>>>> in Documentation/livepatch/module-elf-format.txt.
>>>>
>>>> Currently, there is no trivial way to embed the required information as
>>>> requested in the final livepatch elf object. klp-convert solves this
>>>> problem in two different forms: (i) by relying on a symbol map, which is
>>>> built during kernel compilation, to automatically infers the relocation
>>>> targeted symbol, and, when such inference is not possible (ii) by using
>>>> annotations in the elf object to convert the relocation accordingly to
>>>> the specification, enabling it to be handled by the livepatch loader.
>>>>
>>>> Given the above, add support for symbol mapping in the form of
>>>> Symbols.list file; add klp-convert tool; integrate klp-convert tool into
>>>> kbuild; make livepatch modules discernible during kernel compilation
>>>> pipeline; add data-structure and macros to enable users to annotate
>>>> livepatch source code; make modpost stage compatible with livepatches;
>>>> update livepatch-sample and update documentation.
>>>>
>>>> The patch was tested under three use-cases:
>>>>
>>>> use-case 1: There is a relocation in the lp that can be automatically
>>>> resolved by klp-convert (tested by removing the annotations from
>>>> samples/livepatch/livepatch-annotated-sample.c)
>>>>
>>>> use-case 2: There is a relocation in the lp that cannot be automatically
>>>> resolved, as the name of the respective symbol appears in multiple
>>>> objects. The livepatch contains an annotation to enable a correct
>>>> relocation - reproducible with this livepatch sample:
>>>> www.livewire.com.br/suse/klp/livepatch-sample.1.c
>>>>
>>>> use-case 3: There is a relocation in the lp that cannot be automatically
>>>> resolved similarly as 2, but no annotation was provided in the livepatch,
>>>> triggering an error during compilation - reproducible with this livepatch
>>>> sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c
>>>>
>>>> Joao Moreira (2):
>>>>    kbuild: Support for Symbols.list creation
>>>>    documentation: Update on livepatch elf format
>>>>
>>>> Josh Poimboeuf (5):
>>>>    livepatch: Create and include UAPI headers
>>>>    livepatch: Add klp-convert tool
>>>>    livepatch: Add klp-convert annotation helpers
>>>>    modpost: Integrate klp-convert
>>>>    livepatch: Add sample livepatch module
>>>>
>>>> Miroslav Benes (1):
>>>>    modpost: Add modinfo flag to livepatch modules
>>>
>>> Thanks a lot for picking these patches up and improving them.  I've only
>>> glanced at the code, but so far it's looking good.  It may be a few
>>> weeks before a I get a chance to do a proper review.
>>>
>>> One quick question, possibly for Miroslav.  Do we have a plan yet for
>>> dealing with GCC optimizations?
>>>
>>>    https://lkml.kernel.org/r/20161110161053.heua3abuaekz4yy7@treble
>>>
>>> I still like the '-fpreserve-function-abi' idea, but maybe it's not
>>> realistic.
>>
>> I'm sorry for the late response, I failed to reply immediately and then
>> completely forgot about it :(
> 
> No worries...
> 
>> I talked to Martin Jambor from gcc community month ago and he told me it
>> could be possible to do. But we need to come up with a good proposal. We
>> need a good description of what it should do and provide reasons why we
>> need it. I'll talk to him again tomorrow and I'll start to work on the
>> proposal.
> 
> Sounds good!  For klp-convert to be successful, we really need a
> strategy for dealing with such optimizations.  I'm thinking that a
> '-fpreserve-function-abi' flag would be the cleanest way to handle it.
> 
> If we don't have a strategy for dealing with optimizations, then we may
> instead need to go with a binary diff-based tool like kpatch-build.

I'm currently looking into binary diff-based solutions to deal with this 
problem. My plan is to submit a second patch set once I have it 
functional and land both things (klp-convert and bin-diff) in two 
different steps.

Is there any issue with following this schedule? Meaning, do you guys 
still plan on reviewing this patch set or do you prefer me to do 
something differently in terms of approach?

> 
>> Ideas are of course more than welcome.
>>
>> However that might be the easier part. We need to find out what it would
>> mean for the whole kernel and its performance.
> 
> TBH, I'd be surprised if it affected kernel performance in any
> measurable way.  The vast majority of non-inlined functions seem to
> conform to function ABI.
> 

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Wed, Oct 11, 2017 at 09:42:09AM -0300, Joao Moreira wrote:
> > Sounds good!  For klp-convert to be successful, we really need a
> > strategy for dealing with such optimizations.  I'm thinking that a
> > '-fpreserve-function-abi' flag would be the cleanest way to handle it.
> > 
> > If we don't have a strategy for dealing with optimizations, then we may
> > instead need to go with a binary diff-based tool like kpatch-build.
> 
> I'm currently looking into binary diff-based solutions to deal with this
> problem. My plan is to submit a second patch set once I have it functional
> and land both things (klp-convert and bin-diff) in two different steps.

Instead of having multiple approaches, I'd strongly prefer that we
converge on a single in-tree approach that works for everybody.

(Whether that will be source-based like klp-convert or binary-based like
kpatch-build, I don't know.)

BTW, what is bin-diff?  Have you seen kpatch-build?

> Is there any issue with following this schedule? Meaning, do you guys still
> plan on reviewing this patch set or do you prefer me to do something
> differently in terms of approach?

IMO, klp-convert will only be useful if we have a realistic strategy for
dealing with GCC optimizations.  So I'd say we should follow through on
that with the compiler folks before spending too much more time on it.

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Miroslav Benes]

On Thu, 19 Oct 2017, Josh Poimboeuf wrote:

> On Wed, Oct 11, 2017 at 09:42:09AM -0300, Joao Moreira wrote:
> > > Sounds good!  For klp-convert to be successful, we really need a
> > > strategy for dealing with such optimizations.  I'm thinking that a
> > > '-fpreserve-function-abi' flag would be the cleanest way to handle it.
> > > 
> > > If we don't have a strategy for dealing with optimizations, then we may
> > > instead need to go with a binary diff-based tool like kpatch-build.
> > 
> > I'm currently looking into binary diff-based solutions to deal with this
> > problem. My plan is to submit a second patch set once I have it functional
> > and land both things (klp-convert and bin-diff) in two different steps.
> 
> Instead of having multiple approaches, I'd strongly prefer that we
> converge on a single in-tree approach that works for everybody.
> 
> (Whether that will be source-based like klp-convert or binary-based like
> kpatch-build, I don't know.)

I think that klp-convert can work with both. Even with non-source-based 
solution you need something to generate those relocation records. I 
consider klp-convert as a part of the building pipeline.

> BTW, what is bin-diff?  Have you seen kpatch-build?

I'm speaking for Joao here, but we discussed this personally and I think 
he meant approach based on asmtool 
(https://github.com/joergroedel/asmtool). We'd like to explore as much as 
possible.

We also considered complete source-based solution. Nicolai Stange works on 
that (or at least on something which would make it possible).

We can decide what to have in upstream afterwards. But I still think that 
klp-convert will be part of it in some form. Am I missing something?
 
> > Is there any issue with following this schedule? Meaning, do you guys still
> > plan on reviewing this patch set or do you prefer me to do something
> > differently in terms of approach?
> 
> IMO, klp-convert will only be useful if we have a realistic strategy for
> dealing with GCC optimizations.  So I'd say we should follow through on
> that with the compiler folks before spending too much more time on it.

Yes, I'm all for a solution on GCC side, but that may take a while and 
even then it is still a huge step to get it into a distribution (we have 
GCC 4.8.5 in SLE12 :)).

However, there is an easy temporary solution. You can add all 
referenced optimized functions to a livepatch and let klp-convert process 
the rest.

Miroslav

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Thu, Oct 19, 2017 at 03:24:51PM +0200, Miroslav Benes wrote:
> On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> 
> > On Wed, Oct 11, 2017 at 09:42:09AM -0300, Joao Moreira wrote:
> > > > Sounds good!  For klp-convert to be successful, we really need a
> > > > strategy for dealing with such optimizations.  I'm thinking that a
> > > > '-fpreserve-function-abi' flag would be the cleanest way to handle it.
> > > > 
> > > > If we don't have a strategy for dealing with optimizations, then we may
> > > > instead need to go with a binary diff-based tool like kpatch-build.
> > > 
> > > I'm currently looking into binary diff-based solutions to deal with this
> > > problem. My plan is to submit a second patch set once I have it functional
> > > and land both things (klp-convert and bin-diff) in two different steps.
> > 
> > Instead of having multiple approaches, I'd strongly prefer that we
> > converge on a single in-tree approach that works for everybody.
> > 
> > (Whether that will be source-based like klp-convert or binary-based like
> > kpatch-build, I don't know.)
> 
> I think that klp-convert can work with both. Even with non-source-based 
> solution you need something to generate those relocation records. I 
> consider klp-convert as a part of the building pipeline.

Hm.  If I understand correctly, the binary diff tool (or some tool in
the pipeline) would create the .klp.module_relocs.* section, and then
klp-convert would convert that to the .klp.sym.* and .klp.rela.*
sections which livepatch needs.

But if the original tool is creating a relocation section, can't it
instead just create the livepatch .klp.* sections directly?  What's the
benefit of the extra conversion step?

> > BTW, what is bin-diff?  Have you seen kpatch-build?
> 
> I'm speaking for Joao here, but we discussed this personally and I think 
> he meant approach based on asmtool 
> (https://github.com/joergroedel/asmtool). We'd like to explore as much as 
> possible.

Ok, I'd be interested in seeing that, and also what its benefits are
compared to kpatch-build.

> We also considered complete source-based solution. Nicolai Stange works on 
> that (or at least on something which would make it possible).

What is a complete source-based solution?  Is it just "klp-convert +
some GCC optimization strategy" or is it something more?

> We can decide what to have in upstream afterwards. But I still think that 
> klp-convert will be part of it in some form. Am I missing something?

I guess it really depends on what the solution looks like.  If we
decided on kpatch-build (or some variation of its tooling) then we might
not need klp-convert.

> > > Is there any issue with following this schedule? Meaning, do you guys still
> > > plan on reviewing this patch set or do you prefer me to do something
> > > differently in terms of approach?
> > 
> > IMO, klp-convert will only be useful if we have a realistic strategy for
> > dealing with GCC optimizations.  So I'd say we should follow through on
> > that with the compiler folks before spending too much more time on it.
> 
> Yes, I'm all for a solution on GCC side, but that may take a while and 
> even then it is still a huge step to get it into a distribution (we have 
> GCC 4.8.5 in SLE12 :)).
> 
> However, there is an easy temporary solution. You can add all 
> referenced optimized functions to a livepatch and let klp-convert process 
> the rest.

How do you find all referenced optimized functions?

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Miroslav Benes]

On Thu, 19 Oct 2017, Josh Poimboeuf wrote:

> On Thu, Oct 19, 2017 at 03:24:51PM +0200, Miroslav Benes wrote:
> > On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > 
> > > On Wed, Oct 11, 2017 at 09:42:09AM -0300, Joao Moreira wrote:
> > > > > Sounds good!  For klp-convert to be successful, we really need a
> > > > > strategy for dealing with such optimizations.  I'm thinking that a
> > > > > '-fpreserve-function-abi' flag would be the cleanest way to handle it.
> > > > > 
> > > > > If we don't have a strategy for dealing with optimizations, then we may
> > > > > instead need to go with a binary diff-based tool like kpatch-build.
> > > > 
> > > > I'm currently looking into binary diff-based solutions to deal with this
> > > > problem. My plan is to submit a second patch set once I have it functional
> > > > and land both things (klp-convert and bin-diff) in two different steps.
> > > 
> > > Instead of having multiple approaches, I'd strongly prefer that we
> > > converge on a single in-tree approach that works for everybody.
> > > 
> > > (Whether that will be source-based like klp-convert or binary-based like
> > > kpatch-build, I don't know.)
> > 
> > I think that klp-convert can work with both. Even with non-source-based 
> > solution you need something to generate those relocation records. I 
> > consider klp-convert as a part of the building pipeline.
> 
> Hm.  If I understand correctly, the binary diff tool (or some tool in
> the pipeline) would create the .klp.module_relocs.* section, and then
> klp-convert would convert that to the .klp.sym.* and .klp.rela.*
> sections which livepatch needs.
> 
> But if the original tool is creating a relocation section, can't it
> instead just create the livepatch .klp.* sections directly?  What's the
> benefit of the extra conversion step?

I haven't seen this patch set for a while (which is embarassing), but 
klp-convert tries to generate needed sections automatically without 
.klp.module_relocs.* section. Only when there is an ambiguity which cannot 
be solved automatically, manual annotation (KLP_MODULE_RELOC) is needed. 
In that case klp-convert provides hints what needs to be done.

> > > BTW, what is bin-diff?  Have you seen kpatch-build?
> > 
> > I'm speaking for Joao here, but we discussed this personally and I think 
> > he meant approach based on asmtool 
> > (https://github.com/joergroedel/asmtool). We'd like to explore as much as 
> > possible.
> 
> Ok, I'd be interested in seeing that, and also what its benefits are
> compared to kpatch-build.
> 
> > We also considered complete source-based solution. Nicolai Stange works on 
> > that (or at least on something which would make it possible).
> 
> What is a complete source-based solution?  Is it just "klp-convert +
> some GCC optimization strategy" or is it something more?

There's more. You'd give the tool a fix (patch, diff) and kernel sources, 
and it would automatically generate a source code of its livepatch. If 
possible (and there are some obstacles), there would be an advantage 
compared to kpatch-build or different asm/obj-based solution. You could 
verify the result and its correctness. It could also be beneficial if we'd 
like to pursue automatic verification in the future.

> > We can decide what to have in upstream afterwards. But I still think that 
> > klp-convert will be part of it in some form. Am I missing something?
> 
> I guess it really depends on what the solution looks like.  If we
> decided on kpatch-build (or some variation of its tooling) then we might
> not need klp-convert.

That's possible.

> > > > Is there any issue with following this schedule? Meaning, do you guys still
> > > > plan on reviewing this patch set or do you prefer me to do something
> > > > differently in terms of approach?
> > > 
> > > IMO, klp-convert will only be useful if we have a realistic strategy for
> > > dealing with GCC optimizations.  So I'd say we should follow through on
> > > that with the compiler folks before spending too much more time on it.
> > 
> > Yes, I'm all for a solution on GCC side, but that may take a while and 
> > even then it is still a huge step to get it into a distribution (we have 
> > GCC 4.8.5 in SLE12 :)).
> > 
> > However, there is an easy temporary solution. You can add all 
> > referenced optimized functions to a livepatch and let klp-convert process 
> > the rest.
> 
> How do you find all referenced optimized functions?

I guess that since there is no connection between a symbol and its 
optimized counterpart, klp-convert warns about this.

Joao, is this correct?

I understand your position and I agree that klp-convert may become 
superfluous in the future. Maybe not. And maybe the future is far away. 
Anyway, it looks useful in its current form and it would help tremendously 
at least here at SUSE, which is the reason Joao worked on it and send it 
upstream.

Miroslav

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Thu, Oct 19, 2017 at 04:27:31PM +0200, Miroslav Benes wrote:
> On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > > I think that klp-convert can work with both. Even with non-source-based 
> > > solution you need something to generate those relocation records. I 
> > > consider klp-convert as a part of the building pipeline.
> > 
> > Hm.  If I understand correctly, the binary diff tool (or some tool in
> > the pipeline) would create the .klp.module_relocs.* section, and then
> > klp-convert would convert that to the .klp.sym.* and .klp.rela.*
> > sections which livepatch needs.
> > 
> > But if the original tool is creating a relocation section, can't it
> > instead just create the livepatch .klp.* sections directly?  What's the
> > benefit of the extra conversion step?
> 
> I haven't seen this patch set for a while (which is embarassing), but 
> klp-convert tries to generate needed sections automatically without 
> .klp.module_relocs.* section. Only when there is an ambiguity which cannot 
> be solved automatically, manual annotation (KLP_MODULE_RELOC) is needed. 
> In that case klp-convert provides hints what needs to be done.

Ah right, I forgot about that improvement to the patches.  But wouldn't
the binary diff tool still have to do the manual KLP_MODULE_RELOC
annotation when it's needed?  If so, then I still don't see the benefit
of the extra conversion step.

> > > We also considered complete source-based solution. Nicolai Stange works on 
> > > that (or at least on something which would make it possible).
> > 
> > What is a complete source-based solution?  Is it just "klp-convert +
> > some GCC optimization strategy" or is it something more?
> 
> There's more. You'd give the tool a fix (patch, diff) and kernel sources, 
> and it would automatically generate a source code of its livepatch. If 
> possible (and there are some obstacles), there would be an advantage 
> compared to kpatch-build or different asm/obj-based solution.

Sounds nice, though I wonder what the obstacles are?

> You could verify the result and its correctness.

Does that mean it's easier to do code review?  Or something else?

> It could also be beneficial if we'd like to pursue automatic
> verification in the future.

What do you mean by automatic verification?

> > > > IMO, klp-convert will only be useful if we have a realistic strategy for
> > > > dealing with GCC optimizations.  So I'd say we should follow through on
> > > > that with the compiler folks before spending too much more time on it.
> > > 
> > > Yes, I'm all for a solution on GCC side, but that may take a while and 
> > > even then it is still a huge step to get it into a distribution (we have 
> > > GCC 4.8.5 in SLE12 :)).
> > > 
> > > However, there is an easy temporary solution. You can add all 
> > > referenced optimized functions to a livepatch and let klp-convert process 
> > > the rest.
> > 
> > How do you find all referenced optimized functions?
> 
> I guess that since there is no connection between a symbol and its 
> optimized counterpart, klp-convert warns about this.
> 
> Joao, is this correct?

I'm very confused by this.  You're the expert, so please set me straight :-)

I think you're talking about functions whose symbols have been renamed
by GCC and have been given an optimized suffix, like ".isra" or
".constprop", right?  Wasn't one of your main points at Plumbers last
year that not all such function-ABI-breaking optimizations result in a
rename of the symbol?

> I understand your position and I agree that klp-convert may become 
> superfluous in the future. Maybe not. And maybe the future is far away. 
> Anyway, it looks useful in its current form and it would help tremendously 
> at least here at SUSE, which is the reason Joao worked on it and send it 
> upstream.

My main objection to merging klp-convert in its current state is that
it's not useful by itself.  In fact, it's actively dangerous if people
assume that because it's in-tree, it's the definitive way to safely
create patches.

I have a similar worry about the livepatch-sample module.  It's also
actively dangerous.  Its only decent justification for being in-tree,
IMO, is that we at least need some type of in-tree user of the klp
interfaces.

(But maybe we could solve that by converting livepatch-sample to
selftests.  That would also have another benefit of giving us some
automated test coverage.)

klp-convert is a vast improvement to the livepatch-sample module, but I
view that as a bad thing because it makes it a lot easier to do
something stupid ;-)

If it were part of a complete solution, with some supporting tooling
and/or documentation which prevent the user from making dumb mistakes,
then I think it would make sense to merge it.

Anyway I appreciate all the research efforts you guys are doing.  All
the different options sound promising.

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Miroslav Benes]

On Thu, 19 Oct 2017, Josh Poimboeuf wrote:

> On Thu, Oct 19, 2017 at 04:27:31PM +0200, Miroslav Benes wrote:
> > On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > > > I think that klp-convert can work with both. Even with non-source-based 
> > > > solution you need something to generate those relocation records. I 
> > > > consider klp-convert as a part of the building pipeline.
> > > 
> > > Hm.  If I understand correctly, the binary diff tool (or some tool in
> > > the pipeline) would create the .klp.module_relocs.* section, and then
> > > klp-convert would convert that to the .klp.sym.* and .klp.rela.*
> > > sections which livepatch needs.
> > > 
> > > But if the original tool is creating a relocation section, can't it
> > > instead just create the livepatch .klp.* sections directly?  What's the
> > > benefit of the extra conversion step?
> > 
> > I haven't seen this patch set for a while (which is embarassing), but 
> > klp-convert tries to generate needed sections automatically without 
> > .klp.module_relocs.* section. Only when there is an ambiguity which cannot 
> > be solved automatically, manual annotation (KLP_MODULE_RELOC) is needed. 
> > In that case klp-convert provides hints what needs to be done.
> 
> Ah right, I forgot about that improvement to the patches.  But wouldn't
> the binary diff tool still have to do the manual KLP_MODULE_RELOC
> annotation when it's needed?  If so, then I still don't see the benefit
> of the extra conversion step.

In a way, yes. It depends on the tool. I always pictured it as a pipeline 
(similar to toolchain) and klp-convert as one of its blocks.
 
> > > > We also considered complete source-based solution. Nicolai Stange works on 
> > > > that (or at least on something which would make it possible).
> > > 
> > > What is a complete source-based solution?  Is it just "klp-convert +
> > > some GCC optimization strategy" or is it something more?
> > 
> > There's more. You'd give the tool a fix (patch, diff) and kernel sources, 
> > and it would automatically generate a source code of its livepatch. If 
> > possible (and there are some obstacles), there would be an advantage 
> > compared to kpatch-build or different asm/obj-based solution.
> 
> Sounds nice, though I wonder what the obstacles are?

Those GCC optimizations you mentioned below and which I didn't connect to 
klp-convert itself. More on that later.

Nothing serious aside from that, I hope. Nicolai is currently implementing 
C parser for kernel sources.
 
> > You could verify the result and its correctness.
> 
> Does that mean it's easier to do code review?  Or something else?

Yes, the code review.

> > It could also be beneficial if we'd like to pursue automatic
> > verification in the future.
> 
> What do you mean by automatic verification?

Formal verification. Theoretically we could have a formal specification of 
our consistency model and we could prove/disprove whether a livepatch and 
its implementation are correct with respect to it. It is a vague idea 
though and I personally haven't got sufficient knowledge to do anything 
about it.

> > > > > IMO, klp-convert will only be useful if we have a realistic strategy for
> > > > > dealing with GCC optimizations.  So I'd say we should follow through on
> > > > > that with the compiler folks before spending too much more time on it.
> > > > 
> > > > Yes, I'm all for a solution on GCC side, but that may take a while and 
> > > > even then it is still a huge step to get it into a distribution (we have 
> > > > GCC 4.8.5 in SLE12 :)).
> > > > 
> > > > However, there is an easy temporary solution. You can add all 
> > > > referenced optimized functions to a livepatch and let klp-convert process 
> > > > the rest.
> > > 
> > > How do you find all referenced optimized functions?
> > 
> > I guess that since there is no connection between a symbol and its 
> > optimized counterpart, klp-convert warns about this.
> > 
> > Joao, is this correct?
> 
> I'm very confused by this.  You're the expert, so please set me straight :-)

Oh, am I now? :)

> I think you're talking about functions whose symbols have been renamed
> by GCC and have been given an optimized suffix, like ".isra" or
> ".constprop", right?  Wasn't one of your main points at Plumbers last
> year that not all such function-ABI-breaking optimizations result in a
> rename of the symbol?

I misunderstood your question then. Yes, I was talking about changed 
symbol names, while you asked about something I had in a different block 
of "pipeline".

So yes, it is still a problem, which can be easily solved by asm/obj-based 
approach and not so easily by source-based approach.

> > I understand your position and I agree that klp-convert may become 
> > superfluous in the future. Maybe not. And maybe the future is far away. 
> > Anyway, it looks useful in its current form and it would help tremendously 
> > at least here at SUSE, which is the reason Joao worked on it and send it 
> > upstream.
> 
> My main objection to merging klp-convert in its current state is that
> it's not useful by itself.  In fact, it's actively dangerous if people
> assume that because it's in-tree, it's the definitive way to safely
> create patches.
> 
> I have a similar worry about the livepatch-sample module.  It's also
> actively dangerous.  Its only decent justification for being in-tree,
> IMO, is that we at least need some type of in-tree user of the klp
> interfaces.

Well, you could use this reasoning even for kernel livepatching codebase 
itself. It is hard to use it right, but it is there and thus dangerous.

> (But maybe we could solve that by converting livepatch-sample to
> selftests.  That would also have another benefit of giving us some
> automated test coverage.)

Yes, that would be great.

> klp-convert is a vast improvement to the livepatch-sample module, but I
> view that as a bad thing because it makes it a lot easier to do
> something stupid ;-)
> 
> If it were part of a complete solution, with some supporting tooling
> and/or documentation which prevent the user from making dumb mistakes,
> then I think it would make sense to merge it.

Right, so this is where our views differ a bit. I'd like to get to the 
finish line (whatever that means) slowly but steady and not to wait for 
the ultimate solution if it can be implemented step by step. 

I think it is time for others to express their opinions. We should talk 
about it next week at OSS.

> Anyway I appreciate all the research efforts you guys are doing.  All
> the different options sound promising.

Yeah, I think it is important to try as much as possible.

Thanks,
Miroslav

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Thu, Oct 19, 2017 at 06:00:54PM +0200, Miroslav Benes wrote:
> On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > My main objection to merging klp-convert in its current state is that
> > it's not useful by itself.  In fact, it's actively dangerous if people
> > assume that because it's in-tree, it's the definitive way to safely
> > create patches.
> > 
> > I have a similar worry about the livepatch-sample module.  It's also
> > actively dangerous.  Its only decent justification for being in-tree,
> > IMO, is that we at least need some type of in-tree user of the klp
> > interfaces.
> 
> Well, you could use this reasoning even for kernel livepatching codebase 
> itself. It is hard to use it right, but it is there and thus dangerous.

Indeed, and this is exactly why we've been working on the kpatch author
guide:

  https://github.com/dynup/kpatch/blob/master/doc/patch-author-guide.md

It's currently kpatch-specific, but it mostly applies to livepatch as
well.  It needs to be "ported" to livepatch and moved upstream.

But with klp-convert, there's no such documentation, because there's no
safe way to use it without other supplementary tooling which doesn't
exist.

> > klp-convert is a vast improvement to the livepatch-sample module, but I
> > view that as a bad thing because it makes it a lot easier to do
> > something stupid ;-)
> > 
> > If it were part of a complete solution, with some supporting tooling
> > and/or documentation which prevent the user from making dumb mistakes,
> > then I think it would make sense to merge it.
> 
> Right, so this is where our views differ a bit. I'd like to get to the 
> finish line (whatever that means) slowly but steady and not to wait for 
> the ultimate solution if it can be implemented step by step. 

An iterative approach makes a lot of sense.  But if the intermediate
steps aren't useful, does it make sense for them to be in mainline?
Can't we do development in another tree?

> I think it is time for others to express their opinions. We should talk 
> about it next week at OSS.

Yes, maybe over a pilsner or two...

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Miroslav Benes]

On Thu, 19 Oct 2017, Josh Poimboeuf wrote:

> On Thu, Oct 19, 2017 at 06:00:54PM +0200, Miroslav Benes wrote:
> > On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > > My main objection to merging klp-convert in its current state is that
> > > it's not useful by itself.  In fact, it's actively dangerous if people
> > > assume that because it's in-tree, it's the definitive way to safely
> > > create patches.
> > > 
> > > I have a similar worry about the livepatch-sample module.  It's also
> > > actively dangerous.  Its only decent justification for being in-tree,
> > > IMO, is that we at least need some type of in-tree user of the klp
> > > interfaces.
> > 
> > Well, you could use this reasoning even for kernel livepatching codebase 
> > itself. It is hard to use it right, but it is there and thus dangerous.
> 
> Indeed, and this is exactly why we've been working on the kpatch author
> guide:
> 
>   https://github.com/dynup/kpatch/blob/master/doc/patch-author-guide.md
> 
> It's currently kpatch-specific, but it mostly applies to livepatch as
> well.  It needs to be "ported" to livepatch and moved upstream.

Great, I'll read it.

> But with klp-convert, there's no such documentation, because there's no
> safe way to use it without other supplementary tooling which doesn't
> exist.

True.

I don't know if I mentioned it before. There is -fdump-ipa-clones now in 
GCC for dumping every clone operation GCC does.

https://github.com/marxin/kgraft-analysis-tool processes the dump and 
extracts useful information for a symbol.
 
> > > klp-convert is a vast improvement to the livepatch-sample module, but I
> > > view that as a bad thing because it makes it a lot easier to do
> > > something stupid ;-)
> > > 
> > > If it were part of a complete solution, with some supporting tooling
> > > and/or documentation which prevent the user from making dumb mistakes,
> > > then I think it would make sense to merge it.
> > 
> > Right, so this is where our views differ a bit. I'd like to get to the 
> > finish line (whatever that means) slowly but steady and not to wait for 
> > the ultimate solution if it can be implemented step by step. 
> 
> An iterative approach makes a lot of sense.  But if the intermediate
> steps aren't useful, does it make sense for them to be in mainline?
> Can't we do development in another tree?

Right, that's of course an option too.

> > I think it is time for others to express their opinions. We should talk 
> > about it next week at OSS.
> 
> Yes, maybe over a pilsner or two...

Sure :)

Miroslav

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Fri, Oct 20, 2017 at 10:51:40AM +0200, Miroslav Benes wrote:
> > But with klp-convert, there's no such documentation, because there's no
> > safe way to use it without other supplementary tooling which doesn't
> > exist.
> 
> True.
> 
> I don't know if I mentioned it before. There is -fdump-ipa-clones now in 
> GCC for dumping every clone operation GCC does.
> 
> https://github.com/marxin/kgraft-analysis-tool processes the dump and 
> extracts useful information for a symbol.

Thanks, that looks really nice.  I see that my GCC 7 has
-fdump-ipa-clones.

It would be great if we could integrate that script's functionality with
klp-convert somehow, such that it would warn if you're patching a
function or a relocation which is affected.

Or at least document how a human could do so.

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Torsten Duwe]

On Thu, Oct 19, 2017 at 06:00:54PM +0200, Miroslav Benes wrote:
> On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > 
> > Sounds nice, though I wonder what the obstacles are?
> 
> Those GCC optimizations you mentioned below and which I didn't connect to 
> klp-convert itself.

I have a bad feeling about the IPA stuff in general. An obj-based approach
is cool in a way that it still works, and is sure to work, if the IPA
assumptions that led to the optimisations still hold, but as soon as they
break, you're screwed big time. For -fpatchable-function-entries I switched
off IPA-RA, as especially on RISC there's _nothing_ you can do between
functions without at least one scratch reg. But for live patching, I'd like
the kernel to be compiled in the first place with 100% ABI adherence, IOW
all IPA optimisations turned off. Does anyone have numbers on the performance
impact?

> Nothing serious aside from that, I hope. Nicolai is currently implementing 
> C parser for kernel sources.
>  
> > > You could verify the result and its correctness.
> > 
> > Does that mean it's easier to do code review?  Or something else?
> 
> Yes, the code review.
> 
> > > It could also be beneficial if we'd like to pursue automatic
> > > verification in the future.
> > 
> > What do you mean by automatic verification?
> 
> Formal verification. Theoretically we could have a formal specification of 
> our consistency model and we could prove/disprove whether a livepatch and 
> its implementation are correct with respect to it. It is a vague idea 
> though and I personally haven't got sufficient knowledge to do anything 
> about it.

For example, if the patched functions and the fixes meet its criteria, you
could use CMBC (http://www.cprover.org/cbmc/) to _prove_ that the live patch
changes exactly what you claim to, and nothing else.

	Torsten

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Fri, Oct 20, 2017 at 02:44:32PM +0200, Torsten Duwe wrote:
> On Thu, Oct 19, 2017 at 06:00:54PM +0200, Miroslav Benes wrote:
> > On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > > 
> > > Sounds nice, though I wonder what the obstacles are?
> > 
> > Those GCC optimizations you mentioned below and which I didn't connect to 
> > klp-convert itself.
> 
> I have a bad feeling about the IPA stuff in general. An obj-based approach
> is cool in a way that it still works, and is sure to work, if the IPA
> assumptions that led to the optimisations still hold, but as soon as they
> break, you're screwed big time.

Huh?  The obj-based approach (e.g., kpatch, bin-diff) inherently detects
such changes.  Or am I misunderstanding?  If so, please elaborate.

> For -fpatchable-function-entries I switched
> off IPA-RA, as especially on RISC there's _nothing_ you can do between
> functions without at least one scratch reg. But for live patching, I'd like
> the kernel to be compiled in the first place with 100% ABI adherence, IOW
> all IPA optimisations turned off. Does anyone have numbers on the performance
> impact?

I agree that would be the best option.

I don't think anyone has measured it because we don't know how to get
the compiler to do that :-)  My guess is that it will be something close
to negligible.

> > Nothing serious aside from that, I hope. Nicolai is currently implementing 
> > C parser for kernel sources.
> >  
> > > > You could verify the result and its correctness.
> > > 
> > > Does that mean it's easier to do code review?  Or something else?
> > 
> > Yes, the code review.
> > 
> > > > It could also be beneficial if we'd like to pursue automatic
> > > > verification in the future.
> > > 
> > > What do you mean by automatic verification?
> > 
> > Formal verification. Theoretically we could have a formal specification of 
> > our consistency model and we could prove/disprove whether a livepatch and 
> > its implementation are correct with respect to it. It is a vague idea 
> > though and I personally haven't got sufficient knowledge to do anything 
> > about it.
> 
> For example, if the patched functions and the fixes meet its criteria, you
> could use CMBC (http://www.cprover.org/cbmc/) to _prove_ that the live patch
> changes exactly what you claim to, and nothing else.

Can it also prove that the patch is applied in a safe manner?

-- 
Josh

Re: [PATCH 0/8] livepatch: klp-convert tool

[Miroslav Benes]

> > For -fpatchable-function-entries I switched
> > off IPA-RA, as especially on RISC there's _nothing_ you can do between
> > functions without at least one scratch reg. But for live patching, I'd like
> > the kernel to be compiled in the first place with 100% ABI adherence, IOW
> > all IPA optimisations turned off. Does anyone have numbers on the performance
> > impact?
> 
> I agree that would be the best option.
> 
> I don't think anyone has measured it because we don't know how to get
> the compiler to do that :-)  My guess is that it will be something close
> to negligible.

I asked Nicolai to do some performance testing with all the options that 
could be disabled disabled.

Miroslav

Re: [PATCH 0/8] livepatch: klp-convert tool

[Torsten Duwe]

On Fri, Oct 20, 2017 at 08:24:32AM -0500, Josh Poimboeuf wrote:
> On Fri, Oct 20, 2017 at 02:44:32PM +0200, Torsten Duwe wrote:
> > On Thu, Oct 19, 2017 at 06:00:54PM +0200, Miroslav Benes wrote:
> > > On Thu, 19 Oct 2017, Josh Poimboeuf wrote:
> > > > 
> > > > Sounds nice, though I wonder what the obstacles are?
> > > 
> > > Those GCC optimizations you mentioned below and which I didn't connect to 
> > > klp-convert itself.
> > 
> > I have a bad feeling about the IPA stuff in general. An obj-based approach
> > is cool in a way that it still works, and is sure to work, if the IPA
> > assumptions that led to the optimisations still hold, but as soon as they
> > break, you're screwed big time.
> 
> Huh?  The obj-based approach (e.g., kpatch, bin-diff) inherently detects
> such changes.  Or am I misunderstanding?  If so, please elaborate.

If e.g. the old callee does not use a caller-saved reg, neither does the new
code, there is no reason to actually save it. IPA will detect this and spare
the reg save/restore. If now by any coincidence the new function needs that
register, what can you do? Patch all callers as well? You'll likely end up
with a far-too-big live patch or by coding something in assembler :-(

> > For -fpatchable-function-entries I switched
> > off IPA-RA, as especially on RISC there's _nothing_ you can do between
> > functions without at least one scratch reg. But for live patching, I'd like
> > the kernel to be compiled in the first place with 100% ABI adherence, IOW
> > all IPA optimisations turned off. Does anyone have numbers on the performance
> > impact?
> 
> I agree that would be the best option.

I'm glad to hear this!-)

> > For example, if the patched functions and the fixes meet its criteria, you
> > could use CMBC (http://www.cprover.org/cbmc/) to _prove_ that the live patch
> > changes exactly what you claim to, and nothing else.
> 
> Can it also prove that the patch is applied in a safe manner?

I assume this strongly depends on the test cases you build around it.
OTOH the more code you include in the test, the more likely you'll violate
the cbmc preconditions. So I guess the simple answer is "no". But this might
change in the future.

	Torsten

Re: [PATCH 0/8] livepatch: klp-convert tool

[Josh Poimboeuf]

On Fri, Oct 20, 2017 at 03:44:12PM +0200, Torsten Duwe wrote:
> On Fri, Oct 20, 2017 at 08:24:32AM -0500, Josh Poimboeuf wrote:
> > On Fri, Oct 20, 2017 at 02:44:32PM +0200, Torsten Duwe wrote:
> > > I have a bad feeling about the IPA stuff in general. An obj-based approach
> > > is cool in a way that it still works, and is sure to work, if the IPA
> > > assumptions that led to the optimisations still hold, but as soon as they
> > > break, you're screwed big time.
> > 
> > Huh?  The obj-based approach (e.g., kpatch, bin-diff) inherently detects
> > such changes.  Or am I misunderstanding?  If so, please elaborate.
> 
> If e.g. the old callee does not use a caller-saved reg, neither does the new
> code, there is no reason to actually save it. IPA will detect this and spare
> the reg save/restore. If now by any coincidence the new function needs that
> register, what can you do? Patch all callers as well? You'll likely end up
> with a far-too-big live patch or by coding something in assembler :-(

Yes, you do have to patch the caller in that case.  But kpatch-build
detects that the caller changed as well, so both functions get patched
and it's not a problem.

And I don't remember ever seeing more than one caller for such a case.
At least it's never been a problem so far.

-- 
Josh