* [PATCH v7 01/10] livepatch: Create and include UAPI headers
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-07 11:41 ` Marcos Paulo de Souza
2023-03-06 14:08 ` [PATCH v7 02/10] livepatch: Add klp-convert tool Joe Lawrence
` (10 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
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 a newly
introduced klp-convert script.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
MAINTAINERS | 1 +
include/linux/livepatch.h | 1 +
include/uapi/linux/livepatch.h | 15 +++++++++++++++
kernel/livepatch/core.c | 4 ++--
4 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 include/uapi/linux/livepatch.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 135d93368d36..5bdf333fb1f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12069,6 +12069,7 @@ F: Documentation/ABI/testing/sysfs-kernel-livepatch
F: Documentation/livepatch/
F: arch/powerpc/include/asm/livepatch.h
F: include/linux/livepatch.h
+F: include/uapi/linux/livepatch.h
F: kernel/livepatch/
F: kernel/module/livepatch.c
F: lib/livepatch/
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 293e29960c6e..46da4c0df6c1 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -13,6 +13,7 @@
#include <linux/ftrace.h>
#include <linux/completion.h>
#include <linux/list.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..e19430918a07
--- /dev/null
+++ b/include/uapi/linux/livepatch.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ */
+
+#ifndef _UAPI_LIVEPATCH_H
+#define _UAPI_LIVEPATCH_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 201f0c0482fb..c565d33db582 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -234,7 +234,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab,
/* Format: .klp.sym.sym_objname.sym_name,sympos */
cnt = sscanf(strtab + sym->st_name,
- ".klp.sym.%55[^.].%511[^,],%lu",
+ KLP_SYM_PREFIX "%55[^.].%511[^,],%lu",
sym_objname, sym_name, &sympos);
if (cnt != 3) {
pr_err("symbol %s has an incorrectly formatted name\n",
@@ -305,7 +305,7 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
* See comment in klp_resolve_symbols() for an explanation
* of the selected field width value.
*/
- cnt = sscanf(shstrtab + sec->sh_name, ".klp.rela.%55[^.]",
+ cnt = sscanf(shstrtab + sec->sh_name, KLP_RELA_PREFIX "%55[^.]",
sec_objname);
if (cnt != 1) {
pr_err("section %s has an incorrectly formatted name\n",
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 01/10] livepatch: Create and include UAPI headers
2023-03-06 14:08 ` [PATCH v7 01/10] livepatch: Create and include UAPI headers Joe Lawrence
@ 2023-03-07 11:41 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-07 11:41 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:15AM -0500, Joe Lawrence wrote:
> 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 a newly
> introduced klp-convert script.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Joao Moreira <jmoreira@suse.de>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
LGTM:
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> MAINTAINERS | 1 +
> include/linux/livepatch.h | 1 +
> include/uapi/linux/livepatch.h | 15 +++++++++++++++
> kernel/livepatch/core.c | 4 ++--
> 4 files changed, 19 insertions(+), 2 deletions(-)
> create mode 100644 include/uapi/linux/livepatch.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 135d93368d36..5bdf333fb1f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12069,6 +12069,7 @@ F: Documentation/ABI/testing/sysfs-kernel-livepatch
> F: Documentation/livepatch/
> F: arch/powerpc/include/asm/livepatch.h
> F: include/linux/livepatch.h
> +F: include/uapi/linux/livepatch.h
> F: kernel/livepatch/
> F: kernel/module/livepatch.c
> F: lib/livepatch/
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 293e29960c6e..46da4c0df6c1 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -13,6 +13,7 @@
> #include <linux/ftrace.h>
> #include <linux/completion.h>
> #include <linux/list.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..e19430918a07
> --- /dev/null
> +++ b/include/uapi/linux/livepatch.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +
> +/*
> + * livepatch.h - Kernel Live Patching Core
> + *
> + * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
> + */
> +
> +#ifndef _UAPI_LIVEPATCH_H
> +#define _UAPI_LIVEPATCH_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 201f0c0482fb..c565d33db582 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -234,7 +234,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab,
>
> /* Format: .klp.sym.sym_objname.sym_name,sympos */
> cnt = sscanf(strtab + sym->st_name,
> - ".klp.sym.%55[^.].%511[^,],%lu",
> + KLP_SYM_PREFIX "%55[^.].%511[^,],%lu",
> sym_objname, sym_name, &sympos);
> if (cnt != 3) {
> pr_err("symbol %s has an incorrectly formatted name\n",
> @@ -305,7 +305,7 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
> * See comment in klp_resolve_symbols() for an explanation
> * of the selected field width value.
> */
> - cnt = sscanf(shstrtab + sec->sh_name, ".klp.rela.%55[^.]",
> + cnt = sscanf(shstrtab + sec->sh_name, KLP_RELA_PREFIX "%55[^.]",
> sec_objname);
> if (cnt != 1) {
> pr_err("section %s has an incorrectly formatted name\n",
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7 02/10] livepatch: Add klp-convert tool
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
2023-03-06 14:08 ` [PATCH v7 01/10] livepatch: Create and include UAPI headers Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-14 18:26 ` Marcos Paulo de Souza
2023-03-06 14:08 ` [PATCH v7 03/10] kbuild/modpost: create symbols.klp and integrate klp-convert Joe Lawrence
` (9 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
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.klp, 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.klp file, an input
livepatch module to be converted and the output name for the converted
livepatch. When it starts running, klp-convert parses symbols.klp 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.klp, 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
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>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
MAINTAINERS | 1 +
include/linux/livepatch.h | 12 +
include/uapi/linux/livepatch.h | 10 +
scripts/Makefile | 1 +
scripts/livepatch/.gitignore | 1 +
scripts/livepatch/Makefile | 5 +
scripts/livepatch/elf.c | 817 +++++++++++++++++++++++++++++
scripts/livepatch/elf.h | 74 +++
scripts/livepatch/klp-convert.c | 893 ++++++++++++++++++++++++++++++++
scripts/livepatch/klp-convert.h | 47 ++
scripts/livepatch/list.h | 391 ++++++++++++++
11 files changed, 2252 insertions(+)
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/klp-convert.h
create mode 100644 scripts/livepatch/list.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 5bdf333fb1f3..c68f4c1ac1ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12074,6 +12074,7 @@ F: kernel/livepatch/
F: kernel/module/livepatch.c
F: lib/livepatch/
F: samples/livepatch/
+F: scripts/livepatch/
F: tools/testing/selftests/livepatch/
LLC (802.2)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 46da4c0df6c1..cd5809010bca 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -235,6 +235,18 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
unsigned int symindex, unsigned int secindex,
const char *objname);
+/* Used to annotate symbol relocations in livepatches */
+#define KLP_MODULE_RELOC(obj) \
+ struct klp_module_reloc \
+ __section(".klp.module_relocs." #obj) \
+ __aligned(4)
+
+#define KLP_SYMPOS(symbol, pos) \
+ { \
+ .sym = &symbol, \
+ .sympos = pos, \
+ }
+
#else /* !CONFIG_LIVEPATCH */
static inline int klp_module_coming(struct module *mod) { return 0; }
diff --git a/include/uapi/linux/livepatch.h b/include/uapi/linux/livepatch.h
index e19430918a07..2ee98e985c2a 100644
--- a/include/uapi/linux/livepatch.h
+++ b/include/uapi/linux/livepatch.h
@@ -9,7 +9,17 @@
#ifndef _UAPI_LIVEPATCH_H
#define _UAPI_LIVEPATCH_H
+#include <linux/types.h>
+
#define KLP_RELA_PREFIX ".klp.rela."
#define KLP_SYM_PREFIX ".klp.sym."
+struct klp_module_reloc {
+ union {
+ void *sym;
+ __u64 sym64; /* Force 64-bit width */
+ };
+ __u32 sympos;
+} __packed;
+
#endif /* _UAPI_LIVEPATCH_H */
diff --git a/scripts/Makefile b/scripts/Makefile
index 1575af84d557..46a249ff6815 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -41,6 +41,7 @@ targets += module.lds
subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
subdir-$(CONFIG_MODVERSIONS) += genksyms
subdir-$(CONFIG_SECURITY_SELINUX) += selinux
+subdir-$(CONFIG_LIVEPATCH) += livepatch
# Let clean descend into subdirs
subdir- += basic dtc gdb kconfig mod
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..71dce0f3e893
--- /dev/null
+++ b/scripts/livepatch/Makefile
@@ -0,0 +1,5 @@
+hostprogs-always-y := klp-convert
+
+klp-convert-objs := klp-convert.o elf.o
+
+HOSTLDLIBS_klp-convert := -lelf
diff --git a/scripts/livepatch/elf.c b/scripts/livepatch/elf.c
new file mode 100644
index 000000000000..c649268a4a55
--- /dev/null
+++ b/scripts/livepatch/elf.c
@@ -0,0 +1,817 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 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>
+ */
+
+#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,
+ 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;
+ size_t i;
+
+ if (elf_getshdrnum(elf->elf, §ions_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 = calloc(1, sizeof(*sec));
+ if (!sec) {
+ perror("calloc");
+ return -1;
+ }
+
+ 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 = calloc(1, sizeof(*sym));
+ if (!sym) {
+ perror("calloc");
+ return -1;
+ }
+
+ 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 relas_nr, 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;
+
+ relas_nr = sec->sh.sh_size / sec->sh.sh_entsize;
+ for (i = 0; i < relas_nr; i++) {
+ rela = calloc(1, sizeof(*rela));
+ if (!rela) {
+ perror("calloc");
+ return -1;
+ }
+
+ 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 %u 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 = calloc(1, sizeof(*sec));
+ if (!sec) {
+ WARN("calloc failed");
+ return NULL;
+ }
+ INIT_LIST_HEAD(&sec->relas);
+
+ sec->base = base;
+ sec->name = strdup(name);
+ if (!sec->name) {
+ WARN("strdup failed");
+ return NULL;
+ }
+ sec->sh.sh_name = ~0;
+ sec->sh.sh_type = SHT_RELA;
+
+ if (elf->elf_class == ELFCLASS32) {
+ sec->sh.sh_entsize = sizeof(Elf32_Rela);
+ sec->sh.sh_addralign = 4;
+ } else {
+ sec->sh.sh_entsize = sizeof(Elf64_Rela);
+ sec->sh.sh_addralign = 8;
+ }
+ sec->sh.sh_flags = SHF_ALLOC;
+
+ sec->elf_data = calloc(1, sizeof(*sec->elf_data));
+ if (!sec->elf_data) {
+ WARN("calloc failed");
+ return NULL;
+ }
+ 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 != ~0U)
+ 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 != ~0U)
+ 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 1;
+}
+
+static void free_shstrtab(struct elf *elf)
+{
+ struct section *shstrtab;
+
+ shstrtab = find_section_by_name(elf, ".shstrtab");
+ if (!shstrtab)
+ return;
+
+ free(shstrtab->elf_data->d_buf);
+}
+
+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 != ~0U)
+ 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 != ~0U)
+ 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 1;
+}
+
+static void free_strtab(struct elf *elf)
+{
+ struct section *strtab;
+
+ strtab = find_section_by_name(elf, ".strtab");
+ if (!strtab)
+ return;
+
+ if (strtab->elf_data)
+ free(strtab->elf_data->d_buf);
+}
+
+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 */
+ if (elf->elf_class == ELFCLASS32)
+ size = nr_syms * sizeof(Elf32_Sym);
+ else
+ size = nr_syms * sizeof(Elf64_Sym);
+ buf = calloc(1, size);
+ if (!buf) {
+ WARN("calloc failed");
+ return -1;
+ }
+
+ offset = 0;
+ list_for_each_entry(sym, &elf->symbols, list) {
+
+ if (elf->elf_class == ELFCLASS32) {
+ /* Manually convert to 32-bit Elf32_Sym */
+ Elf32_Sym sym32;
+
+ sym32.st_name = sym->sym.st_name;
+ sym32.st_info = sym->sym.st_info;
+ sym32.st_other = sym->sym.st_other;
+ sym32.st_shndx = sym->sym.st_shndx;
+ sym32.st_value = sym->sym.st_value;
+ sym32.st_size = sym->sym.st_size;
+ memcpy(buf + offset, &sym32, sizeof(Elf32_Sym));
+ } else {
+ /* Existing 64-bit GElf_Syms are fine */
+ memcpy(buf + offset, &sym->sym, sizeof(Elf64_Sym));
+ }
+
+ 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 1;
+}
+
+static void free_symtab(struct elf *elf)
+{
+ struct section *symtab;
+
+ symtab = find_section_by_name(elf, ".symtab");
+ if (!symtab)
+ return;
+
+ free(symtab->elf_data->d_buf);
+}
+
+static int update_relas(struct elf *elf)
+{
+ struct section *sec, *symtab;
+ struct rela *rela;
+ int nr_relas, idx, size;
+ void *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++;
+
+ if (elf->elf_class == ELFCLASS32)
+ size = nr_relas * sizeof(Elf32_Rela);
+ else
+ size = nr_relas * sizeof(Elf64_Rela);
+
+ 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) {
+ if (elf->elf_class == ELFCLASS32) {
+ Elf32_Rela *relas32 = relas;
+
+ relas32[idx].r_offset = rela->offset;
+ relas32[idx].r_addend = rela->addend;
+ relas32[idx].r_info = ELF32_R_INFO(rela->sym->idx,
+ rela->type);
+ } else {
+ Elf64_Rela *relas64 = relas;
+
+ relas64[idx].r_offset = rela->offset;
+ relas64[idx].r_addend = rela->addend;
+ relas64[idx].r_info = ELF64_R_INFO(rela->sym->idx,
+ rela->type);
+ }
+ idx++;
+ }
+ }
+
+ return 1;
+}
+
+static void update_groups(struct elf *elf)
+{
+ struct section *sec, *symtab;
+
+ symtab = find_section_by_name(elf, ".symtab");
+
+ list_for_each_entry(sec, &elf->sections, list) {
+ if (sec->sh.sh_type == SHT_GROUP)
+ sec->sh.sh_link = symtab->idx;
+ }
+}
+
+static void free_relas(struct elf *elf)
+{
+ struct section *sec, *symtab;
+
+ symtab = find_section_by_name(elf, ".symtab");
+ if (!symtab)
+ return;
+
+ list_for_each_entry(sec, &elf->sections, list) {
+ if (!is_rela_section(sec))
+ continue;
+
+ free(sec->elf_data->d_buf);
+ }
+}
+
+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_flags = eh.e_flags;
+ 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;
+ }
+
+ elf_end(e);
+
+ return 0;
+}
+
+int elf_write_file(struct elf *elf, const char *file)
+{
+ int ret_shstrtab = 0;
+ int ret_strtab = 0;
+ int ret_symtab = 0;
+ int ret_relas = 0;
+ int ret;
+
+ ret_shstrtab = update_shstrtab(elf);
+ if (ret_shstrtab < 0) {
+ ret = ret_shstrtab;
+ goto out;
+ }
+
+ ret_strtab = update_strtab(elf);
+ if (ret_strtab < 0) {
+ ret = ret_strtab;
+ goto out;
+ }
+
+ ret_symtab = update_symtab(elf);
+ if (ret_symtab < 0) {
+ ret = ret_symtab;
+ goto out;
+ }
+
+ ret_relas = update_relas(elf);
+ if (ret_relas < 0) {
+ ret = ret_relas;
+ goto out;
+ }
+
+ update_groups(elf);
+
+ ret = write_file(elf, file);
+ if (ret)
+ return ret;
+
+out:
+ if (ret_relas > 0)
+ free_relas(elf);
+ if (ret_symtab > 0)
+ free_symtab(elf);
+ if (ret_strtab > 0)
+ free_strtab(elf);
+ if (ret_shstrtab > 0)
+ free_shstrtab(elf);
+
+ return ret;
+}
+
+struct elf *elf_open(const char *name)
+{
+ struct elf *elf;
+
+ elf_version(EV_CURRENT);
+
+ elf = calloc(1, sizeof(*elf));
+ if (!elf) {
+ perror("calloc");
+ return NULL;
+ }
+
+ 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;
+ }
+
+ elf->elf_class = gelf_getclass(elf->elf);
+ if ((elf->elf_class != ELFCLASS32) && (elf->elf_class != ELFCLASS64)) {
+ WARN("invalid elf class");
+ 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..a5969ae6a0be
--- /dev/null
+++ b/scripts/livepatch/elf.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2015-2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ */
+
+#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;
+ struct section *klp_rela_sec;
+ unsigned int type;
+ unsigned long offset;
+ int addend;
+};
+
+struct elf {
+ Elf *elf;
+ GElf_Ehdr ehdr;
+ int fd;
+ char *name;
+ int elf_class;
+ 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..f5d126b96922
--- /dev/null
+++ b/scripts/livepatch/klp-convert.c
@@ -0,0 +1,893 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ * Copyright (C) 2017 Joao Moreira <jmoreira@suse.de>
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include "elf.h"
+#include "list.h"
+#include "klp-convert.h"
+
+/*
+ * Symbols parsed from symbols.klp 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);
+
+/* Converted symbols and their struct symbol -> struct sympos association */
+static LIST_HEAD(converted_symbols);
+
+struct converted_sym {
+ struct list_head list;
+ struct symbol *symbol;
+ struct sympos sympos;
+};
+
+#define safe_snprintf(var, size, format, args...) \
+ ({ \
+ size_t __size = size; \
+ int __ret; \
+ \
+ __ret = snprintf(var, size, format, ##args); \
+ __ret < 0 || (size_t)__ret >= __size; \
+ })
+
+static 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 */
+static bool load_syms_lists(const char *symbols_list)
+{
+ FILE *fsyms;
+ size_t len = 0;
+ ssize_t n;
+ char *obj = NULL, *sym = NULL;
+ bool ret = false;
+
+ fsyms = fopen(symbols_list, "r");
+ if (!fsyms) {
+ WARN("Unable to open Symbol list: %s", symbols_list);
+ return false;
+ }
+
+ /* read file format version */
+ n = getline(&sym, &len, fsyms);
+ if (n <= 0) {
+ WARN("Unable to read Symbol list: %s", symbols_list);
+ goto done;
+ }
+
+ if (strncmp(sym, "klp-convert-symbol-data.0.1", 27) != 0) {
+ WARN("Symbol list is in unknown format.");
+ goto done;
+ }
+
+ len = 0;
+ free(sym);
+ sym = NULL;
+
+ /* read file */
+ n = getline(&sym, &len, fsyms);
+ while (n > 0) {
+ if (sym[n-1] == '\n')
+ sym[n-1] = '\0';
+
+ /* Objects in symbols.klp are flagged with '*' */
+ if (sym[0] == '*') {
+ free(obj);
+ obj = strdup(sym+1);
+ if (!obj) {
+ WARN("Unable to allocate object name\n");
+ goto done;
+ }
+ free(sym);
+ } else {
+ struct symbol_entry *entry;
+
+ if (!obj) {
+ WARN("File format error\n");
+ goto done;
+ }
+
+ entry = calloc(1, sizeof(struct symbol_entry));
+ if (!entry) {
+ WARN("Unable to allocate Symbol entry\n");
+ goto done;
+ }
+
+ entry->object_name = strdup(obj);
+ if (!entry->object_name) {
+ WARN("Unable to allocate entry object name\n");
+ free(entry);
+ goto done;
+ }
+
+ 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);
+ }
+ ret = true;
+
+done:
+ free(sym);
+ free(obj);
+ fclose(fsyms);
+ return ret;
+}
+
+/* Searches for sympos of specific symbol in usr_symbols list */
+static 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 */
+static void clear_sympos_symbols(struct section *annotation_sec,
+ struct elf *klp_elf)
+{
+ struct symbol *sym, *aux;
+
+ list_for_each_entry_safe(sym, aux, &klp_elf->symbols, list) {
+ if (sym->sec == annotation_sec) {
+
+ struct section *sec;
+ struct rela *rela, *tmprela;
+
+ list_for_each_entry(sec, &klp_elf->sections, list) {
+ list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+ if (rela->sym == sym) {
+ list_del(&rela->list);
+ free(rela);
+ }
+ }
+ }
+
+ list_del(&sym->list);
+ free(sym);
+ }
+ }
+}
+
+/* Removes annotation from livepatch elf object */
+static 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) {
+
+ struct rela *rela, *tmprela;
+
+ list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+ list_del(&rela->list);
+ free(rela);
+ }
+ list_del(&sec->list);
+ free(sec);
+ continue;
+ }
+ }
+}
+
+/*
+ * User provided sympos annotation checks:
+ * - do two or more elements in usr_symbols have the same object and
+ * name, but different symbol position
+ * - are there any usr_symbols without a rela?
+ */
+static bool sympos_sanity_check(struct elf *klp_elf)
+{
+ bool sane = true;
+ struct sympos *sp, *aux;
+ struct section *sec;
+ struct rela *rela;
+
+ list_for_each_entry(sp, &usr_symbols, list) {
+ bool found_rela = false;
+
+ aux = list_next_entry(sp, list);
+ list_for_each_entry_from(aux, &usr_symbols, list) {
+ if (sp->pos != aux->pos &&
+ strcmp(sp->object_name, aux->object_name) == 0 &&
+ strcmp(sp->symbol_name, aux->symbol_name) == 0) {
+ WARN("Conflicting KLP_SYMPOS definition: %s.%s %d vs. %d.",
+ sp->object_name, sp->symbol_name, sp->pos, aux->pos);
+ sane = false;
+ }
+ }
+
+ list_for_each_entry(sec, &klp_elf->sections, list) {
+ list_for_each_entry(rela, &sec->relas, list) {
+ if (!strcmp(sp->symbol_name, rela->sym->name)) {
+ found_rela = true;
+ break;
+ }
+ }
+ }
+ if (!found_rela) {
+ sane = false;
+ WARN("Couldn't find rela for annotated symbol: %s",
+ sp->symbol_name);
+ }
+ }
+
+ return sane;
+}
+
+/* Parses the livepatch elf object and fills usr_symbols */
+static bool load_usr_symbols(struct elf *klp_elf)
+{
+ char objname[MODULE_NAME_LEN];
+ struct sympos *sp;
+ struct section *sec, *relasec;
+ struct rela *rela;
+ Elf_Data converted_data;
+ struct klp_module_reloc *reloc;
+ int i, nr_entries;
+
+ list_for_each_entry(sec, &klp_elf->sections, list) {
+ if (sscanf(sec->name, ".klp.module_relocs.%55s", objname) != 1)
+ continue;
+
+ /*
+ * SYMPOS annotations are saved into arrays in
+ * .klp.module_relocs.* sections of type PROGBITS, so we
+ * need to manually translate the .sympos endianness in
+ * case we may be cross-compiling.
+ */
+ sec->elf_data->d_type = ELF_T_WORD;
+ converted_data.d_buf = sec->elf_data->d_buf;
+ converted_data.d_size = sec->elf_data->d_size;
+ converted_data.d_version = sec->elf_data->d_version;
+ gelf_xlatetom(klp_elf->elf, &converted_data, sec->elf_data,
+ elf_getident(klp_elf->elf, NULL)[EI_DATA]);
+
+ reloc = converted_data.d_buf;
+ relasec = sec->rela;
+
+ i = 0;
+ nr_entries = sec->size / sizeof(*reloc);
+ list_for_each_entry(rela, &relasec->relas, list) {
+ if (i >= nr_entries) {
+ WARN("section %s length beyond nr_entries\n",
+ relasec->name);
+ return false;
+ }
+ 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");
+ free(sp);
+ return false;
+ }
+ sp->symbol_name = strdup(rela->sym->name);
+ if (!sp->symbol_name) {
+ WARN("Unable to allocate symbol name\n");
+ free(sp);
+ return false;
+ }
+ 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(klp_elf);
+}
+
+/* prints list of valid sympos for symbol with provided name */
+static void print_valid_module_relocs(char *name)
+{
+ struct symbol_entry *e;
+ char *cur_obj = "";
+ int counter = 0;
+ bool first = true;
+
+ /* Symbols from the same object are locally gathered in the list */
+ 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, "Valid KLP_SYMPOS for symbol %s:\n", name);
+ fprintf(stderr, "-------------------------------------------------\n");
+ } else {
+ fprintf(stderr, "}\n");
+ }
+
+ fprintf(stderr, "KLP_MODULE_RELOC(%s){\n",
+ cur_obj);
+ first = false;
+ }
+ fprintf(stderr, "\tKLP_SYMPOS(%s,%d)\n", name, counter);
+ counter++;
+ }
+ }
+ if (!first) {
+ fprintf(stderr, "}\n");
+ 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
+ */
+static 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) {
+ WARN("Define KLP_SYMPOS for the symbol: %s",
+ e->symbol_name);
+
+ print_valid_module_relocs(name);
+ return NULL;
+ }
+ found = e;
+ }
+ }
+
+ return found;
+}
+
+/* Checks if sympos is valid, otherwise prints valid sympos list */
+static bool valid_sympos(struct sympos *sp)
+{
+ struct symbol_entry *e;
+
+ if (sp->pos == 0) {
+
+ /*
+ * sympos of 0 is reserved for uniquely named obj:sym,
+ * verify that this is the case
+ */
+ 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)) {
+ counter++;
+ }
+ }
+ if (counter == 1)
+ return true;
+
+ WARN("Provided KLP_SYMPOS of 0, but found %d symbols matching: %s.%s,%d",
+ counter, sp->object_name, sp->symbol_name,
+ sp->pos);
+
+ } else {
+
+ /*
+ * sympos > 0 indicates a specific commonly-named obj:sym,
+ * indexing starts with 1
+ */
+ int index = 1;
+
+ 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 (index == sp->pos)
+ return true;
+ index++;
+ }
+ }
+
+ WARN("Provided KLP_SYMPOS does not match a symbol: %s.%s,%d",
+ sp->object_name, sp->symbol_name, sp->pos);
+ }
+
+ print_valid_module_relocs(sp->symbol_name);
+
+ return false;
+}
+
+/*
+ * Add this symbol to the converted_symbols list to cache its sympos and
+ * for later renaming.
+ */
+static bool remember_sympos(struct symbol *s, struct sympos *sp)
+{
+ struct converted_sym *cs;
+
+ cs = calloc(1, sizeof(*cs));
+ if (!cs) {
+ WARN("Unable to allocate converted_symbol entry");
+ return false;
+ }
+
+ cs->symbol = s;
+ cs->sympos = *sp;
+ list_add(&cs->list, &converted_symbols);
+
+ return true;
+}
+
+/* Returns the right sympos respective to a symbol to be relocated */
+static bool find_sympos(struct symbol *s, struct sympos *sp)
+{
+ struct symbol_entry *entry;
+ struct converted_sym *cs;
+
+ /* did we already convert this symbol? */
+ list_for_each_entry(cs, &converted_symbols, list) {
+ if (cs->symbol == s) {
+ *sp = cs->sympos;
+ return true;
+ }
+ }
+
+ /* did the user specified via annotation? */
+ if (get_usr_sympos(s, sp)) {
+ if (valid_sympos(sp)) {
+ remember_sympos(s, sp);
+ return true;
+ }
+ return false;
+ }
+
+ /*
+ * user didn't specify, so search the symbols list. A pos of 0
+ * indicates this should be a unique symbol.
+ */
+ 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;
+ remember_sympos(s, sp);
+ return true;
+ }
+ return false;
+}
+
+/*
+ * Finds or creates a klp rela section based on another given section (@oldsec)
+ * and sympos (@*sp), then returns it
+ */
+static 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 (safe_snprintf(name, length, KLP_RELA_PREFIX "%s.%s",
+ sp->object_name, oldsec->base->name)) {
+ WARN("Length error (%s)", name);
+ free(name);
+ return NULL;
+ }
+
+ 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;
+}
+
+/* Converts rela symbol names */
+static bool convert_symbol(struct symbol *s, struct sympos *sp)
+{
+ char *name;
+ char pos[4]; /* assume that pos will never be > 999 */
+ unsigned int length;
+
+ if (safe_snprintf(pos, sizeof(pos), "%d", sp->pos)) {
+ WARN("Insufficient 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) + sizeof(pos) + 3;
+
+ 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 (safe_snprintf(name, length, KLP_SYM_PREFIX "%s.%s,%s",
+ sp->object_name, sp->symbol_name, pos)) {
+
+ WARN("Length error (%s%s.%s,%s)", KLP_SYM_PREFIX,
+ sp->object_name, sp->symbol_name, pos);
+ free(name);
+ return false;
+ }
+
+ /*
+ * Despite the memory waste, we don't mind freeing the original symbol
+ * name memory chunk. Keeping it there is harmless and, since removing
+ * bytes from the string section is non-trivial, it is unworthy.
+ */
+ s->name = name;
+ s->sec = NULL;
+ s->sym.st_name = -1;
+ s->sym.st_shndx = SHN_LIVEPATCH;
+
+ return true;
+}
+
+/* Checks if a rela was converted */
+static bool is_converted_rela(struct rela *rela)
+{
+ return !!rela->klp_rela_sec;
+}
+
+/*
+ * Convert rela that cannot be resolved by the classic module loader
+ * to the special klp rela one.
+ */
+static bool convert_rela(struct section *oldsec, struct rela *rela,
+ struct sympos *sp, struct elf *klp_elf)
+{
+ struct section *sec;
+
+ 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;
+ }
+
+ rela->klp_rela_sec = sec;
+
+ return true;
+}
+
+static void move_rela(struct rela *r)
+{
+ /* Move the converted rela to klp rela section */
+ list_del(&r->list);
+ list_add_tail(&r->list, &r->klp_rela_sec->relas);
+}
+
+/* Checks if given symbol name matches a symbol in exp_symbols */
+static 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 symbol should be skipped */
+static bool skip_symbol(struct symbol *sym)
+{
+ /* already resolved? */
+ if (sym->sec)
+ return true;
+
+ /* skip symbol with index 0 */
+ if (!sym->idx)
+ return true;
+
+ /* we should not touch .TOC. on ppc64le */
+ if (strcmp(sym->name, ".TOC.") == 0)
+ return true;
+
+ if (is_exported(sym->name))
+ return true;
+
+ return false;
+}
+
+/* Checks if a section is a klp rela section */
+static bool is_klp_rela_section(char *sname)
+{
+ int len = strlen(KLP_RELA_PREFIX);
+
+ if (strncmp(sname, KLP_RELA_PREFIX, len) == 0)
+ return true;
+ return false;
+}
+
+/*
+ * Frees the list, new names and rela sections as created by
+ * remember_sympos(), convert_rela(), and convert_symbol()
+ */
+static void free_converted_resources(struct elf *klp_elf)
+{
+ struct converted_sym *cs, *cs_aux;
+ struct section *sec;
+
+ list_for_each_entry_safe(cs, cs_aux, &converted_symbols, list) {
+ free(cs->symbol->name);
+ free(cs);
+ }
+
+ list_for_each_entry(sec, &klp_elf->sections, list) {
+ if (is_klp_rela_section(sec->name)) {
+ free(sec->elf_data);
+ free(sec->name);
+ }
+ }
+}
+
+/*
+ * Checks if section may be skipped (conditions)
+ */
+static bool skip_section(struct section *sec)
+{
+ if (!is_rela_section(sec))
+ return true;
+
+ if (is_klp_rela_section(sec->name))
+ return true;
+
+ return false;
+}
+
+/*
+ * Checks if rela conversion is supported in given section
+ */
+static bool supported_section(struct section *sec, char *object_name)
+{
+#if 0
+ /*
+ * klp-relocations forbidden in sections that otherwise would
+ * match in allowed_prefixes[]
+ */
+ static const char * const not_allowed[] = {
+ ".rela.data.rel.ro",
+ ".rela.data.rel.ro.local",
+ ".rela.data..ro_after_init",
+ NULL
+ };
+#endif
+
+ /* klp-relocations allowed in sections only for vmlinux */
+ static const char * const allowed_vmlinux[] = {
+ ".rela__jump_table",
+ NULL
+ };
+
+ /* klp-relocations allowed in sections with prefixes */
+ static const char * const allowed_prefixes[] = {
+ ".rela.data",
+ ".rela.rodata", // supported ???
+ ".rela.sdata",
+ ".rela.text",
+ ".rela.toc",
+ NULL
+ };
+
+ const char * const *name;
+
+#if 0
+ for (name = not_allowed; *name; name++)
+ if (strcmp(sec->name, *name) == 0)
+ return false;
+#endif
+
+ if (strcmp(object_name, "vmlinux") == 0) {
+ for (name = allowed_vmlinux; *name; name++)
+ if (strcmp(sec->name, *name) == 0)
+ return true;
+ }
+
+ for (name = allowed_prefixes; *name; name++)
+ if (strncmp(sec->name, *name, strlen(*name)) == 0)
+ return true;
+
+ return false;
+}
+
+int main(int argc, const char **argv)
+{
+ const char *klp_in_module, *klp_out_module, *symbols_list;
+ struct rela *rela, *tmprela;
+ struct section *sec;
+ struct sympos sp;
+ struct elf *klp_elf;
+ struct converted_sym *cs;
+ int errors = 0;
+
+ if (argc != 4) {
+ WARN("Usage: %s <symbols.klp> <input.ko> <output.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(sec, &klp_elf->sections, list) {
+ if (skip_section(sec))
+ continue;
+
+ list_for_each_entry(rela, &sec->relas, list) {
+ if (skip_symbol(rela->sym))
+ continue;
+
+ /* rela needs to be converted */
+
+ if (!find_sympos(rela->sym, &sp)) {
+ WARN("Unable to find missing symbol: %s",
+ rela->sym->name);
+ errors++;
+ continue;
+ }
+ if (!supported_section(sec, sp.object_name)) {
+ WARN("Conversion not supported for symbol: %s section: %s object: %s",
+ rela->sym->name, sec->name,
+ sp.object_name);
+ errors++;
+ continue;
+ }
+ if (!convert_rela(sec, rela, &sp, klp_elf)) {
+ WARN("Unable to convert relocation: %s",
+ rela->sym->name);
+ return -1;
+ }
+ }
+
+ if (errors)
+ return -1;
+
+ /* Now move all converted relas in list-safe manner */
+ list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+ if (is_converted_rela(rela))
+ move_rela(rela);
+ }
+ }
+
+ /* Rename the converted symbols */
+ list_for_each_entry(cs, &converted_symbols, list) {
+ if (!convert_symbol(cs->symbol, &cs->sympos)) {
+ WARN("Unable to convert symbol name (%s)\n",
+ cs->symbol->name);
+ return -1;
+ }
+ }
+
+ free_syms_lists();
+ if (elf_write_file(klp_elf, klp_out_module))
+ return -1;
+
+ free_converted_resources(klp_elf);
+ elf_close(klp_elf);
+
+ return 0;
+}
+
+/* Functions kept commented since they might be useful for future debugging */
+
+/* Dumps sympos list (useful for debugging purposes)
+ * static 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");
+ * }
+ *
+ *
+ * / Dump symbols list for debugging purposes /
+ * static 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");
+ * }
+ */
diff --git a/scripts/livepatch/klp-convert.h b/scripts/livepatch/klp-convert.h
new file mode 100644
index 000000000000..35b9dc4e32c4
--- /dev/null
+++ b/scripts/livepatch/klp-convert.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2016 Josh Poimboeuf <jpoimboe@redhat.com>
+ * Copyright (C) 2017 Joao Moreira <jmoreira@suse.de>
+ *
+ */
+
+#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;
+};
+
+/*
+ * klp-convert uses macros and structures defined in the linux sources
+ * package (see include/uapi/linux/livepatch.h). To prevent the
+ * dependency when building locally, they are defined below. Also notice
+ * that these should match the definitions from the targeted kernel.
+ */
+
+#define KLP_RELA_PREFIX ".klp.rela."
+#define KLP_SYM_PREFIX ".klp.sym."
+
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+struct klp_module_reloc {
+ union {
+ void *sym;
+ uint64_t sym64; /* Force 64-bit width */
+ };
+ uint32_t sympos;
+} __packed;
diff --git a/scripts/livepatch/list.h b/scripts/livepatch/list.h
new file mode 100644
index 000000000000..4d429120fabf
--- /dev/null
+++ b/scripts/livepatch/list.h
@@ -0,0 +1,391 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#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.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 02/10] livepatch: Add klp-convert tool
2023-03-06 14:08 ` [PATCH v7 02/10] livepatch: Add klp-convert tool Joe Lawrence
@ 2023-03-14 18:26 ` Marcos Paulo de Souza
2023-03-17 20:06 ` Joe Lawrence
0 siblings, 1 reply; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-14 18:26 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:16AM -0500, Joe Lawrence 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 symbols.klp, 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.klp file, an input
> livepatch module to be converted and the output name for the converted
> livepatch. When it starts running, klp-convert parses symbols.klp 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.klp, 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
LGTM:
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
I only have two remarks:
>
> 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>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
...
> +#if 0
> + /*
> + * klp-relocations forbidden in sections that otherwise would
> + * match in allowed_prefixes[]
> + */
> + static const char * const not_allowed[] = {
> + ".rela.data.rel.ro",
> + ".rela.data.rel.ro.local",
> + ".rela.data..ro_after_init",
> + NULL
> + };
> +#endif
> +
> + /* klp-relocations allowed in sections only for vmlinux */
> + static const char * const allowed_vmlinux[] = {
> + ".rela__jump_table",
> + NULL
> + };
> +
> + /* klp-relocations allowed in sections with prefixes */
> + static const char * const allowed_prefixes[] = {
> + ".rela.data",
> + ".rela.rodata", // supported ???
> + ".rela.sdata",
> + ".rela.text",
> + ".rela.toc",
> + NULL
> + };
> +
> + const char * const *name;
> +
> +#if 0
> + for (name = not_allowed; *name; name++)
> + if (strcmp(sec->name, *name) == 0)
> + return false;
> +#endif
> +
Have you needed to enable the not_allowed checks when creating your livepatches?
Otherwise I believe that this can be removed and added again in the future is
needed.
> +int main(int argc, const char **argv)
> +{
> + const char *klp_in_module, *klp_out_module, *symbols_list;
...
> +
> +/* Functions kept commented since they might be useful for future debugging */
> +
> +/* Dumps sympos list (useful for debugging purposes)
> + * static 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");
> + * }
> + *
> + *
> + * / Dump symbols list for debugging purposes /
> + * static 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");
> + * }
Same here. Have you used these functions recently when debugging klp-convert?
Othewise it can be removed as well.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v7 02/10] livepatch: Add klp-convert tool
2023-03-14 18:26 ` Marcos Paulo de Souza
@ 2023-03-17 20:06 ` Joe Lawrence
2023-03-20 19:53 ` Marcos Paulo de Souza
0 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-17 20:06 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On 3/14/23 14:26, Marcos Paulo de Souza wrote:
> On Mon, Mar 06, 2023 at 09:08:16AM -0500, Joe Lawrence 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 symbols.klp, 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.klp file, an input
>> livepatch module to be converted and the output name for the converted
>> livepatch. When it starts running, klp-convert parses symbols.klp 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.klp, 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
>
> LGTM:
>
> Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>
> I only have two remarks:
>
>>
>> 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>
>> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
>
> ...
>
>
>> +#if 0
>> + /*
>> + * klp-relocations forbidden in sections that otherwise would
>> + * match in allowed_prefixes[]
>> + */
>> + static const char * const not_allowed[] = {
>> + ".rela.data.rel.ro",
>> + ".rela.data.rel.ro.local",
>> + ".rela.data..ro_after_init",
>> + NULL
>> + };
>> +#endif
>> +
>> + /* klp-relocations allowed in sections only for vmlinux */
>> + static const char * const allowed_vmlinux[] = {
>> + ".rela__jump_table",
>> + NULL
>> + };
>> +
>> + /* klp-relocations allowed in sections with prefixes */
>> + static const char * const allowed_prefixes[] = {
>> + ".rela.data",
>> + ".rela.rodata", // supported ???
>> + ".rela.sdata",
>> + ".rela.text",
>> + ".rela.toc",
>> + NULL
>> + };
>> +
>> + const char * const *name;
>> +
>> +#if 0
>> + for (name = not_allowed; *name; name++)
>> + if (strcmp(sec->name, *name) == 0)
>> + return false;
>> +#endif
>> +
>
> Have you needed to enable the not_allowed checks when creating your livepatches?
> Otherwise I believe that this can be removed and added again in the future is
> needed.
>
Good question.
I left the disabled blocks in the code as a bookmark for an outstanding
question: should klp-convert avoid converting relocations in any
read-only-ish section?
This becomes interesting in the late module loading case -- some arches
(ppc64le IIRC) do not like the module loader tweaking these relocations
post module load. [1]
In "[PATCH v7 09/10] livepatch/selftests: add data relocations test"
test_klp_convert_data.c, you'll see a few "// .rela.data.rel.ro,
.rela.rodata supported ??" comments. Those would generate relocations
in such sections.
Do they *need* to be supported? AFAIK kpatch-build hasn't needed to
create any of those. That said, it's not too difficult for this
patchset's self-tests to generate these. klp-convert could easily
detect this scenario. The livepatch author could be advised to remove
const or __ro_after_init annotation to move the relocation out of the
read-only-ish section.
[1] https://github.com/joe-lawrence/klp-convert-tree/issues/5
>> +int main(int argc, const char **argv)
>> +{
>> + const char *klp_in_module, *klp_out_module, *symbols_list;
>
> ...
>
>> +
>> +/* Functions kept commented since they might be useful for future debugging */
>> +
>> +/* Dumps sympos list (useful for debugging purposes)
>> + * static 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");
>> + * }
>> + *
>> + *
>> + * / Dump symbols list for debugging purposes /
>> + * static 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");
>> + * }
>
> Same here. Have you used these functions recently when debugging klp-convert?
> Othewise it can be removed as well.
>
I was tinkering with an alternate sympos annotation (I'll describe it in
a separate reply) and think that these debug routines could be activated
with --debug cmdline flag(s). They can be handy for debug/development,
so better to make them always active rather than #if 0'd out.
--
Joe
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v7 02/10] livepatch: Add klp-convert tool
2023-03-17 20:06 ` Joe Lawrence
@ 2023-03-20 19:53 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-20 19:53 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Fri, Mar 17, 2023 at 04:06:15PM -0400, Joe Lawrence wrote:
> On 3/14/23 14:26, Marcos Paulo de Souza wrote:
> > On Mon, Mar 06, 2023 at 09:08:16AM -0500, Joe Lawrence 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 symbols.klp, 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.klp file, an input
> >> livepatch module to be converted and the output name for the converted
> >> livepatch. When it starts running, klp-convert parses symbols.klp 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.klp, 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
> >
> > LGTM:
> >
> > Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> >
> > I only have two remarks:
> >
> >>
> >> 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>
> >> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> >
> > ...
> >
> >
> >> +#if 0
> >> + /*
> >> + * klp-relocations forbidden in sections that otherwise would
> >> + * match in allowed_prefixes[]
> >> + */
> >> + static const char * const not_allowed[] = {
> >> + ".rela.data.rel.ro",
> >> + ".rela.data.rel.ro.local",
> >> + ".rela.data..ro_after_init",
> >> + NULL
> >> + };
> >> +#endif
> >> +
> >> + /* klp-relocations allowed in sections only for vmlinux */
> >> + static const char * const allowed_vmlinux[] = {
> >> + ".rela__jump_table",
> >> + NULL
> >> + };
> >> +
> >> + /* klp-relocations allowed in sections with prefixes */
> >> + static const char * const allowed_prefixes[] = {
> >> + ".rela.data",
> >> + ".rela.rodata", // supported ???
> >> + ".rela.sdata",
> >> + ".rela.text",
> >> + ".rela.toc",
> >> + NULL
> >> + };
> >> +
> >> + const char * const *name;
> >> +
> >> +#if 0
> >> + for (name = not_allowed; *name; name++)
> >> + if (strcmp(sec->name, *name) == 0)
> >> + return false;
> >> +#endif
> >> +
> >
> > Have you needed to enable the not_allowed checks when creating your livepatches?
> > Otherwise I believe that this can be removed and added again in the future is
> > needed.
> >
>
> Good question.
>
> I left the disabled blocks in the code as a bookmark for an outstanding
> question: should klp-convert avoid converting relocations in any
> read-only-ish section?
>
> This becomes interesting in the late module loading case -- some arches
> (ppc64le IIRC) do not like the module loader tweaking these relocations
> post module load. [1]
>
> In "[PATCH v7 09/10] livepatch/selftests: add data relocations test"
> test_klp_convert_data.c, you'll see a few "// .rela.data.rel.ro,
> .rela.rodata supported ??" comments. Those would generate relocations
> in such sections.
>
> Do they *need* to be supported? AFAIK kpatch-build hasn't needed to
> create any of those. That said, it's not too difficult for this
> patchset's self-tests to generate these. klp-convert could easily
> detect this scenario. The livepatch author could be advised to remove
> const or __ro_after_init annotation to move the relocation out of the
> read-only-ish section.
I'm not sure if it needs to be supported as well. Maybe other people could give
their inputs in this regard? Suggesting the livepatch author about this case
seems a good option either way.
>
> [1] https://github.com/joe-lawrence/klp-convert-tree/issues/5
>
> >> +int main(int argc, const char **argv)
> >> +{
> >> + const char *klp_in_module, *klp_out_module, *symbols_list;
> >
> > ...
> >
> >> +
> >> +/* Functions kept commented since they might be useful for future debugging */
> >> +
> >> +/* Dumps sympos list (useful for debugging purposes)
> >> + * static 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");
> >> + * }
> >> + *
> >> + *
> >> + * / Dump symbols list for debugging purposes /
> >> + * static 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");
> >> + * }
> >
> > Same here. Have you used these functions recently when debugging klp-convert?
> > Othewise it can be removed as well.
> >
>
> I was tinkering with an alternate sympos annotation (I'll describe it in
> a separate reply) and think that these debug routines could be activated
> with --debug cmdline flag(s). They can be handy for debug/development,
> so better to make them always active rather than #if 0'd out.
Seems a good plan.
>
> --
> Joe
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7 03/10] kbuild/modpost: create symbols.klp and integrate klp-convert
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
2023-03-06 14:08 ` [PATCH v7 01/10] livepatch: Create and include UAPI headers Joe Lawrence
2023-03-06 14:08 ` [PATCH v7 02/10] livepatch: Add klp-convert tool Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-14 18:48 ` Marcos Paulo de Souza
2023-03-06 14:08 ` [PATCH v7 04/10] livepatch: Add sample livepatch module Joe Lawrence
` (8 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
For automatic resolution of livepatch relocations, a file called
symbols.klp 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.klp in the main Makefile. First, ensure
that built-in is compiled when CONFIG_LIVEPATCH is enabled (as required
to achieve a complete symbols.klp file). Define the command to build
symbols.klp (filechk_klp_map) and hook it in the modules rule. Save the
list of livepatch modules in $(MODULES_LIVEPATCH).
As it is undesirable to have symbols from livepatch objects inside
symbols.klp, filechk_klp_map filters out modules.livepatch from
modules.order: `sort $(MODORDER) $(MODULES_LIVEPATCH) | uniq -u`.
The "clean" Makefile target may remove the modules.livepatch file,
however, symbols.klp may be needed for building external modules, so
defer its cleanup to the "mrproper" target.
Finally, update the modpost program so that it does not warn about
unresolved symbols resolved by klp-convert.
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>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
.gitignore | 2 ++
Documentation/dontdiff | 1 +
Makefile | 16 +++++++++++-----
scripts/Makefile.modfinal | 33 +++++++++++++++++++++++++++++++++
scripts/Makefile.modpost | 5 +++++
scripts/mod/modpost.c | 28 ++++++++++++++++++++++++++--
scripts/mod/modpost.h | 1 +
7 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/.gitignore b/.gitignore
index 20dce5c3b9e0..fc9b2f13b049 100644
--- a/.gitignore
+++ b/.gitignore
@@ -53,6 +53,7 @@
*.xz
*.zst
Module.symvers
+modules.livepatch
modules.order
#
@@ -66,6 +67,7 @@ modules.order
/vmlinux.symvers
/vmlinux-gdb.py
/vmlinuz
+/symbols.klp
/System.map
/Module.markers
/modules.builtin
diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index 352ff53a2306..23c2a89fb791 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -76,6 +76,7 @@ Module.markers
Module.symvers
PENDING
SCCS
+symbols.klp
System.map*
TAGS
aconf
diff --git a/Makefile b/Makefile
index 3f6628780eb2..dd5d6c258906 100644
--- a/Makefile
+++ b/Makefile
@@ -730,8 +730,13 @@ KBUILD_MODULES :=
KBUILD_BUILTIN := 1
# If we have only "make modules", don't compile built-in objects.
+# When we're building livepatch modules, we need to consider the
+# built-in objects during the descend as well, 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 :=
+ KBUILD_BUILTIN := $(if $(CONFIG_LIVEPATCH),1)
endif
# If we have "make <whatever> modules", compile modules
@@ -1183,6 +1188,7 @@ PHONY += prepare0
export extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
export MODORDER := $(extmod_prefix)modules.order
export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps
+export MODULES_LIVEPATCH := $(extmod-prefix)modules.livepatch
ifeq ($(KBUILD_EXTMOD),)
@@ -1543,8 +1549,8 @@ endif
#
# *.ko are usually independent of vmlinux, but CONFIG_DEBUG_INFOBTF_MODULES
-# is an exception.
-ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+# and CONFIG_LIVEPATCH are exceptions.
+ifneq ($(or $(CONFIG_DEBUG_INFO_BTF_MODULES),$(CONFIG_LIVEPATCH)),)
KBUILD_BUILTIN := 1
modules: vmlinux
endif
@@ -1602,14 +1608,14 @@ endif # CONFIG_MODULES
CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
modules.builtin modules.builtin.modinfo modules.nsdeps \
compile_commands.json .thinlto-cache rust/test rust/doc \
- .vmlinux.objs .vmlinux.export.c
+ modules.livepatch .vmlinux.objs .vmlinux.export.c
# Directories & files removed with 'make mrproper'
MRPROPER_FILES += include/config include/generated \
arch/$(SRCARCH)/include/generated .objdiff \
debian snap tar-install \
.config .config.old .version \
- Module.symvers \
+ Module.symvers symbols.klp \
certs/signing_key.pem \
certs/x509.genkey \
vmlinux-gdb.py \
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index a30d5b08eee9..a8901e4e98c5 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -14,6 +14,7 @@ include $(srctree)/scripts/Makefile.lib
# find all modules listed in modules.order
modules := $(call read-file, $(MODORDER))
+modules-klp := $(call read-file, $(MODULES_LIVEPATCH))
__modfinal: $(modules:%.o=%.ko)
@:
@@ -65,6 +66,38 @@ endif
targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)
+# Livepatch
+# ---------------------------------------------------------------------------
+
+%.tmp.ko: %.o %.mod.o symbols.klp FORCE
+ +$(call if_changed,ld_ko_o)
+
+quiet_cmd_klp_convert = KLP $@
+ cmd_klp_convert = scripts/livepatch/klp-convert symbols.klp $< $@
+
+$(modules-klp:%.o=%.ko): %.ko: %.tmp.ko FORCE
+ $(call if_changed,klp_convert)
+
+targets += $(modules-klp:.ko=.tmp.ko)
+
+ifeq ($(KBUILD_EXTMOD),)
+filechk_klp_map = \
+ echo "klp-convert-symbol-data.0.1"; \
+ echo "*vmlinux"; \
+ $(NM) -f posix vmlinux | cut -d\ -f1; \
+ sort $(MODORDER) $(MODULES_LIVEPATCH) | \
+ uniq -u | \
+ sed 's/\.o$$//' | \
+ while read o; \
+ do \
+ echo "*$$(basename $$o)"; \
+ $(NM) -f posix $$o.o | cut -d\ -f1; \
+ done
+
+symbols.klp: FORCE
+ $(call filechk,klp_map)
+endif
+
# Add FORCE to the prequisites of a target to force it to be always rebuilt.
# ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 43343e13c542..02f1354d4cff 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -47,6 +47,7 @@ modpost-args = \
$(if $(KBUILD_MODPOST_WARN),-w) \
$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS)) \
$(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N) \
+ $(if $(CONFIG_LIVEPATCH),-l $(MODULES_LIVEPATCH)) \
-o $@
modpost-deps := $(MODPOST)
@@ -138,6 +139,10 @@ $(output-symdump): $(modpost-deps) FORCE
$(call if_changed,modpost)
__modpost: $(output-symdump)
+ifndef CONFIG_LIVEPATCH
+ $(Q)rm -f $(MODULES_LIVEPATCH)
+ $(Q)touch $(MODULES_LIVEPATCH)
+endif
PHONY += FORCE
FORCE:
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index efff8078e395..0a8f0ce75761 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1831,6 +1831,10 @@ static void read_symbols(const char *modname)
handle_moddevtable(mod, &info, sym, symname);
}
+ /* Livepatch modules have unresolved symbols resolved by klp-convert */
+ if (get_modinfo(&info, "livepatch"))
+ mod->is_livepatch = true;
+
for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
symname = remove_dot(info.strtab + sym->st_name);
@@ -1919,7 +1923,7 @@ static void check_exports(struct module *mod)
const char *basename;
exp = find_symbol(s->name);
if (!exp) {
- if (!s->weak && nr_unresolved++ < MAX_UNRESOLVED_REPORTS)
+ if (!s->weak && !mod->is_livepatch && nr_unresolved++ < MAX_UNRESOLVED_REPORTS)
modpost_log(warn_unresolved ? LOG_WARN : LOG_ERROR,
"\"%s\" [%s.ko] undefined!\n",
s->name, mod->name);
@@ -2320,6 +2324,20 @@ static void write_namespace_deps_files(const char *fname)
free(ns_deps_buf.p);
}
+static void write_livepatch_modules(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+
+ list_for_each_entry(mod, &modules, list) {
+ if (mod->is_livepatch)
+ buf_printf(&buf, "%s.o\n", mod->name);
+ }
+
+ write_if_changed(&buf, fname);
+ free(buf.p);
+}
+
struct dump_list {
struct list_head list;
const char *file;
@@ -2330,11 +2348,12 @@ int main(int argc, char **argv)
struct module *mod;
char *missing_namespace_deps = NULL;
char *dump_write = NULL, *files_source = NULL;
+ char *livepatch_modules = NULL;
int opt;
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;
- while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+ while ((opt = getopt(argc, argv, "ei:l:mnT:o:awENd:")) != -1) {
switch (opt) {
case 'e':
external_module = true;
@@ -2344,6 +2363,9 @@ int main(int argc, char **argv)
dl->file = optarg;
list_add_tail(&dl->list, &dump_lists);
break;
+ case 'l':
+ livepatch_modules = optarg;
+ break;
case 'm':
modversions = true;
break;
@@ -2403,6 +2425,8 @@ int main(int argc, char **argv)
if (dump_write)
write_dump(dump_write);
+ if (livepatch_modules)
+ write_livepatch_modules(livepatch_modules);
if (sec_mismatch_count && !sec_mismatch_warn_only)
error("Section mismatches detected.\n"
"Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1178f40a73f3..9be9bf6fb7da 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -119,6 +119,7 @@ struct module {
bool is_gpl_compatible;
bool from_dump; /* true if module was loaded from *.symvers */
bool is_vmlinux;
+ bool is_livepatch;
bool seen;
bool has_init;
bool has_cleanup;
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 03/10] kbuild/modpost: create symbols.klp and integrate klp-convert
2023-03-06 14:08 ` [PATCH v7 03/10] kbuild/modpost: create symbols.klp and integrate klp-convert Joe Lawrence
@ 2023-03-14 18:48 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-14 18:48 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:17AM -0500, Joe Lawrence wrote:
> For automatic resolution of livepatch relocations, a file called
> symbols.klp 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.klp in the main Makefile. First, ensure
> that built-in is compiled when CONFIG_LIVEPATCH is enabled (as required
> to achieve a complete symbols.klp file). Define the command to build
> symbols.klp (filechk_klp_map) and hook it in the modules rule. Save the
> list of livepatch modules in $(MODULES_LIVEPATCH).
>
> As it is undesirable to have symbols from livepatch objects inside
> symbols.klp, filechk_klp_map filters out modules.livepatch from
> modules.order: `sort $(MODORDER) $(MODULES_LIVEPATCH) | uniq -u`.
>
> The "clean" Makefile target may remove the modules.livepatch file,
> however, symbols.klp may be needed for building external modules, so
> defer its cleanup to the "mrproper" target.
>
> Finally, update the modpost program so that it does not warn about
> unresolved symbols resolved by klp-convert.
>
I don't have strong skills on Kbuild, but the changes makes sense.
Acked-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> 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>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> .gitignore | 2 ++
> Documentation/dontdiff | 1 +
> Makefile | 16 +++++++++++-----
> scripts/Makefile.modfinal | 33 +++++++++++++++++++++++++++++++++
> scripts/Makefile.modpost | 5 +++++
> scripts/mod/modpost.c | 28 ++++++++++++++++++++++++++--
> scripts/mod/modpost.h | 1 +
> 7 files changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 20dce5c3b9e0..fc9b2f13b049 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -53,6 +53,7 @@
> *.xz
> *.zst
> Module.symvers
> +modules.livepatch
> modules.order
>
> #
> @@ -66,6 +67,7 @@ modules.order
> /vmlinux.symvers
> /vmlinux-gdb.py
> /vmlinuz
> +/symbols.klp
> /System.map
> /Module.markers
> /modules.builtin
> diff --git a/Documentation/dontdiff b/Documentation/dontdiff
> index 352ff53a2306..23c2a89fb791 100644
> --- a/Documentation/dontdiff
> +++ b/Documentation/dontdiff
> @@ -76,6 +76,7 @@ Module.markers
> Module.symvers
> PENDING
> SCCS
> +symbols.klp
> System.map*
> TAGS
> aconf
> diff --git a/Makefile b/Makefile
> index 3f6628780eb2..dd5d6c258906 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -730,8 +730,13 @@ KBUILD_MODULES :=
> KBUILD_BUILTIN := 1
>
> # If we have only "make modules", don't compile built-in objects.
> +# When we're building livepatch modules, we need to consider the
> +# built-in objects during the descend as well, 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 :=
> + KBUILD_BUILTIN := $(if $(CONFIG_LIVEPATCH),1)
> endif
>
> # If we have "make <whatever> modules", compile modules
> @@ -1183,6 +1188,7 @@ PHONY += prepare0
> export extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
> export MODORDER := $(extmod_prefix)modules.order
> export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps
> +export MODULES_LIVEPATCH := $(extmod-prefix)modules.livepatch
>
> ifeq ($(KBUILD_EXTMOD),)
>
> @@ -1543,8 +1549,8 @@ endif
> #
>
> # *.ko are usually independent of vmlinux, but CONFIG_DEBUG_INFOBTF_MODULES
> -# is an exception.
> -ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +# and CONFIG_LIVEPATCH are exceptions.
> +ifneq ($(or $(CONFIG_DEBUG_INFO_BTF_MODULES),$(CONFIG_LIVEPATCH)),)
> KBUILD_BUILTIN := 1
> modules: vmlinux
> endif
> @@ -1602,14 +1608,14 @@ endif # CONFIG_MODULES
> CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
> modules.builtin modules.builtin.modinfo modules.nsdeps \
> compile_commands.json .thinlto-cache rust/test rust/doc \
> - .vmlinux.objs .vmlinux.export.c
> + modules.livepatch .vmlinux.objs .vmlinux.export.c
>
> # Directories & files removed with 'make mrproper'
> MRPROPER_FILES += include/config include/generated \
> arch/$(SRCARCH)/include/generated .objdiff \
> debian snap tar-install \
> .config .config.old .version \
> - Module.symvers \
> + Module.symvers symbols.klp \
> certs/signing_key.pem \
> certs/x509.genkey \
> vmlinux-gdb.py \
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index a30d5b08eee9..a8901e4e98c5 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -14,6 +14,7 @@ include $(srctree)/scripts/Makefile.lib
>
> # find all modules listed in modules.order
> modules := $(call read-file, $(MODORDER))
> +modules-klp := $(call read-file, $(MODULES_LIVEPATCH))
>
> __modfinal: $(modules:%.o=%.ko)
> @:
> @@ -65,6 +66,38 @@ endif
>
> targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)
>
> +# Livepatch
> +# ---------------------------------------------------------------------------
> +
> +%.tmp.ko: %.o %.mod.o symbols.klp FORCE
> + +$(call if_changed,ld_ko_o)
> +
> +quiet_cmd_klp_convert = KLP $@
> + cmd_klp_convert = scripts/livepatch/klp-convert symbols.klp $< $@
> +
> +$(modules-klp:%.o=%.ko): %.ko: %.tmp.ko FORCE
> + $(call if_changed,klp_convert)
> +
> +targets += $(modules-klp:.ko=.tmp.ko)
> +
> +ifeq ($(KBUILD_EXTMOD),)
> +filechk_klp_map = \
> + echo "klp-convert-symbol-data.0.1"; \
> + echo "*vmlinux"; \
> + $(NM) -f posix vmlinux | cut -d\ -f1; \
> + sort $(MODORDER) $(MODULES_LIVEPATCH) | \
> + uniq -u | \
> + sed 's/\.o$$//' | \
> + while read o; \
> + do \
> + echo "*$$(basename $$o)"; \
> + $(NM) -f posix $$o.o | cut -d\ -f1; \
> + done
> +
> +symbols.klp: FORCE
> + $(call filechk,klp_map)
> +endif
> +
> # Add FORCE to the prequisites of a target to force it to be always rebuilt.
> # ---------------------------------------------------------------------------
>
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 43343e13c542..02f1354d4cff 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -47,6 +47,7 @@ modpost-args = \
> $(if $(KBUILD_MODPOST_WARN),-w) \
> $(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS)) \
> $(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N) \
> + $(if $(CONFIG_LIVEPATCH),-l $(MODULES_LIVEPATCH)) \
> -o $@
>
> modpost-deps := $(MODPOST)
> @@ -138,6 +139,10 @@ $(output-symdump): $(modpost-deps) FORCE
> $(call if_changed,modpost)
>
> __modpost: $(output-symdump)
> +ifndef CONFIG_LIVEPATCH
> + $(Q)rm -f $(MODULES_LIVEPATCH)
> + $(Q)touch $(MODULES_LIVEPATCH)
> +endif
> PHONY += FORCE
> FORCE:
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index efff8078e395..0a8f0ce75761 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1831,6 +1831,10 @@ static void read_symbols(const char *modname)
> handle_moddevtable(mod, &info, sym, symname);
> }
>
> + /* Livepatch modules have unresolved symbols resolved by klp-convert */
> + if (get_modinfo(&info, "livepatch"))
> + mod->is_livepatch = true;
> +
> for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
> symname = remove_dot(info.strtab + sym->st_name);
>
> @@ -1919,7 +1923,7 @@ static void check_exports(struct module *mod)
> const char *basename;
> exp = find_symbol(s->name);
> if (!exp) {
> - if (!s->weak && nr_unresolved++ < MAX_UNRESOLVED_REPORTS)
> + if (!s->weak && !mod->is_livepatch && nr_unresolved++ < MAX_UNRESOLVED_REPORTS)
> modpost_log(warn_unresolved ? LOG_WARN : LOG_ERROR,
> "\"%s\" [%s.ko] undefined!\n",
> s->name, mod->name);
> @@ -2320,6 +2324,20 @@ static void write_namespace_deps_files(const char *fname)
> free(ns_deps_buf.p);
> }
>
> +static void write_livepatch_modules(const char *fname)
> +{
> + struct buffer buf = { };
> + struct module *mod;
> +
> + list_for_each_entry(mod, &modules, list) {
> + if (mod->is_livepatch)
> + buf_printf(&buf, "%s.o\n", mod->name);
> + }
> +
> + write_if_changed(&buf, fname);
> + free(buf.p);
> +}
> +
> struct dump_list {
> struct list_head list;
> const char *file;
> @@ -2330,11 +2348,12 @@ int main(int argc, char **argv)
> struct module *mod;
> char *missing_namespace_deps = NULL;
> char *dump_write = NULL, *files_source = NULL;
> + char *livepatch_modules = NULL;
> int opt;
> LIST_HEAD(dump_lists);
> struct dump_list *dl, *dl2;
>
> - while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
> + while ((opt = getopt(argc, argv, "ei:l:mnT:o:awENd:")) != -1) {
> switch (opt) {
> case 'e':
> external_module = true;
> @@ -2344,6 +2363,9 @@ int main(int argc, char **argv)
> dl->file = optarg;
> list_add_tail(&dl->list, &dump_lists);
> break;
> + case 'l':
> + livepatch_modules = optarg;
> + break;
> case 'm':
> modversions = true;
> break;
> @@ -2403,6 +2425,8 @@ int main(int argc, char **argv)
>
> if (dump_write)
> write_dump(dump_write);
> + if (livepatch_modules)
> + write_livepatch_modules(livepatch_modules);
> if (sec_mismatch_count && !sec_mismatch_warn_only)
> error("Section mismatches detected.\n"
> "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 1178f40a73f3..9be9bf6fb7da 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -119,6 +119,7 @@ struct module {
> bool is_gpl_compatible;
> bool from_dump; /* true if module was loaded from *.symvers */
> bool is_vmlinux;
> + bool is_livepatch;
> bool seen;
> bool has_init;
> bool has_cleanup;
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7 04/10] livepatch: Add sample livepatch module
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (2 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 03/10] kbuild/modpost: create symbols.klp and integrate klp-convert Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-14 18:17 ` Marcos Paulo de Souza
2023-03-06 14:08 ` [PATCH v7 05/10] documentation: Update on livepatch elf format Joe Lawrence
` (7 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
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_RELOC 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".
Update livepatch-sample.c to remove livepatch MODULE_INFO statement.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joao Moreira <jmoreira@suse.de>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
samples/livepatch/Makefile | 1 +
.../livepatch/livepatch-annotated-sample.c | 93 +++++++++++++++++++
2 files changed, 94 insertions(+)
create mode 100644 samples/livepatch/livepatch-annotated-sample.c
diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
index 9f853eeb6140..f2b41f4d6c16 100644
--- a/samples/livepatch/Makefile
+++ b/samples/livepatch/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix2.o
obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-demo.o
obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-mod.o
obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-busymod.o
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += 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..4fe0e16423c7
--- /dev/null
+++ b/samples/livepatch/livepatch-annotated-sample.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2014 Seth Jennings <sjenning@redhat.com>
+ */
+
+/*
+ * livepatch-annotated-sample.c - Kernel Live Patching Sample Module
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.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;
+
+#include <linux/seq_file.h>
+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 funcs[] = {
+ {
+ .old_name = "cmdline_proc_show",
+ .new_func = livepatch_cmdline_proc_show,
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int livepatch_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void livepatch_exit(void)
+{
+}
+
+module_init(livepatch_init);
+module_exit(livepatch_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 04/10] livepatch: Add sample livepatch module
2023-03-06 14:08 ` [PATCH v7 04/10] livepatch: Add sample livepatch module Joe Lawrence
@ 2023-03-14 18:17 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-14 18:17 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:18AM -0500, Joe Lawrence wrote:
> 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_RELOC 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".
>
> Update livepatch-sample.c to remove livepatch MODULE_INFO statement.
LGTM:
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Joao Moreira <jmoreira@suse.de>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> samples/livepatch/Makefile | 1 +
> .../livepatch/livepatch-annotated-sample.c | 93 +++++++++++++++++++
> 2 files changed, 94 insertions(+)
> create mode 100644 samples/livepatch/livepatch-annotated-sample.c
>
> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
> index 9f853eeb6140..f2b41f4d6c16 100644
> --- a/samples/livepatch/Makefile
> +++ b/samples/livepatch/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix2.o
> obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-demo.o
> obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-mod.o
> obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-busymod.o
> +obj-$(CONFIG_SAMPLE_LIVEPATCH) += 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..4fe0e16423c7
> --- /dev/null
> +++ b/samples/livepatch/livepatch-annotated-sample.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2014 Seth Jennings <sjenning@redhat.com>
> + */
> +
> +/*
> + * livepatch-annotated-sample.c - Kernel Live Patching Sample Module
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/livepatch.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;
> +
> +#include <linux/seq_file.h>
> +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 funcs[] = {
> + {
> + .old_name = "cmdline_proc_show",
> + .new_func = livepatch_cmdline_proc_show,
> + }, { }
> +};
> +
> +static struct klp_object objs[] = {
> + {
> + /* name being NULL means vmlinux */
> + .funcs = funcs,
> + }, { }
> +};
> +
> +static struct klp_patch patch = {
> + .mod = THIS_MODULE,
> + .objs = objs,
> +};
> +
> +static int livepatch_init(void)
> +{
> + return klp_enable_patch(&patch);
> +}
> +
> +static void livepatch_exit(void)
> +{
> +}
> +
> +module_init(livepatch_init);
> +module_exit(livepatch_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_INFO(livepatch, "Y");
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7 05/10] documentation: Update on livepatch elf format
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (3 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 04/10] livepatch: Add sample livepatch module Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-07 11:48 ` Marcos Paulo de Souza
2023-03-06 14:08 ` [PATCH v7 06/10] livepatch/selftests: add klp-convert Joe Lawrence
` (6 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
Add a section to Documentation/livepatch/module-elf-format.rst
describing how klp-convert works for fixing relocations.
Signed-off-by: Joao Moreira <jmoreira@suse.de>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
Documentation/livepatch/livepatch.rst | 3 ++
Documentation/livepatch/module-elf-format.rst | 42 +++++++++++++++++--
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/Documentation/livepatch/livepatch.rst b/Documentation/livepatch/livepatch.rst
index 68e3651e8af9..6b317a57c276 100644
--- a/Documentation/livepatch/livepatch.rst
+++ b/Documentation/livepatch/livepatch.rst
@@ -261,6 +261,9 @@ into three levels:
absolute position in the database, but rather the order it has been found
only for a particular object ( vmlinux or a kernel module ). Note that
kallsyms allows for searching symbols according to the object name.
+ Uniquely named symbols may use a symbol position of 0. Non-unique
+ symbols need to specify their object / kallsyms position, starting
+ at position 1.
- struct klp_object defines an array of patched functions (struct
klp_func) in the same object. Where the object is either vmlinux
diff --git a/Documentation/livepatch/module-elf-format.rst b/Documentation/livepatch/module-elf-format.rst
index 7347638895a0..72a072514581 100644
--- a/Documentation/livepatch/module-elf-format.rst
+++ b/Documentation/livepatch/module-elf-format.rst
@@ -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
@@ -259,7 +260,8 @@ Livepatch symbol names must conform to the following format::
The position of the symbol in the object (as according to kallsyms)
This is used to differentiate duplicate symbols within the same
object. The symbol position is expressed numerically (0, 1, 2...).
- The symbol position of a unique symbol is 0.
+ The symbol position of a unique symbol is 0. The symbol position of
+ the first non-unique symbol is 1, the second is 2, etc.
Examples:
---------
@@ -291,7 +293,41 @@ Examples:
Note that the 'Ndx' (Section index) for these symbols is SHN_LIVEPATCH (0xff20).
"OS" means OS-specific.
-5. Symbol table and Elf section access
+5. 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.klp, 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.
+
+6. Symbol table and Elf section access
======================================
A livepatch module's symbol table is accessible through module->symtab.
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 05/10] documentation: Update on livepatch elf format
2023-03-06 14:08 ` [PATCH v7 05/10] documentation: Update on livepatch elf format Joe Lawrence
@ 2023-03-07 11:48 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-07 11:48 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:19AM -0500, Joe Lawrence wrote:
> Add a section to Documentation/livepatch/module-elf-format.rst
> describing how klp-convert works for fixing relocations.
>
> Signed-off-by: Joao Moreira <jmoreira@suse.de>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
LGTM:
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> Documentation/livepatch/livepatch.rst | 3 ++
> Documentation/livepatch/module-elf-format.rst | 42 +++++++++++++++++--
> 2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/livepatch/livepatch.rst b/Documentation/livepatch/livepatch.rst
> index 68e3651e8af9..6b317a57c276 100644
> --- a/Documentation/livepatch/livepatch.rst
> +++ b/Documentation/livepatch/livepatch.rst
> @@ -261,6 +261,9 @@ into three levels:
> absolute position in the database, but rather the order it has been found
> only for a particular object ( vmlinux or a kernel module ). Note that
> kallsyms allows for searching symbols according to the object name.
> + Uniquely named symbols may use a symbol position of 0. Non-unique
> + symbols need to specify their object / kallsyms position, starting
> + at position 1.
>
> - struct klp_object defines an array of patched functions (struct
> klp_func) in the same object. Where the object is either vmlinux
> diff --git a/Documentation/livepatch/module-elf-format.rst b/Documentation/livepatch/module-elf-format.rst
> index 7347638895a0..72a072514581 100644
> --- a/Documentation/livepatch/module-elf-format.rst
> +++ b/Documentation/livepatch/module-elf-format.rst
> @@ -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
> @@ -259,7 +260,8 @@ Livepatch symbol names must conform to the following format::
> The position of the symbol in the object (as according to kallsyms)
> This is used to differentiate duplicate symbols within the same
> object. The symbol position is expressed numerically (0, 1, 2...).
> - The symbol position of a unique symbol is 0.
> + The symbol position of a unique symbol is 0. The symbol position of
> + the first non-unique symbol is 1, the second is 2, etc.
>
> Examples:
> ---------
> @@ -291,7 +293,41 @@ Examples:
> Note that the 'Ndx' (Section index) for these symbols is SHN_LIVEPATCH (0xff20).
> "OS" means OS-specific.
>
> -5. Symbol table and Elf section access
> +5. 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.klp, 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.
> +
> +6. Symbol table and Elf section access
> ======================================
> A livepatch module's symbol table is accessible through module->symtab.
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7 06/10] livepatch/selftests: add klp-convert
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (4 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 05/10] documentation: Update on livepatch elf format Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-14 20:22 ` Marcos Paulo de Souza
2023-03-06 14:08 ` [PATCH v7 07/10] livepatch/selftests: test multiple sections Joe Lawrence
` (5 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
Add a simple klp-convert livepatch selftest that exercises various
symbol homonym sympos scenarios.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
lib/livepatch/Makefile | 7 +
lib/livepatch/test_klp_convert.h | 14 ++
lib/livepatch/test_klp_convert1.c | 113 +++++++++++++++
lib/livepatch/test_klp_convert2.c | 110 ++++++++++++++
lib/livepatch/test_klp_convert_mod_a.c | 25 ++++
lib/livepatch/test_klp_convert_mod_b.c | 13 ++
.../selftests/livepatch/test-livepatch.sh | 134 ++++++++++++++++++
7 files changed, 416 insertions(+)
create mode 100644 lib/livepatch/test_klp_convert.h
create mode 100644 lib/livepatch/test_klp_convert1.c
create mode 100644 lib/livepatch/test_klp_convert2.c
create mode 100644 lib/livepatch/test_klp_convert_mod_a.c
create mode 100644 lib/livepatch/test_klp_convert_mod_b.c
diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
index dcc912b3478f..ced00515ff84 100644
--- a/lib/livepatch/Makefile
+++ b/lib/livepatch/Makefile
@@ -7,8 +7,15 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_callbacks_demo2.o \
test_klp_callbacks_busy.o \
test_klp_callbacks_mod.o \
+ test_klp_convert1.o \
+ test_klp_convert2.o \
+ test_klp_convert_mod.o \
test_klp_livepatch.o \
test_klp_shadow_vars.o \
test_klp_state.o \
test_klp_state2.o \
test_klp_state3.o
+
+test_klp_convert_mod-y := \
+ test_klp_convert_mod_a.o \
+ test_klp_convert_mod_b.o
diff --git a/lib/livepatch/test_klp_convert.h b/lib/livepatch/test_klp_convert.h
new file mode 100644
index 000000000000..5d97bc546d6e
--- /dev/null
+++ b/lib/livepatch/test_klp_convert.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _TEST_KLP_CONVERT_
+#define _TEST_KLP_CONVERT_
+
+/* klp-convert symbols - vmlinux */
+extern char *saved_command_line;
+/* klp-convert symbols - test_klp_convert_mod.ko */
+extern char driver_name[];
+extern char homonym_string[];
+extern const char *get_homonym_string(void);
+extern const char *test_klp_get_driver_name(void);
+
+#endif
diff --git a/lib/livepatch/test_klp_convert1.c b/lib/livepatch/test_klp_convert1.c
new file mode 100644
index 000000000000..d4e1163c01cc
--- /dev/null
+++ b/lib/livepatch/test_klp_convert1.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include "test_klp_convert.h"
+
+static noinline void print_saved_command_line(void)
+{
+ pr_info("saved_command_line, 0: %s\n", saved_command_line);
+}
+
+static noinline void print_driver_name(void)
+{
+ pr_info("driver_name, 0: %s\n", driver_name);
+ pr_info("test_klp_get_driver_name(), 0: %s\n", test_klp_get_driver_name());
+}
+
+static noinline void print_homonym_string(void)
+{
+ pr_info("homonym_string, 1: %s\n", homonym_string);
+ pr_info("get_homonym_string(), 1: %s\n", get_homonym_string());
+}
+
+/* provide a sysfs handle to invoke debug functions */
+static int print_debug;
+static int print_debug_set(const char *val, const struct kernel_param *kp)
+{
+ print_saved_command_line();
+ print_driver_name();
+ print_homonym_string();
+
+ return 0;
+}
+static const struct kernel_param_ops print_debug_ops = {
+ .set = print_debug_set,
+ .get = param_get_int,
+};
+module_param_cb(print_debug, &print_debug_ops, &print_debug, 0200);
+MODULE_PARM_DESC(print_debug, "print klp-convert debugging info");
+
+/*
+ * saved_command_line is a unique symbol, so the sympos annotation is
+ * optional. Provide to test that sympos=0 works correctly.
+ */
+KLP_MODULE_RELOC(vmlinux) vmlinux_relocs[] = {
+ KLP_SYMPOS(saved_command_line, 0)
+};
+
+/*
+ * driver_name symbols can be found in vmlinux (multiple) and also
+ * test_klp_convert_mod, therefore the annotation is required to
+ * clarify that we want the one from test_klp_convert_mod.
+ *
+ * test_klp_convert_mod contains multiple homonym_string and
+ * get_homonym_string symbols, test resolving the first set here and
+ * the others in test_klp_convert2.c
+ *
+ * test_klp_get_driver_name is a uniquely named symbol, test that sympos=0
+ * work correctly.
+ */
+KLP_MODULE_RELOC(test_klp_convert_mod) test_klp_convert_mod_relocs_a[] = {
+ KLP_SYMPOS(driver_name, 0),
+ KLP_SYMPOS(homonym_string, 1),
+ KLP_SYMPOS(get_homonym_string, 1),
+ KLP_SYMPOS(test_klp_get_driver_name, 0),
+};
+
+static struct klp_func funcs[] = {
+ {
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ },
+ {
+ .name = "test_klp_convert_mod",
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int test_klp_convert_init(void)
+{
+ int ret;
+
+ ret = klp_enable_patch(&patch);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void test_klp_convert_exit(void)
+{
+}
+
+module_init(test_klp_convert_init);
+module_exit(test_klp_convert_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: klp-convert1");
+MODULE_INFO(livepatch, "Y");
diff --git a/lib/livepatch/test_klp_convert2.c b/lib/livepatch/test_klp_convert2.c
new file mode 100644
index 000000000000..2a2a153aa0b9
--- /dev/null
+++ b/lib/livepatch/test_klp_convert2.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include "test_klp_convert.h"
+
+static noinline void print_saved_command_line(void)
+{
+ pr_info("saved_command_line (auto): %s\n", saved_command_line);
+}
+
+static noinline void print_driver_name(void)
+{
+ pr_info("driver_name, 0: %s\n", driver_name);
+ pr_info("test_klp_get_driver_name(), (auto): %s\n", test_klp_get_driver_name());
+}
+
+static noinline void print_homonym_string(void)
+{
+ pr_info("homonym_string, 2: %s\n", homonym_string);
+ pr_info("get_homonym_string(), 2: %s\n", get_homonym_string());
+}
+
+/* provide a sysfs handle to invoke debug functions */
+static int print_debug;
+static int print_debug_set(const char *val, const struct kernel_param *kp)
+{
+ print_saved_command_line();
+ print_driver_name();
+ print_homonym_string();
+
+ return 0;
+}
+static const struct kernel_param_ops print_debug_ops = {
+ .set = print_debug_set,
+ .get = param_get_int,
+};
+module_param_cb(print_debug, &print_debug_ops, &print_debug, 0200);
+MODULE_PARM_DESC(print_debug, "print klp-convert debugging info");
+
+/*
+ * saved_command_line is a uniquely named symbol, so the sympos
+ * annotation is optional. Skip it and test that klp-convert can
+ * resolve the symbol on its own.
+ */
+
+/*
+ * driver_name symbols can be found in vmlinux (multiple) and also
+ * test_klp_convert_mod, therefore the annotation is required to
+ * clarify that we want the one from test_klp_convert_mod.
+ *
+ * test_klp_convert_mod contains multiple homonym_string symbols,
+ * test_klp_convert1.c resolved to the first one, resolve to the
+ * second one here.
+ *
+ * test_klp_get_driver_name is a uniquely named symbol, test klp-convert can
+ * resolve it automatically.
+ */
+KLP_MODULE_RELOC(test_klp_convert_mod) test_klp_convert_mod_relocs_a[] = {
+ KLP_SYMPOS(driver_name, 0),
+ KLP_SYMPOS(homonym_string, 2),
+ KLP_SYMPOS(get_homonym_string, 2),
+};
+
+static struct klp_func funcs[] = {
+ {
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ },
+ {
+ .name = "test_klp_convert_mod",
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int test_klp_convert_init(void)
+{
+ int ret;
+
+ ret = klp_enable_patch(&patch);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void test_klp_convert_exit(void)
+{
+}
+
+module_init(test_klp_convert_init);
+module_exit(test_klp_convert_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: klp-convert2");
+MODULE_INFO(livepatch, "Y");
diff --git a/lib/livepatch/test_klp_convert_mod_a.c b/lib/livepatch/test_klp_convert_mod_a.c
new file mode 100644
index 000000000000..ae5e911fbb9b
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_mod_a.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+/* Unique symbols that don't need sympos annotation */
+static const char driver_name[] = KBUILD_MODNAME;
+__used static const char *test_klp_get_driver_name(void)
+{
+ return driver_name;
+}
+
+/* Common symbol names that need sympos */
+static const char homonym_string[] = "homonym string A";
+__used static const char *get_homonym_string(void)
+{
+ return homonym_string;
+}
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: klp-convert module");
diff --git a/lib/livepatch/test_klp_convert_mod_b.c b/lib/livepatch/test_klp_convert_mod_b.c
new file mode 100644
index 000000000000..5eca8a4cae38
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_mod_b.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
+
+/*
+ * A second compilation unit to provide another set of similarly named
+ * symbols, forcing a livepatch to use sympos annotations.
+ */
+
+static const char homonym_string[] = "homonym string B";
+__used static const char *get_homonym_string(void)
+{
+ return homonym_string;
+}
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index 5fe79ac34be1..25c21ddf30f0 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -6,6 +6,10 @@
MOD_LIVEPATCH=test_klp_livepatch
MOD_REPLACE=test_klp_atomic_replace
+MOD_KLP_CONVERT_MOD=test_klp_convert_mod
+MOD_KLP_CONVERT1=test_klp_convert1
+MOD_KLP_CONVERT2=test_klp_convert2
+MOD_KLP_CONVERT_SECTIONS=test_klp_convert_sections
setup_config
@@ -159,4 +163,134 @@ livepatch: '$MOD_REPLACE': unpatching complete
% rmmod $MOD_REPLACE"
+# TEST: klp-convert symbols
+# - load a livepatch that modifies the output from /proc/cmdline
+# including a reference to vmlinux-local symbol that klp-convert
+# will process
+# - verify correct behavior
+# - unload the livepatch and make sure the patch was removed
+
+start_test "klp-convert symbols"
+
+saved_cmdline=$(cat /proc/cmdline)
+
+load_mod $MOD_KLP_CONVERT_MOD
+
+load_lp $MOD_KLP_CONVERT1
+echo 1 > /sys/module/$MOD_KLP_CONVERT1/parameters/print_debug
+disable_lp $MOD_KLP_CONVERT1
+unload_lp $MOD_KLP_CONVERT1
+
+load_lp $MOD_KLP_CONVERT2
+echo 1 > /sys/module/$MOD_KLP_CONVERT2/parameters/print_debug
+disable_lp $MOD_KLP_CONVERT2
+unload_lp $MOD_KLP_CONVERT2
+
+unload_mod $MOD_KLP_CONVERT_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT_MOD
+% modprobe $MOD_KLP_CONVERT1
+livepatch: enabling patch '$MOD_KLP_CONVERT1'
+livepatch: '$MOD_KLP_CONVERT1': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT1': starting patching transition
+livepatch: '$MOD_KLP_CONVERT1': completing patching transition
+livepatch: '$MOD_KLP_CONVERT1': patching complete
+$MOD_KLP_CONVERT1: saved_command_line, 0: $saved_cmdline
+$MOD_KLP_CONVERT1: driver_name, 0: $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT1: test_klp_get_driver_name(), 0: $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT1: homonym_string, 1: homonym string A
+$MOD_KLP_CONVERT1: get_homonym_string(), 1: homonym string A
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT1/enabled
+livepatch: '$MOD_KLP_CONVERT1': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT1': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT1': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT1': unpatching complete
+% rmmod $MOD_KLP_CONVERT1
+% modprobe $MOD_KLP_CONVERT2
+livepatch: enabling patch '$MOD_KLP_CONVERT2'
+livepatch: '$MOD_KLP_CONVERT2': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT2': starting patching transition
+livepatch: '$MOD_KLP_CONVERT2': completing patching transition
+livepatch: '$MOD_KLP_CONVERT2': patching complete
+$MOD_KLP_CONVERT2: saved_command_line (auto): $saved_cmdline
+$MOD_KLP_CONVERT2: driver_name, 0: $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT2: test_klp_get_driver_name(), (auto): $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT2: homonym_string, 2: homonym string B
+$MOD_KLP_CONVERT2: get_homonym_string(), 2: homonym string B
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT2/enabled
+livepatch: '$MOD_KLP_CONVERT2': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT2': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT2': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT2': unpatching complete
+% rmmod $MOD_KLP_CONVERT2
+% rmmod $MOD_KLP_CONVERT_MOD"
+
+
+# TEST: klp-convert symbols (late module patching)
+# - load a livepatch that modifies the output from /proc/cmdline
+# including a reference to vmlinux-local symbol that klp-convert
+# will process
+# - load target module
+# - verify correct behavior
+# - unload the livepatch
+
+start_test "klp-convert symbols (late module patching)"
+
+saved_cmdline=$(cat /proc/cmdline)
+
+load_lp $MOD_KLP_CONVERT1
+load_mod $MOD_KLP_CONVERT_MOD
+echo 1 > /sys/module/$MOD_KLP_CONVERT1/parameters/print_debug
+disable_lp $MOD_KLP_CONVERT1
+unload_lp $MOD_KLP_CONVERT1
+unload_mod $MOD_KLP_CONVERT_MOD
+
+load_lp $MOD_KLP_CONVERT2
+load_mod $MOD_KLP_CONVERT_MOD
+echo 1 > /sys/module/$MOD_KLP_CONVERT2/parameters/print_debug
+disable_lp $MOD_KLP_CONVERT2
+unload_lp $MOD_KLP_CONVERT2
+unload_mod $MOD_KLP_CONVERT_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT1
+livepatch: enabling patch '$MOD_KLP_CONVERT1'
+livepatch: '$MOD_KLP_CONVERT1': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT1': starting patching transition
+livepatch: '$MOD_KLP_CONVERT1': completing patching transition
+livepatch: '$MOD_KLP_CONVERT1': patching complete
+% modprobe $MOD_KLP_CONVERT_MOD
+livepatch: applying patch '$MOD_KLP_CONVERT1' to loading module '$MOD_KLP_CONVERT_MOD'
+$MOD_KLP_CONVERT1: saved_command_line, 0: $saved_cmdline
+$MOD_KLP_CONVERT1: driver_name, 0: $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT1: test_klp_get_driver_name(), 0: $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT1: homonym_string, 1: homonym string A
+$MOD_KLP_CONVERT1: get_homonym_string(), 1: homonym string A
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT1/enabled
+livepatch: '$MOD_KLP_CONVERT1': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT1': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT1': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT1': unpatching complete
+% rmmod $MOD_KLP_CONVERT1
+% rmmod $MOD_KLP_CONVERT_MOD
+% modprobe $MOD_KLP_CONVERT2
+livepatch: enabling patch '$MOD_KLP_CONVERT2'
+livepatch: '$MOD_KLP_CONVERT2': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT2': starting patching transition
+livepatch: '$MOD_KLP_CONVERT2': completing patching transition
+livepatch: '$MOD_KLP_CONVERT2': patching complete
+% modprobe $MOD_KLP_CONVERT_MOD
+livepatch: applying patch '$MOD_KLP_CONVERT2' to loading module '$MOD_KLP_CONVERT_MOD'
+$MOD_KLP_CONVERT2: saved_command_line (auto): $saved_cmdline
+$MOD_KLP_CONVERT2: driver_name, 0: $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT2: test_klp_get_driver_name(), (auto): $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT2: homonym_string, 2: homonym string B
+$MOD_KLP_CONVERT2: get_homonym_string(), 2: homonym string B
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT2/enabled
+livepatch: '$MOD_KLP_CONVERT2': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT2': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT2': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT2': unpatching complete
+% rmmod $MOD_KLP_CONVERT2
+% rmmod $MOD_KLP_CONVERT_MOD"
+
exit 0
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 06/10] livepatch/selftests: add klp-convert
2023-03-06 14:08 ` [PATCH v7 06/10] livepatch/selftests: add klp-convert Joe Lawrence
@ 2023-03-14 20:22 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-14 20:22 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:20AM -0500, Joe Lawrence wrote:
> Add a simple klp-convert livepatch selftest that exercises various
> symbol homonym sympos scenarios.
LGTM:
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> lib/livepatch/Makefile | 7 +
> lib/livepatch/test_klp_convert.h | 14 ++
> lib/livepatch/test_klp_convert1.c | 113 +++++++++++++++
> lib/livepatch/test_klp_convert2.c | 110 ++++++++++++++
> lib/livepatch/test_klp_convert_mod_a.c | 25 ++++
> lib/livepatch/test_klp_convert_mod_b.c | 13 ++
> .../selftests/livepatch/test-livepatch.sh | 134 ++++++++++++++++++
> 7 files changed, 416 insertions(+)
> create mode 100644 lib/livepatch/test_klp_convert.h
> create mode 100644 lib/livepatch/test_klp_convert1.c
> create mode 100644 lib/livepatch/test_klp_convert2.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_a.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_b.c
>
> diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
> index dcc912b3478f..ced00515ff84 100644
> --- a/lib/livepatch/Makefile
> +++ b/lib/livepatch/Makefile
> @@ -7,8 +7,15 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
> test_klp_callbacks_demo2.o \
> test_klp_callbacks_busy.o \
> test_klp_callbacks_mod.o \
> + test_klp_convert1.o \
> + test_klp_convert2.o \
> + test_klp_convert_mod.o \
> test_klp_livepatch.o \
> test_klp_shadow_vars.o \
> test_klp_state.o \
> test_klp_state2.o \
> test_klp_state3.o
> +
> +test_klp_convert_mod-y := \
> + test_klp_convert_mod_a.o \
> + test_klp_convert_mod_b.o
> diff --git a/lib/livepatch/test_klp_convert.h b/lib/livepatch/test_klp_convert.h
> new file mode 100644
> index 000000000000..5d97bc546d6e
> --- /dev/null
> +++ b/lib/livepatch/test_klp_convert.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _TEST_KLP_CONVERT_
> +#define _TEST_KLP_CONVERT_
> +
> +/* klp-convert symbols - vmlinux */
> +extern char *saved_command_line;
> +/* klp-convert symbols - test_klp_convert_mod.ko */
> +extern char driver_name[];
> +extern char homonym_string[];
> +extern const char *get_homonym_string(void);
> +extern const char *test_klp_get_driver_name(void);
> +
> +#endif
> diff --git a/lib/livepatch/test_klp_convert1.c b/lib/livepatch/test_klp_convert1.c
> new file mode 100644
> index 000000000000..d4e1163c01cc
> --- /dev/null
> +++ b/lib/livepatch/test_klp_convert1.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/livepatch.h>
> +#include "test_klp_convert.h"
> +
> +static noinline void print_saved_command_line(void)
> +{
> + pr_info("saved_command_line, 0: %s\n", saved_command_line);
> +}
> +
> +static noinline void print_driver_name(void)
> +{
> + pr_info("driver_name, 0: %s\n", driver_name);
> + pr_info("test_klp_get_driver_name(), 0: %s\n", test_klp_get_driver_name());
> +}
> +
> +static noinline void print_homonym_string(void)
> +{
> + pr_info("homonym_string, 1: %s\n", homonym_string);
> + pr_info("get_homonym_string(), 1: %s\n", get_homonym_string());
> +}
> +
> +/* provide a sysfs handle to invoke debug functions */
> +static int print_debug;
> +static int print_debug_set(const char *val, const struct kernel_param *kp)
> +{
> + print_saved_command_line();
> + print_driver_name();
> + print_homonym_string();
> +
> + return 0;
> +}
> +static const struct kernel_param_ops print_debug_ops = {
> + .set = print_debug_set,
> + .get = param_get_int,
> +};
> +module_param_cb(print_debug, &print_debug_ops, &print_debug, 0200);
> +MODULE_PARM_DESC(print_debug, "print klp-convert debugging info");
> +
> +/*
> + * saved_command_line is a unique symbol, so the sympos annotation is
> + * optional. Provide to test that sympos=0 works correctly.
> + */
> +KLP_MODULE_RELOC(vmlinux) vmlinux_relocs[] = {
> + KLP_SYMPOS(saved_command_line, 0)
> +};
> +
> +/*
> + * driver_name symbols can be found in vmlinux (multiple) and also
> + * test_klp_convert_mod, therefore the annotation is required to
> + * clarify that we want the one from test_klp_convert_mod.
> + *
> + * test_klp_convert_mod contains multiple homonym_string and
> + * get_homonym_string symbols, test resolving the first set here and
> + * the others in test_klp_convert2.c
> + *
> + * test_klp_get_driver_name is a uniquely named symbol, test that sympos=0
> + * work correctly.
> + */
> +KLP_MODULE_RELOC(test_klp_convert_mod) test_klp_convert_mod_relocs_a[] = {
> + KLP_SYMPOS(driver_name, 0),
> + KLP_SYMPOS(homonym_string, 1),
> + KLP_SYMPOS(get_homonym_string, 1),
> + KLP_SYMPOS(test_klp_get_driver_name, 0),
> +};
> +
> +static struct klp_func funcs[] = {
> + {
> + }, { }
> +};
> +
> +static struct klp_object objs[] = {
> + {
> + /* name being NULL means vmlinux */
> + .funcs = funcs,
> + },
> + {
> + .name = "test_klp_convert_mod",
> + .funcs = funcs,
> + }, { }
> +};
> +
> +static struct klp_patch patch = {
> + .mod = THIS_MODULE,
> + .objs = objs,
> +};
> +
> +static int test_klp_convert_init(void)
> +{
> + int ret;
> +
> + ret = klp_enable_patch(&patch);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static void test_klp_convert_exit(void)
> +{
> +}
> +
> +module_init(test_klp_convert_init);
> +module_exit(test_klp_convert_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
> +MODULE_DESCRIPTION("Livepatch test: klp-convert1");
> +MODULE_INFO(livepatch, "Y");
> diff --git a/lib/livepatch/test_klp_convert2.c b/lib/livepatch/test_klp_convert2.c
> new file mode 100644
> index 000000000000..2a2a153aa0b9
> --- /dev/null
> +++ b/lib/livepatch/test_klp_convert2.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/livepatch.h>
> +#include "test_klp_convert.h"
> +
> +static noinline void print_saved_command_line(void)
> +{
> + pr_info("saved_command_line (auto): %s\n", saved_command_line);
> +}
> +
> +static noinline void print_driver_name(void)
> +{
> + pr_info("driver_name, 0: %s\n", driver_name);
> + pr_info("test_klp_get_driver_name(), (auto): %s\n", test_klp_get_driver_name());
> +}
> +
> +static noinline void print_homonym_string(void)
> +{
> + pr_info("homonym_string, 2: %s\n", homonym_string);
> + pr_info("get_homonym_string(), 2: %s\n", get_homonym_string());
> +}
> +
> +/* provide a sysfs handle to invoke debug functions */
> +static int print_debug;
> +static int print_debug_set(const char *val, const struct kernel_param *kp)
> +{
> + print_saved_command_line();
> + print_driver_name();
> + print_homonym_string();
> +
> + return 0;
> +}
> +static const struct kernel_param_ops print_debug_ops = {
> + .set = print_debug_set,
> + .get = param_get_int,
> +};
> +module_param_cb(print_debug, &print_debug_ops, &print_debug, 0200);
> +MODULE_PARM_DESC(print_debug, "print klp-convert debugging info");
> +
> +/*
> + * saved_command_line is a uniquely named symbol, so the sympos
> + * annotation is optional. Skip it and test that klp-convert can
> + * resolve the symbol on its own.
> + */
> +
> +/*
> + * driver_name symbols can be found in vmlinux (multiple) and also
> + * test_klp_convert_mod, therefore the annotation is required to
> + * clarify that we want the one from test_klp_convert_mod.
> + *
> + * test_klp_convert_mod contains multiple homonym_string symbols,
> + * test_klp_convert1.c resolved to the first one, resolve to the
> + * second one here.
> + *
> + * test_klp_get_driver_name is a uniquely named symbol, test klp-convert can
> + * resolve it automatically.
> + */
> +KLP_MODULE_RELOC(test_klp_convert_mod) test_klp_convert_mod_relocs_a[] = {
> + KLP_SYMPOS(driver_name, 0),
> + KLP_SYMPOS(homonym_string, 2),
> + KLP_SYMPOS(get_homonym_string, 2),
> +};
> +
> +static struct klp_func funcs[] = {
> + {
> + }, { }
> +};
> +
> +static struct klp_object objs[] = {
> + {
> + /* name being NULL means vmlinux */
> + .funcs = funcs,
> + },
> + {
> + .name = "test_klp_convert_mod",
> + .funcs = funcs,
> + }, { }
> +};
> +
> +static struct klp_patch patch = {
> + .mod = THIS_MODULE,
> + .objs = objs,
> +};
> +
> +static int test_klp_convert_init(void)
> +{
> + int ret;
> +
> + ret = klp_enable_patch(&patch);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static void test_klp_convert_exit(void)
> +{
> +}
> +
> +module_init(test_klp_convert_init);
> +module_exit(test_klp_convert_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
> +MODULE_DESCRIPTION("Livepatch test: klp-convert2");
> +MODULE_INFO(livepatch, "Y");
> diff --git a/lib/livepatch/test_klp_convert_mod_a.c b/lib/livepatch/test_klp_convert_mod_a.c
> new file mode 100644
> index 000000000000..ae5e911fbb9b
> --- /dev/null
> +++ b/lib/livepatch/test_klp_convert_mod_a.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +
> +/* Unique symbols that don't need sympos annotation */
> +static const char driver_name[] = KBUILD_MODNAME;
> +__used static const char *test_klp_get_driver_name(void)
> +{
> + return driver_name;
> +}
> +
> +/* Common symbol names that need sympos */
> +static const char homonym_string[] = "homonym string A";
> +__used static const char *get_homonym_string(void)
> +{
> + return homonym_string;
> +}
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
> +MODULE_DESCRIPTION("Livepatch test: klp-convert module");
> diff --git a/lib/livepatch/test_klp_convert_mod_b.c b/lib/livepatch/test_klp_convert_mod_b.c
> new file mode 100644
> index 000000000000..5eca8a4cae38
> --- /dev/null
> +++ b/lib/livepatch/test_klp_convert_mod_b.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019 Joe Lawrence <joe.lawrence@redhat.com>
> +
> +/*
> + * A second compilation unit to provide another set of similarly named
> + * symbols, forcing a livepatch to use sympos annotations.
> + */
> +
> +static const char homonym_string[] = "homonym string B";
> +__used static const char *get_homonym_string(void)
> +{
> + return homonym_string;
> +}
> diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
> index 5fe79ac34be1..25c21ddf30f0 100755
> --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
> @@ -6,6 +6,10 @@
>
> MOD_LIVEPATCH=test_klp_livepatch
> MOD_REPLACE=test_klp_atomic_replace
> +MOD_KLP_CONVERT_MOD=test_klp_convert_mod
> +MOD_KLP_CONVERT1=test_klp_convert1
> +MOD_KLP_CONVERT2=test_klp_convert2
> +MOD_KLP_CONVERT_SECTIONS=test_klp_convert_sections
>
> setup_config
>
> @@ -159,4 +163,134 @@ livepatch: '$MOD_REPLACE': unpatching complete
> % rmmod $MOD_REPLACE"
>
>
> +# TEST: klp-convert symbols
> +# - load a livepatch that modifies the output from /proc/cmdline
> +# including a reference to vmlinux-local symbol that klp-convert
> +# will process
> +# - verify correct behavior
> +# - unload the livepatch and make sure the patch was removed
> +
> +start_test "klp-convert symbols"
> +
> +saved_cmdline=$(cat /proc/cmdline)
> +
> +load_mod $MOD_KLP_CONVERT_MOD
> +
> +load_lp $MOD_KLP_CONVERT1
> +echo 1 > /sys/module/$MOD_KLP_CONVERT1/parameters/print_debug
> +disable_lp $MOD_KLP_CONVERT1
> +unload_lp $MOD_KLP_CONVERT1
> +
> +load_lp $MOD_KLP_CONVERT2
> +echo 1 > /sys/module/$MOD_KLP_CONVERT2/parameters/print_debug
> +disable_lp $MOD_KLP_CONVERT2
> +unload_lp $MOD_KLP_CONVERT2
> +
> +unload_mod $MOD_KLP_CONVERT_MOD
> +
> +check_result "% modprobe $MOD_KLP_CONVERT_MOD
> +% modprobe $MOD_KLP_CONVERT1
> +livepatch: enabling patch '$MOD_KLP_CONVERT1'
> +livepatch: '$MOD_KLP_CONVERT1': initializing patching transition
> +livepatch: '$MOD_KLP_CONVERT1': starting patching transition
> +livepatch: '$MOD_KLP_CONVERT1': completing patching transition
> +livepatch: '$MOD_KLP_CONVERT1': patching complete
> +$MOD_KLP_CONVERT1: saved_command_line, 0: $saved_cmdline
> +$MOD_KLP_CONVERT1: driver_name, 0: $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT1: test_klp_get_driver_name(), 0: $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT1: homonym_string, 1: homonym string A
> +$MOD_KLP_CONVERT1: get_homonym_string(), 1: homonym string A
> +% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT1/enabled
> +livepatch: '$MOD_KLP_CONVERT1': initializing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT1': starting unpatching transition
> +livepatch: '$MOD_KLP_CONVERT1': completing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT1': unpatching complete
> +% rmmod $MOD_KLP_CONVERT1
> +% modprobe $MOD_KLP_CONVERT2
> +livepatch: enabling patch '$MOD_KLP_CONVERT2'
> +livepatch: '$MOD_KLP_CONVERT2': initializing patching transition
> +livepatch: '$MOD_KLP_CONVERT2': starting patching transition
> +livepatch: '$MOD_KLP_CONVERT2': completing patching transition
> +livepatch: '$MOD_KLP_CONVERT2': patching complete
> +$MOD_KLP_CONVERT2: saved_command_line (auto): $saved_cmdline
> +$MOD_KLP_CONVERT2: driver_name, 0: $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT2: test_klp_get_driver_name(), (auto): $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT2: homonym_string, 2: homonym string B
> +$MOD_KLP_CONVERT2: get_homonym_string(), 2: homonym string B
> +% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT2/enabled
> +livepatch: '$MOD_KLP_CONVERT2': initializing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT2': starting unpatching transition
> +livepatch: '$MOD_KLP_CONVERT2': completing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT2': unpatching complete
> +% rmmod $MOD_KLP_CONVERT2
> +% rmmod $MOD_KLP_CONVERT_MOD"
> +
> +
> +# TEST: klp-convert symbols (late module patching)
> +# - load a livepatch that modifies the output from /proc/cmdline
> +# including a reference to vmlinux-local symbol that klp-convert
> +# will process
> +# - load target module
> +# - verify correct behavior
> +# - unload the livepatch
> +
> +start_test "klp-convert symbols (late module patching)"
> +
> +saved_cmdline=$(cat /proc/cmdline)
> +
> +load_lp $MOD_KLP_CONVERT1
> +load_mod $MOD_KLP_CONVERT_MOD
> +echo 1 > /sys/module/$MOD_KLP_CONVERT1/parameters/print_debug
> +disable_lp $MOD_KLP_CONVERT1
> +unload_lp $MOD_KLP_CONVERT1
> +unload_mod $MOD_KLP_CONVERT_MOD
> +
> +load_lp $MOD_KLP_CONVERT2
> +load_mod $MOD_KLP_CONVERT_MOD
> +echo 1 > /sys/module/$MOD_KLP_CONVERT2/parameters/print_debug
> +disable_lp $MOD_KLP_CONVERT2
> +unload_lp $MOD_KLP_CONVERT2
> +unload_mod $MOD_KLP_CONVERT_MOD
> +
> +check_result "% modprobe $MOD_KLP_CONVERT1
> +livepatch: enabling patch '$MOD_KLP_CONVERT1'
> +livepatch: '$MOD_KLP_CONVERT1': initializing patching transition
> +livepatch: '$MOD_KLP_CONVERT1': starting patching transition
> +livepatch: '$MOD_KLP_CONVERT1': completing patching transition
> +livepatch: '$MOD_KLP_CONVERT1': patching complete
> +% modprobe $MOD_KLP_CONVERT_MOD
> +livepatch: applying patch '$MOD_KLP_CONVERT1' to loading module '$MOD_KLP_CONVERT_MOD'
> +$MOD_KLP_CONVERT1: saved_command_line, 0: $saved_cmdline
> +$MOD_KLP_CONVERT1: driver_name, 0: $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT1: test_klp_get_driver_name(), 0: $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT1: homonym_string, 1: homonym string A
> +$MOD_KLP_CONVERT1: get_homonym_string(), 1: homonym string A
> +% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT1/enabled
> +livepatch: '$MOD_KLP_CONVERT1': initializing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT1': starting unpatching transition
> +livepatch: '$MOD_KLP_CONVERT1': completing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT1': unpatching complete
> +% rmmod $MOD_KLP_CONVERT1
> +% rmmod $MOD_KLP_CONVERT_MOD
> +% modprobe $MOD_KLP_CONVERT2
> +livepatch: enabling patch '$MOD_KLP_CONVERT2'
> +livepatch: '$MOD_KLP_CONVERT2': initializing patching transition
> +livepatch: '$MOD_KLP_CONVERT2': starting patching transition
> +livepatch: '$MOD_KLP_CONVERT2': completing patching transition
> +livepatch: '$MOD_KLP_CONVERT2': patching complete
> +% modprobe $MOD_KLP_CONVERT_MOD
> +livepatch: applying patch '$MOD_KLP_CONVERT2' to loading module '$MOD_KLP_CONVERT_MOD'
> +$MOD_KLP_CONVERT2: saved_command_line (auto): $saved_cmdline
> +$MOD_KLP_CONVERT2: driver_name, 0: $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT2: test_klp_get_driver_name(), (auto): $MOD_KLP_CONVERT_MOD
> +$MOD_KLP_CONVERT2: homonym_string, 2: homonym string B
> +$MOD_KLP_CONVERT2: get_homonym_string(), 2: homonym string B
> +% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT2/enabled
> +livepatch: '$MOD_KLP_CONVERT2': initializing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT2': starting unpatching transition
> +livepatch: '$MOD_KLP_CONVERT2': completing unpatching transition
> +livepatch: '$MOD_KLP_CONVERT2': unpatching complete
> +% rmmod $MOD_KLP_CONVERT2
> +% rmmod $MOD_KLP_CONVERT_MOD"
> +
> exit 0
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v7 07/10] livepatch/selftests: test multiple sections
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (5 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 06/10] livepatch/selftests: add klp-convert Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-06 14:08 ` [PATCH v7 08/10] livepatch/selftests: add __asm__ symbol renaming examples Joe Lawrence
` (4 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
Add a test case in which klp-convert needs to convert a symbol that is
shared across ELF sections. Special sections like .altinstructions do
this. It is possible that symbols may be shared across various .text
sections as well.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
lib/livepatch/Makefile | 1 +
lib/livepatch/test_klp_convert_sections.c | 120 ++++++++++++++++++
.../selftests/livepatch/test-livepatch.sh | 43 +++++++
3 files changed, 164 insertions(+)
create mode 100644 lib/livepatch/test_klp_convert_sections.c
diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
index ced00515ff84..77e21317d4da 100644
--- a/lib/livepatch/Makefile
+++ b/lib/livepatch/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_callbacks_mod.o \
test_klp_convert1.o \
test_klp_convert2.o \
+ test_klp_convert_sections.o \
test_klp_convert_mod.o \
test_klp_livepatch.o \
test_klp_shadow_vars.o \
diff --git a/lib/livepatch/test_klp_convert_sections.c b/lib/livepatch/test_klp_convert_sections.c
new file mode 100644
index 000000000000..fb6cdb9aaa90
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_sections.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include "test_klp_convert.h"
+
+/* klp-convert symbols - vmlinux */
+extern char *saved_command_line;
+
+/*
+ * Scatter references to the same symbol (saved_command_line) across a
+ * few different ELF sections. At the same time, include multiple
+ * references within the same function.
+ */
+__section(".text.print_saved_command_line")
+static noinline void print_saved_command_line(void)
+{
+ pr_info("saved_command_line (1): %s\n", saved_command_line);
+}
+
+__section(".text.print_saved_command_line2")
+static noinline void print_saved_command_line2(void)
+{
+ pr_info("saved_command_line (1): %s\n", saved_command_line);
+ pr_info("saved_command_line (2): %s\n", saved_command_line);
+}
+
+__section(".text.print_saved_command_line3")
+static noinline void print_saved_command_line3(void)
+{
+ pr_info("saved_command_line (1): %s\n", saved_command_line);
+ pr_info("saved_command_line (2): %s\n", saved_command_line);
+ pr_info("saved_command_line (3): %s\n", saved_command_line);
+}
+
+/*
+ * Create relocations in .rela.data that need conversion, sharing
+ * symbols with ordinary .text relas.
+ */
+const char *(*p_test_klp_get_driver_name)(void) = test_klp_get_driver_name;
+const char *(*p_get_homonym_string)(void) = get_homonym_string;
+
+static noinline void print_via_function_pointers(void)
+{
+ pr_info("test_klp_get_driver_name(): %s\n", test_klp_get_driver_name());
+ pr_info("p_test_klp_get_driver_name(): %s\n", p_test_klp_get_driver_name());
+ pr_info("get_homonym_string(): %s\n", get_homonym_string());
+ pr_info("p_get_homonym_string(): %s\n", p_get_homonym_string());
+}
+
+/* provide a sysfs handle to invoke debug functions */
+static int print_debug;
+static int print_debug_set(const char *val, const struct kernel_param *kp)
+{
+ print_saved_command_line();
+ print_saved_command_line2();
+ print_saved_command_line3();
+ print_via_function_pointers();
+
+ return 0;
+}
+static const struct kernel_param_ops print_debug_ops = {
+ .set = print_debug_set,
+ .get = param_get_int,
+};
+
+module_param_cb(print_debug, &print_debug_ops, &print_debug, 0200);
+MODULE_PARM_DESC(print_debug, "print klp-convert debugging info");
+
+
+KLP_MODULE_RELOC(test_klp_convert_mod) test_klp_convert_mod_relocs_a[] = {
+ KLP_SYMPOS(get_homonym_string, 1),
+};
+
+static struct klp_func funcs[] = {
+ {
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ },
+ {
+ .name = "test_klp_convert_mod",
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int test_klp_convert_sections_init(void)
+{
+ int ret;
+
+ ret = klp_enable_patch(&patch);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void test_klp_convert_sections_exit(void)
+{
+}
+
+module_init(test_klp_convert_sections_init);
+module_exit(test_klp_convert_sections_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: klp-convert-sections");
+MODULE_INFO(livepatch, "Y");
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index 25c21ddf30f0..bcb8b468b80a 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -293,4 +293,47 @@ livepatch: '$MOD_KLP_CONVERT2': unpatching complete
% rmmod $MOD_KLP_CONVERT2
% rmmod $MOD_KLP_CONVERT_MOD"
+
+# TEST: klp-convert symbols across ELF sections
+# - load a livepatch that references symbols that require klp-convert
+# and reference the same symbol from multiple ELF sections
+# - verify correct behavior
+# - unload the livepatch
+
+start_test "klp-convert symbols across ELF sections"
+
+saved_cmdline=$(cat /proc/cmdline)
+
+load_mod $MOD_KLP_CONVERT_MOD
+load_lp $MOD_KLP_CONVERT_SECTIONS
+echo 1 > /sys/module/$MOD_KLP_CONVERT_SECTIONS/parameters/print_debug
+disable_lp $MOD_KLP_CONVERT_SECTIONS
+unload_lp $MOD_KLP_CONVERT_SECTIONS
+unload_mod $MOD_KLP_CONVERT_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT_MOD
+% modprobe $MOD_KLP_CONVERT_SECTIONS
+livepatch: enabling patch '$MOD_KLP_CONVERT_SECTIONS'
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': starting patching transition
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': completing patching transition
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': patching complete
+$MOD_KLP_CONVERT_SECTIONS: saved_command_line (1): $saved_cmdline
+$MOD_KLP_CONVERT_SECTIONS: saved_command_line (1): $saved_cmdline
+$MOD_KLP_CONVERT_SECTIONS: saved_command_line (2): $saved_cmdline
+$MOD_KLP_CONVERT_SECTIONS: saved_command_line (1): $saved_cmdline
+$MOD_KLP_CONVERT_SECTIONS: saved_command_line (2): $saved_cmdline
+$MOD_KLP_CONVERT_SECTIONS: saved_command_line (3): $saved_cmdline
+$MOD_KLP_CONVERT_SECTIONS: test_klp_get_driver_name(): $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT_SECTIONS: p_test_klp_get_driver_name(): $MOD_KLP_CONVERT_MOD
+$MOD_KLP_CONVERT_SECTIONS: get_homonym_string(): homonym string A
+$MOD_KLP_CONVERT_SECTIONS: p_get_homonym_string(): homonym string A
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT_SECTIONS/enabled
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_SECTIONS': unpatching complete
+% rmmod $MOD_KLP_CONVERT_SECTIONS
+% rmmod $MOD_KLP_CONVERT_MOD"
+
exit 0
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v7 08/10] livepatch/selftests: add __asm__ symbol renaming examples
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (6 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 07/10] livepatch/selftests: test multiple sections Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-06 14:08 ` [PATCH v7 09/10] livepatch/selftests: add data relocations test Joe Lawrence
` (3 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
GCC can rename symbols like static data and optimized functions, adding
a suffix that includes illegal C characters. Extend the klp-convert
examples to demonstrate how to use __asm__ renaming from C code to
create klp-relocations to such renamed symbols.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
lib/livepatch/test_klp_convert.h | 2 ++
lib/livepatch/test_klp_convert1.c | 8 ++++++++
lib/livepatch/test_klp_convert_mod_a.c | 6 ++++++
lib/livepatch/test_klp_convert_mod_b.c | 6 ++++++
tools/testing/selftests/livepatch/test-livepatch.sh | 4 ++++
5 files changed, 26 insertions(+)
diff --git a/lib/livepatch/test_klp_convert.h b/lib/livepatch/test_klp_convert.h
index 5d97bc546d6e..42befbfd63cb 100644
--- a/lib/livepatch/test_klp_convert.h
+++ b/lib/livepatch/test_klp_convert.h
@@ -10,5 +10,7 @@ extern char driver_name[];
extern char homonym_string[];
extern const char *get_homonym_string(void);
extern const char *test_klp_get_driver_name(void);
+extern char klp_string_a[] __asm__("klp_string.12345");
+extern char klp_string_b[] __asm__("klp_string.67890");
#endif
diff --git a/lib/livepatch/test_klp_convert1.c b/lib/livepatch/test_klp_convert1.c
index d4e1163c01cc..9541f40d70a8 100644
--- a/lib/livepatch/test_klp_convert1.c
+++ b/lib/livepatch/test_klp_convert1.c
@@ -25,6 +25,12 @@ static noinline void print_homonym_string(void)
pr_info("get_homonym_string(), 1: %s\n", get_homonym_string());
}
+static noinline void print_static_strings(void)
+{
+ pr_info("klp_string.12345 = %s\n", klp_string_a);
+ pr_info("klp_string.67890 = %s\n", klp_string_b);
+}
+
/* provide a sysfs handle to invoke debug functions */
static int print_debug;
static int print_debug_set(const char *val, const struct kernel_param *kp)
@@ -32,6 +38,7 @@ static int print_debug_set(const char *val, const struct kernel_param *kp)
print_saved_command_line();
print_driver_name();
print_homonym_string();
+ print_static_strings();
return 0;
}
@@ -67,6 +74,7 @@ KLP_MODULE_RELOC(test_klp_convert_mod) test_klp_convert_mod_relocs_a[] = {
KLP_SYMPOS(homonym_string, 1),
KLP_SYMPOS(get_homonym_string, 1),
KLP_SYMPOS(test_klp_get_driver_name, 0),
+ KLP_SYMPOS(klp_string_b, 1),
};
static struct klp_func funcs[] = {
diff --git a/lib/livepatch/test_klp_convert_mod_a.c b/lib/livepatch/test_klp_convert_mod_a.c
index ae5e911fbb9b..9af0fcab0c8d 100644
--- a/lib/livepatch/test_klp_convert_mod_a.c
+++ b/lib/livepatch/test_klp_convert_mod_a.c
@@ -20,6 +20,12 @@ __used static const char *get_homonym_string(void)
return homonym_string;
}
+__used static void static_string_function(void)
+{
+ __used static char klp_string[] __asm__("klp_string.12345") =
+ __FILE__ " static string";
+}
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
MODULE_DESCRIPTION("Livepatch test: klp-convert module");
diff --git a/lib/livepatch/test_klp_convert_mod_b.c b/lib/livepatch/test_klp_convert_mod_b.c
index 5eca8a4cae38..0a68e898fe03 100644
--- a/lib/livepatch/test_klp_convert_mod_b.c
+++ b/lib/livepatch/test_klp_convert_mod_b.c
@@ -11,3 +11,9 @@ __used static const char *get_homonym_string(void)
{
return homonym_string;
}
+
+__used static void static_string_function(void)
+{
+ __used static char klp_string[] __asm__("klp_string.67890") =
+ __FILE__ " static string";
+}
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index bcb8b468b80a..ec3b6c919b01 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -200,6 +200,8 @@ $MOD_KLP_CONVERT1: driver_name, 0: $MOD_KLP_CONVERT_MOD
$MOD_KLP_CONVERT1: test_klp_get_driver_name(), 0: $MOD_KLP_CONVERT_MOD
$MOD_KLP_CONVERT1: homonym_string, 1: homonym string A
$MOD_KLP_CONVERT1: get_homonym_string(), 1: homonym string A
+test_klp_convert1: klp_string.12345 = lib/livepatch/test_klp_convert_mod_a.c static string
+test_klp_convert1: klp_string.67890 = lib/livepatch/test_klp_convert_mod_b.c static string
% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT1/enabled
livepatch: '$MOD_KLP_CONVERT1': initializing unpatching transition
livepatch: '$MOD_KLP_CONVERT1': starting unpatching transition
@@ -265,6 +267,8 @@ $MOD_KLP_CONVERT1: driver_name, 0: $MOD_KLP_CONVERT_MOD
$MOD_KLP_CONVERT1: test_klp_get_driver_name(), 0: $MOD_KLP_CONVERT_MOD
$MOD_KLP_CONVERT1: homonym_string, 1: homonym string A
$MOD_KLP_CONVERT1: get_homonym_string(), 1: homonym string A
+test_klp_convert1: klp_string.12345 = lib/livepatch/test_klp_convert_mod_a.c static string
+test_klp_convert1: klp_string.67890 = lib/livepatch/test_klp_convert_mod_b.c static string
% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT1/enabled
livepatch: '$MOD_KLP_CONVERT1': initializing unpatching transition
livepatch: '$MOD_KLP_CONVERT1': starting unpatching transition
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v7 09/10] livepatch/selftests: add data relocations test
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (7 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 08/10] livepatch/selftests: add __asm__ symbol renaming examples Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-06 14:08 ` [PATCH v7 10/10] livepatch/selftests: add static keys test Joe Lawrence
` (2 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
Add a test which creates a variety of klp-data-relocations. Note that
this test case exercises late-klp-relocations for references to data.
For this, we only need to adjust pointer storage classes (and not the
target data) to affect where the relocation ends up.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
lib/livepatch/Makefile | 4 +-
lib/livepatch/test_klp_convert.h | 21 ++
lib/livepatch/test_klp_convert_data.c | 190 ++++++++++++++++++
lib/livepatch/test_klp_convert_mod_c.c | 36 ++++
.../selftests/livepatch/test-livepatch.sh | 95 +++++++++
5 files changed, 345 insertions(+), 1 deletion(-)
create mode 100644 lib/livepatch/test_klp_convert_data.c
create mode 100644 lib/livepatch/test_klp_convert_mod_c.c
diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
index 77e21317d4da..da39aaa5c8fc 100644
--- a/lib/livepatch/Makefile
+++ b/lib/livepatch/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_callbacks_mod.o \
test_klp_convert1.o \
test_klp_convert2.o \
+ test_klp_convert_data.o \
test_klp_convert_sections.o \
test_klp_convert_mod.o \
test_klp_livepatch.o \
@@ -19,4 +20,5 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_convert_mod-y := \
test_klp_convert_mod_a.o \
- test_klp_convert_mod_b.o
+ test_klp_convert_mod_b.o \
+ test_klp_convert_mod_c.o
diff --git a/lib/livepatch/test_klp_convert.h b/lib/livepatch/test_klp_convert.h
index 42befbfd63cb..08c0f4b1dc6b 100644
--- a/lib/livepatch/test_klp_convert.h
+++ b/lib/livepatch/test_klp_convert.h
@@ -12,5 +12,26 @@ extern const char *get_homonym_string(void);
extern const char *test_klp_get_driver_name(void);
extern char klp_string_a[] __asm__("klp_string.12345");
extern char klp_string_b[] __asm__("klp_string.67890");
+extern int global_small;
+// .rela.data.rel.ro, .rela.rodata supported ???:
+extern int const_global_small;
+extern int static_small;
+extern int static_const_small;
+extern int global_large[];
+// .rela.data.rel.ro, .rela.rodata supported ???:
+extern int const_global_large[];
+extern int static_large[];
+extern int static_const_large[];
+extern int local_small;
+extern int const_local_small;
+extern int static_local_small;
+extern int static_const_local_small;
+extern int local_large[4];
+extern int const_local_large[4];
+extern int static_local_large[4];
+extern int static_const_local_large[4];
+// .rela.data..ro_after_init supported ???:
+extern int static_ro_after_init;
+extern int static_read_mostly;
#endif
diff --git a/lib/livepatch/test_klp_convert_data.c b/lib/livepatch/test_klp_convert_data.c
new file mode 100644
index 000000000000..da94f7af272f
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_data.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/cache.h>
+#include <linux/livepatch.h>
+#include "test_klp_convert.h"
+
+/* Small global */
+int *p_global_small = &global_small;
+
+/* Small global (const) */
+// .rela.data.rel.ro, .rela.rodata supported ???:
+int * const p_const_global_small = &const_global_small;
+
+/* Small file-static */
+static int *p_static_small = &static_small;
+
+/* Small file-static (constant) */
+static int * const p_static_const_small = &static_const_small;
+
+/* Large global */
+int *p_global_large[4] = {
+ &global_large[0], &global_large[1],
+ &global_large[2], &global_large[3],
+};
+
+/* Large global (const) */
+// .rela.data.rel.ro, .rela.rodata supported ???:
+int * const p_const_global_large[4] = {
+ &const_global_large[0], &const_global_large[1],
+ &const_global_large[2], &const_global_large[3],
+};
+
+/* Large file-static global */
+static int *p_static_large[4] = {
+ &static_large[0], &static_large[1],
+ &static_large[2], &static_large[3],
+};
+
+/* Large file-static (const) */
+static int * const p_static_const_large[4] = {
+ &static_const_large[0], &static_const_large[1],
+ &static_const_large[2], &static_const_large[3],
+};
+
+// .rela.data.rel.ro, .rela.rodata supported ???:
+// static int * __ro_after_init p_static_ro_after_init = &static_ro_after_init;
+static int * __read_mostly p_static_read_mostly = &static_read_mostly;
+
+static void print_variables(void)
+{
+ /* Small local */
+ int *p_local_small = &local_small;
+
+ /* Small local (const) */
+ int * const p_const_local_small = &const_local_small;
+
+ /* Small static-local */
+ static int *p_static_local_small = &static_local_small;
+
+ /* Small static-local (const) */
+ static int * const p_static_const_local_small = &static_const_local_small;
+
+ /* Large local */
+ int *p_local_large[4] = {
+ &local_large[0], &local_large[1],
+ &local_large[2], &local_large[3],
+ };
+
+ /* Large local (const) */
+ int * const p_const_local_large[4] = {
+ &const_local_large[0], &const_local_large[1],
+ &const_local_large[2], &const_local_large[3],
+ };
+
+ /* Large static-local local */
+ static int *p_static_local_large[4] = {
+ &static_local_large[0], &static_local_large[1],
+ &static_local_large[2], &static_local_large[3],
+ };
+
+ /* Large static-local (const) */
+ static int * const p_static_const_local_large[4] = {
+ &static_const_local_large[0], &static_const_local_large[1],
+ &static_const_local_large[2], &static_const_local_large[3],
+ };
+
+ pr_info("local_small: %x\n", *p_local_small);
+ pr_info("const_local_small: %x\n", *p_const_local_small);
+ pr_info("static_local_small: %x\n", *p_static_local_small);
+ pr_info("static_const_local_small: %x\n", *p_static_const_local_small);
+ pr_info("local_large[0..3]: %x %x %x %x\n",
+ *p_local_large[0], *p_local_large[1],
+ *p_local_large[2], *p_local_large[3]);
+ pr_info("const_local_large[0..3]: %x %x %x %x\n",
+ *p_const_local_large[0], *p_const_local_large[1],
+ *p_const_local_large[2], *p_const_local_large[3]);
+ pr_info("static_local_large[0..3]: %x %x %x %x\n",
+ *p_static_local_large[0], *p_static_local_large[1],
+ *p_static_local_large[2], *p_static_local_large[3]);
+ pr_info("static_const_local_large[0..3]: %x %x %x %x\n",
+ *p_static_const_local_large[0], *p_static_const_local_large[1],
+ *p_static_const_local_large[2], *p_static_const_local_large[3]);
+
+ pr_info("global_small: %x\n", *p_global_small);
+ // .rela.data.rel.ro, .rela.rodata supported ???:
+ pr_info("const_global_small: %x\n", *p_const_global_small);
+ pr_info("static_small: %x\n", *p_static_small);
+ pr_info("static_const_small: %x\n", *p_static_const_small);
+ pr_info("global_large[0..3]: %x %x %x %x\n",
+ *p_global_large[0], *p_global_large[1],
+ *p_global_large[2], *p_global_large[3]);
+ // .rela.data.rel.ro, .rela.rodata supported ???:
+ pr_info("const_global_large[0..3]: %x %x %x %x\n",
+ *p_const_global_large[0], *p_const_global_large[1],
+ *p_const_global_large[2], *p_const_global_large[3]);
+ pr_info("static_large[0..3]: %x %x %x %x\n",
+ *p_static_large[0], *p_static_large[1],
+ *p_static_large[2], *p_static_large[3]);
+ pr_info("static_const_large[0..3]: %x %x %x %x\n",
+ *p_static_const_large[0], *p_static_const_large[1],
+ *p_static_const_large[2], *p_static_const_large[3]);
+
+ // .rela.data..ro_after_init supported ???:
+ // pr_info("static_ro_after_init: %x\n", *p_static_ro_after_init);
+ pr_info("static_read_mostly: %x\n", *p_static_read_mostly);
+}
+
+/* provide a sysfs handle to invoke debug functions */
+static int print_debug;
+static int print_debug_set(const char *val, const struct kernel_param *kp)
+{
+ print_variables();
+
+ return 0;
+}
+static const struct kernel_param_ops print_debug_ops = {
+ .set = print_debug_set,
+ .get = param_get_int,
+};
+module_param_cb(print_debug, &print_debug_ops, &print_debug, 0200);
+MODULE_PARM_DESC(print_debug, "print klp-convert debugging info");
+
+
+static struct klp_func funcs[] = {
+ {
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ },
+ {
+ .name = "test_klp_convert_mod",
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int test_klp_convert_init(void)
+{
+ int ret;
+
+ ret = klp_enable_patch(&patch);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void test_klp_convert_exit(void)
+{
+}
+
+module_init(test_klp_convert_init);
+module_exit(test_klp_convert_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: klp-convert-data");
+MODULE_INFO(livepatch, "Y");
diff --git a/lib/livepatch/test_klp_convert_mod_c.c b/lib/livepatch/test_klp_convert_mod_c.c
new file mode 100644
index 000000000000..8f94a313e651
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_mod_c.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021 Joe Lawrence <joe.lawrence@redhat.com>
+
+#include <linux/types.h>
+
+/*
+ * test_klp_convert_data will create klp-relocations to these variables.
+ * They are named for the storage class of the variable that refers to
+ * the. This makes it easier to correlate those symbols to the
+ * relocations that refer to them in readelf output.
+ */
+__used static int global_small = 0x1111;
+// .rela.data.rel.ro, .rela.rodata supported ???:
+__used static int const_global_small = 0x2222;
+__used static int static_small = 0x3333;
+__used static int static_const_small = 0x4444;
+
+__used static int global_large[4] = { 0x1111, 0x2222, 0x3333, 0x4444 };
+// .rela.data.rel.ro, .rela.rodata supported ???:
+__used static int const_global_large[4] = { 0x5555, 0x6666, 0x7777, 0x8888 };
+__used static int static_large[4] = { 0x9999, 0xaaaa, 0xbbbb, 0xcccc };
+__used static int static_const_large[4] = { 0xdddd, 0xeeee, 0xffff, 0x0000 };
+
+__used static int local_small = 0x1111;
+__used static int const_local_small = 0x2222;
+__used static int static_local_small = 0x3333;
+__used static int static_const_local_small = 0x4444;
+
+__used static int local_large[4] = { 0x1111, 0x2222, 0x3333, 0x4444 };
+__used static int const_local_large[4] = { 0x5555, 0x6666, 0x7777, 0x8888 };
+__used static int static_local_large[4] = { 0x9999, 0xaaaa, 0xbbbb, 0xcccc };
+__used static int static_const_local_large[4] = { 0xdddd, 0xeeee, 0xffff, 0x0000 };
+
+// .rela.data..ro_after_init supported ???:
+// __used static int static_ro_after_init = 0x1111;
+__used static int static_read_mostly = 0x2222;
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index ec3b6c919b01..5bda36b65bb5 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -9,6 +9,7 @@ MOD_REPLACE=test_klp_atomic_replace
MOD_KLP_CONVERT_MOD=test_klp_convert_mod
MOD_KLP_CONVERT1=test_klp_convert1
MOD_KLP_CONVERT2=test_klp_convert2
+MOD_KLP_CONVERT_DATA=test_klp_convert_data
MOD_KLP_CONVERT_SECTIONS=test_klp_convert_sections
setup_config
@@ -340,4 +341,98 @@ livepatch: '$MOD_KLP_CONVERT_SECTIONS': unpatching complete
% rmmod $MOD_KLP_CONVERT_SECTIONS
% rmmod $MOD_KLP_CONVERT_MOD"
+
+# TEST: klp-convert data relocations
+
+start_test "klp-convert data relocations"
+
+load_mod $MOD_KLP_CONVERT_MOD
+load_lp $MOD_KLP_CONVERT_DATA
+
+echo 1 > /sys/module/$MOD_KLP_CONVERT_DATA/parameters/print_debug
+
+disable_lp $MOD_KLP_CONVERT_DATA
+unload_lp $MOD_KLP_CONVERT_DATA
+unload_mod $MOD_KLP_CONVERT_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT_MOD
+% modprobe $MOD_KLP_CONVERT_DATA
+livepatch: enabling patch '$MOD_KLP_CONVERT_DATA'
+livepatch: '$MOD_KLP_CONVERT_DATA': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': starting patching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': completing patching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': patching complete
+$MOD_KLP_CONVERT_DATA: local_small: 1111
+$MOD_KLP_CONVERT_DATA: const_local_small: 2222
+$MOD_KLP_CONVERT_DATA: static_local_small: 3333
+$MOD_KLP_CONVERT_DATA: static_const_local_small: 4444
+$MOD_KLP_CONVERT_DATA: local_large[0..3]: 1111 2222 3333 4444
+$MOD_KLP_CONVERT_DATA: const_local_large[0..3]: 5555 6666 7777 8888
+$MOD_KLP_CONVERT_DATA: static_local_large[0..3]: 9999 aaaa bbbb cccc
+$MOD_KLP_CONVERT_DATA: static_const_local_large[0..3]: dddd eeee ffff 0
+$MOD_KLP_CONVERT_DATA: global_small: 1111
+$MOD_KLP_CONVERT_DATA: const_global_small: 2222
+$MOD_KLP_CONVERT_DATA: static_small: 3333
+$MOD_KLP_CONVERT_DATA: static_const_small: 4444
+$MOD_KLP_CONVERT_DATA: global_large[0..3]: 1111 2222 3333 4444
+$MOD_KLP_CONVERT_DATA: const_global_large[0..3]: 5555 6666 7777 8888
+$MOD_KLP_CONVERT_DATA: static_large[0..3]: 9999 aaaa bbbb cccc
+$MOD_KLP_CONVERT_DATA: static_const_large[0..3]: dddd eeee ffff 0
+$MOD_KLP_CONVERT_DATA: static_read_mostly: 2222
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT_DATA/enabled
+livepatch: '$MOD_KLP_CONVERT_DATA': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': unpatching complete
+% rmmod $MOD_KLP_CONVERT_DATA
+% rmmod $MOD_KLP_CONVERT_MOD"
+
+
+# TEST: klp-convert data relocations (late module patching)
+
+start_test "klp-convert data relocations (late module patching)"
+
+load_lp $MOD_KLP_CONVERT_DATA
+load_mod $MOD_KLP_CONVERT_MOD
+
+echo 1 > /sys/module/$MOD_KLP_CONVERT_DATA/parameters/print_debug
+
+disable_lp $MOD_KLP_CONVERT_DATA
+unload_lp $MOD_KLP_CONVERT_DATA
+unload_mod $MOD_KLP_CONVERT_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT_DATA
+livepatch: enabling patch '$MOD_KLP_CONVERT_DATA'
+livepatch: '$MOD_KLP_CONVERT_DATA': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': starting patching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': completing patching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': patching complete
+% modprobe $MOD_KLP_CONVERT_MOD
+livepatch: applying patch '$MOD_KLP_CONVERT_DATA' to loading module '$MOD_KLP_CONVERT_MOD'
+$MOD_KLP_CONVERT_DATA: local_small: 1111
+$MOD_KLP_CONVERT_DATA: const_local_small: 2222
+$MOD_KLP_CONVERT_DATA: static_local_small: 3333
+$MOD_KLP_CONVERT_DATA: static_const_local_small: 4444
+$MOD_KLP_CONVERT_DATA: local_large[0..3]: 1111 2222 3333 4444
+$MOD_KLP_CONVERT_DATA: const_local_large[0..3]: 5555 6666 7777 8888
+$MOD_KLP_CONVERT_DATA: static_local_large[0..3]: 9999 aaaa bbbb cccc
+$MOD_KLP_CONVERT_DATA: static_const_local_large[0..3]: dddd eeee ffff 0
+$MOD_KLP_CONVERT_DATA: global_small: 1111
+$MOD_KLP_CONVERT_DATA: const_global_small: 2222
+$MOD_KLP_CONVERT_DATA: static_small: 3333
+$MOD_KLP_CONVERT_DATA: static_const_small: 4444
+$MOD_KLP_CONVERT_DATA: global_large[0..3]: 1111 2222 3333 4444
+$MOD_KLP_CONVERT_DATA: const_global_large[0..3]: 5555 6666 7777 8888
+$MOD_KLP_CONVERT_DATA: static_large[0..3]: 9999 aaaa bbbb cccc
+$MOD_KLP_CONVERT_DATA: static_const_large[0..3]: dddd eeee ffff 0
+$MOD_KLP_CONVERT_DATA: static_read_mostly: 2222
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT_DATA/enabled
+livepatch: '$MOD_KLP_CONVERT_DATA': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_DATA': unpatching complete
+% rmmod $MOD_KLP_CONVERT_DATA
+% rmmod $MOD_KLP_CONVERT_MOD"
+
+
exit 0
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v7 10/10] livepatch/selftests: add static keys test
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (8 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 09/10] livepatch/selftests: add data relocations test Joe Lawrence
@ 2023-03-06 14:08 ` Joe Lawrence
2023-03-14 20:23 ` [PATCH v7 00/10] livepatch: klp-convert tool Marcos Paulo de Souza
2023-04-19 20:27 ` Marcos Paulo de Souza
11 siblings, 0 replies; 29+ messages in thread
From: Joe Lawrence @ 2023-03-06 14:08 UTC (permalink / raw)
To: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
Add a livepatch kselftest that exercises klp-convert support for static
keys:
- Use static_branch_(un)likely() on vmlinux-defined keys, forcing
.rela__jump_table klp-relocations for them.
- Use only static_key_enable() on module-defined keys, creating .text
klp-relocations for them.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
lib/livepatch/Makefile | 2 +
lib/livepatch/test_klp_convert.h | 8 ++
lib/livepatch/test_klp_convert_keys.c | 91 +++++++++++++
lib/livepatch/test_klp_convert_keys_mod.c | 52 +++++++
.../selftests/livepatch/test-livepatch.sh | 127 ++++++++++++++++++
5 files changed, 280 insertions(+)
create mode 100644 lib/livepatch/test_klp_convert_keys.c
create mode 100644 lib/livepatch/test_klp_convert_keys_mod.c
diff --git a/lib/livepatch/Makefile b/lib/livepatch/Makefile
index da39aaa5c8fc..a3c2ac61387f 100644
--- a/lib/livepatch/Makefile
+++ b/lib/livepatch/Makefile
@@ -11,6 +11,8 @@ obj-$(CONFIG_TEST_LIVEPATCH) += test_klp_atomic_replace.o \
test_klp_convert2.o \
test_klp_convert_data.o \
test_klp_convert_sections.o \
+ test_klp_convert_keys.o \
+ test_klp_convert_keys_mod.o \
test_klp_convert_mod.o \
test_klp_livepatch.o \
test_klp_shadow_vars.o \
diff --git a/lib/livepatch/test_klp_convert.h b/lib/livepatch/test_klp_convert.h
index 08c0f4b1dc6b..97d4c26e4c39 100644
--- a/lib/livepatch/test_klp_convert.h
+++ b/lib/livepatch/test_klp_convert.h
@@ -34,4 +34,12 @@ extern int static_const_local_large[4];
extern int static_ro_after_init;
extern int static_read_mostly;
+/* klp-convert symbols - vmlinux */
+extern struct static_key_false tracepoint_printk_key;
+
+/* klp-convert symbols - test_klp_keys_mod.ko */
+extern struct static_key_true test_klp_true_key;
+extern struct static_key_false test_klp_false_key;
+
+
#endif
diff --git a/lib/livepatch/test_klp_convert_keys.c b/lib/livepatch/test_klp_convert_keys.c
new file mode 100644
index 000000000000..90c20e84a146
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_keys.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include <linux/jump_label.h>
+#include "test_klp_convert.h"
+
+/*
+ * Carry our own copy of print_key_status() as we want static key code
+ * patching updates to occur in the livepatch module as well as the
+ * target module that defines the static keys.
+ */
+static void print_key_status(char *msg)
+{
+ pr_info("%s: %s\n", __func__, msg);
+
+ /* static_key_enable() only tests the key value */
+ pr_info("static_key_enabled(&tracepoint_printk_key) is %s\n",
+ static_key_enabled(&tracepoint_printk_key) ? "true" : "false");
+ pr_info("static_key_enabled(&test_klp_true_key) is %s\n",
+ static_key_enabled(&test_klp_true_key) ? "true" : "false");
+ pr_info("static_key_enabled(&test_klp_false_key) is %s\n",
+ static_key_enabled(&test_klp_false_key) ? "true" : "false");
+
+ /*
+ * static_branch_(un)likely() requires code patching when the
+ * key value changes
+ */
+ pr_info("static_branch_unlikely(&tracepoint_printk_key) is %s\n",
+ static_branch_unlikely(&tracepoint_printk_key) ? "true" : "false");
+}
+
+/*
+ * sysfs interface to poke the key
+ */
+static bool enable_false_key;
+static int set_enable_false_key(const char *val, const struct kernel_param *kp)
+{
+ print_key_status("set_enable_false_key start");
+ static_branch_enable(&test_klp_false_key);
+ print_key_status("set_enable_false_key enabling test_klp_false_key");
+
+ return 0;
+}
+module_param_call(enable_false_key, set_enable_false_key, NULL,
+ &enable_false_key, 0644);
+MODULE_PARM_DESC(enable_false_key, "Static branch enable");
+
+
+static struct klp_func funcs[] = {
+ { }
+};
+
+static struct klp_object objs[] = {
+ {
+ .name = "test_klp_convert_keys_mod",
+ .funcs = funcs,
+ }, {}
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int test_klp_convert_keys_init(void)
+{
+ int ret;
+
+ ret = klp_enable_patch(&patch);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void test_klp_convert_keys_exit(void)
+{
+}
+
+module_init(test_klp_convert_keys_init);
+module_exit(test_klp_convert_keys_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: static keys");
+MODULE_INFO(livepatch, "Y");
diff --git a/lib/livepatch/test_klp_convert_keys_mod.c b/lib/livepatch/test_klp_convert_keys_mod.c
new file mode 100644
index 000000000000..7b11c2da09c9
--- /dev/null
+++ b/lib/livepatch/test_klp_convert_keys_mod.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Joe Lawrence <joe.lawrence@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/jump_label.h>
+
+static DEFINE_STATIC_KEY_TRUE(test_klp_true_key);
+static DEFINE_STATIC_KEY_FALSE(test_klp_false_key);
+
+static void print_key_status(char *msg)
+{
+ pr_info("%s: %s\n", __func__, msg);
+
+ /* static_key_enable() only tests the key value */
+ pr_info("static_key_enabled(&test_klp_true_key) is %s\n",
+ static_key_enabled(&test_klp_true_key) ? "true" : "false");
+ pr_info("static_key_enabled(&test_klp_false_key) is %s\n",
+ static_key_enabled(&test_klp_false_key) ? "true" : "false");
+
+ /*
+ * static_branch_(un)likely() requires code patching when the
+ * key value changes
+ */
+ pr_info("static_branch_likely(&test_klp_true_key) is %s\n",
+ static_branch_likely(&test_klp_true_key) ? "true" : "false");
+ pr_info("static_branch_unlikely(&test_klp_false_key) is %s\n",
+ static_branch_unlikely(&test_klp_false_key) ? "true" : "false");
+}
+
+static int test_klp_keys_mod_init(void)
+{
+ print_key_status("initial conditions");
+ static_branch_disable(&test_klp_true_key);
+ print_key_status("disabled test_klp_true_key");
+
+ return 0;
+}
+
+static void test_klp_keys_mod_exit(void)
+{
+ print_key_status("unloading conditions");
+}
+
+module_init(test_klp_keys_mod_init);
+module_exit(test_klp_keys_mod_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joe Lawrence <joe.lawrence@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: static keys target module");
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index 5bda36b65bb5..8ad284a57770 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -11,6 +11,8 @@ MOD_KLP_CONVERT1=test_klp_convert1
MOD_KLP_CONVERT2=test_klp_convert2
MOD_KLP_CONVERT_DATA=test_klp_convert_data
MOD_KLP_CONVERT_SECTIONS=test_klp_convert_sections
+MOD_KLP_CONVERT_KEYS_MOD=test_klp_convert_keys_mod
+MOD_KLP_CONVERT_KEYS=test_klp_convert_keys
setup_config
@@ -435,4 +437,129 @@ livepatch: '$MOD_KLP_CONVERT_DATA': unpatching complete
% rmmod $MOD_KLP_CONVERT_MOD"
+# TEST: klp-convert static keys
+# - load a module which defines static keys, updates one of the keys on
+# load (forcing jump table patching)
+# - load a livepatch that references the same keys, resolved by
+# klp-convert tool
+# - poke the livepatch sysfs interface to update one of the key (forcing
+# jump table patching again)
+# - disable and unload the livepatch
+# - remove the module
+
+start_test "klp-convert static keys"
+
+load_mod $MOD_KLP_CONVERT_KEYS_MOD
+load_lp $MOD_KLP_CONVERT_KEYS
+
+echo 1 > /sys/module/$MOD_KLP_CONVERT_KEYS/parameters/enable_false_key
+
+disable_lp $MOD_KLP_CONVERT_KEYS
+unload_lp $MOD_KLP_CONVERT_KEYS
+unload_mod $MOD_KLP_CONVERT_KEYS_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT_KEYS_MOD
+$MOD_KLP_CONVERT_KEYS_MOD: print_key_status: initial conditions
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_true_key) is true
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_likely(&test_klp_true_key) is true
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_unlikely(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: print_key_status: disabled test_klp_true_key
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_likely(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_unlikely(&test_klp_false_key) is false
+% modprobe $MOD_KLP_CONVERT_KEYS
+livepatch: enabling patch '$MOD_KLP_CONVERT_KEYS'
+livepatch: '$MOD_KLP_CONVERT_KEYS': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': starting patching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': completing patching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': patching complete
+$MOD_KLP_CONVERT_KEYS: print_key_status: set_enable_false_key start
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&tracepoint_printk_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS: static_branch_unlikely(&tracepoint_printk_key) is false
+$MOD_KLP_CONVERT_KEYS: print_key_status: set_enable_false_key enabling test_klp_false_key
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&tracepoint_printk_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_false_key) is true
+$MOD_KLP_CONVERT_KEYS: static_branch_unlikely(&tracepoint_printk_key) is false
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT_KEYS/enabled
+livepatch: '$MOD_KLP_CONVERT_KEYS': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': unpatching complete
+% rmmod $MOD_KLP_CONVERT_KEYS
+% rmmod $MOD_KLP_CONVERT_KEYS_MOD
+$MOD_KLP_CONVERT_KEYS_MOD: print_key_status: unloading conditions
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_false_key) is true
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_likely(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_unlikely(&test_klp_false_key) is true"
+
+
+# TEST: klp-convert static keys (late module patching)
+# - load a module which defines static keys, updates one of the keys on
+# load (forcing jump table patching)
+# - load a livepatch that references the same keys, resolved by
+# klp-convert tool
+# - poke the livepatch sysfs interface to update one of the key (forcing
+# jump table patching again)
+# - disable and unload the livepatch
+# - remove the module
+
+start_test "klp-convert static keys (late module patching)"
+
+load_lp $MOD_KLP_CONVERT_KEYS
+load_mod $MOD_KLP_CONVERT_KEYS_MOD
+
+echo 1 > /sys/module/$MOD_KLP_CONVERT_KEYS/parameters/enable_false_key
+
+disable_lp $MOD_KLP_CONVERT_KEYS
+unload_lp $MOD_KLP_CONVERT_KEYS
+unload_mod $MOD_KLP_CONVERT_KEYS_MOD
+
+check_result "% modprobe $MOD_KLP_CONVERT_KEYS
+livepatch: enabling patch '$MOD_KLP_CONVERT_KEYS'
+livepatch: '$MOD_KLP_CONVERT_KEYS': initializing patching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': starting patching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': completing patching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': patching complete
+% modprobe $MOD_KLP_CONVERT_KEYS_MOD
+livepatch: applying patch '$MOD_KLP_CONVERT_KEYS' to loading module '$MOD_KLP_CONVERT_KEYS_MOD'
+$MOD_KLP_CONVERT_KEYS_MOD: print_key_status: initial conditions
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_true_key) is true
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_likely(&test_klp_true_key) is true
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_unlikely(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: print_key_status: disabled test_klp_true_key
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_likely(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_unlikely(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS: print_key_status: set_enable_false_key start
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&tracepoint_printk_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_false_key) is false
+$MOD_KLP_CONVERT_KEYS: static_branch_unlikely(&tracepoint_printk_key) is false
+$MOD_KLP_CONVERT_KEYS: print_key_status: set_enable_false_key enabling test_klp_false_key
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&tracepoint_printk_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS: static_key_enabled(&test_klp_false_key) is true
+$MOD_KLP_CONVERT_KEYS: static_branch_unlikely(&tracepoint_printk_key) is false
+% echo 0 > /sys/kernel/livepatch/$MOD_KLP_CONVERT_KEYS/enabled
+livepatch: '$MOD_KLP_CONVERT_KEYS': initializing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': starting unpatching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': completing unpatching transition
+livepatch: '$MOD_KLP_CONVERT_KEYS': unpatching complete
+% rmmod $MOD_KLP_CONVERT_KEYS
+% rmmod $MOD_KLP_CONVERT_KEYS_MOD
+$MOD_KLP_CONVERT_KEYS_MOD: print_key_status: unloading conditions
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_key_enabled(&test_klp_false_key) is true
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_likely(&test_klp_true_key) is false
+$MOD_KLP_CONVERT_KEYS_MOD: static_branch_unlikely(&test_klp_false_key) is true"
+
+
exit 0
--
2.39.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (9 preceding siblings ...)
2023-03-06 14:08 ` [PATCH v7 10/10] livepatch/selftests: add static keys test Joe Lawrence
@ 2023-03-14 20:23 ` Marcos Paulo de Souza
2023-03-17 20:29 ` Joe Lawrence
2023-04-19 20:27 ` Marcos Paulo de Souza
11 siblings, 1 reply; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-14 20:23 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Mon, Mar 06, 2023 at 09:08:14AM -0500, Joe Lawrence wrote:
> Summary
> -------
>
> 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 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, add support for symbol mapping in the form of a
> symbols.klp 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. For example. see the saved_command_line
> variable in lib/livepatch/test_klp_convert2.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. See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> in lib/livepatch/test_klp_convert{1,2}.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 by
> removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> lib/livepatch/test_klp_convert{1,2}.c.
>
> Selftests have been added to exercise these klp-convert use-cases
> through several tests.
>
>
> Testing
> -------
>
> The patchset selftests build and execute on x86_64, s390x, and ppc64le
> for both default config (with added livepatch dependencies) and a larger
> RHEL-9-ish config.
>
> Using the Intel's Linux Kernel Performance tests's make.cross,
> klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> ppc32 s390 arm64 arches.
>
>
> Summary of changes in v7
> ------------------------
>
> - rebase for v6.2
> - combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
> klp-convert annotation helpers")
> - combine ("kbuild: Support for symbols.klp creation") with ("modpost:
> Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
> - klp-convert: add safe_snprintf() (-Wsign-compare)
> - klp-convert: fix -Wsign-compare warnings
> - klp-convert: use calloc() where appropriate
> - klp-convert: copy ELF e_flags
> - selftests: fix various build warnings
> - klp-convert: WARN msg simplification, failed sanity checks, and sympos
> comment [Marcos]
> - klp-convert: fix elf_write_file() error paths [Petr]
Thanks for the new version Joe. I've run the ksefltests on my x86 laptop, and it
succeed as expected, so
Tested-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>
>
> Previous versions
> -----------------
>
> RFC:
> https://lore.kernel.org/lkml/cover.1477578530.git.jpoimboe@redhat.com/
> v2:
> https://lore.kernel.org/lkml/f52d29f7-7d1b-ad3d-050b-a9fa8878faf2@redhat.com/
> v3:
> https://lore.kernel.org/lkml/20190410155058.9437-1-joe.lawrence@redhat.com/
> v4:
> https://lore.kernel.org/lkml/20190509143859.9050-1-joe.lawrence@redhat.com/
> v5:
> (not posted)
> https://github.com/joe-lawrence/klp-convert-tree/tree/klp-convert-v5-devel
> v6:
> https://lore.kernel.org/live-patching/20220216163940.228309-1-joe.lawrence@redhat.com/
>
>
> Joe Lawrence (10):
> livepatch: Create and include UAPI headers
> livepatch: Add klp-convert tool
> kbuild/modpost: create symbols.klp and integrate klp-convert
> livepatch: Add sample livepatch module
> documentation: Update on livepatch elf format
> livepatch/selftests: add klp-convert
> livepatch/selftests: test multiple sections
> livepatch/selftests: add __asm__ symbol renaming examples
> livepatch/selftests: add data relocations test
> livepatch/selftests: add static keys test
>
> .gitignore | 2 +
> Documentation/dontdiff | 1 +
> Documentation/livepatch/livepatch.rst | 3 +
> Documentation/livepatch/module-elf-format.rst | 42 +-
> MAINTAINERS | 2 +
> Makefile | 16 +-
> include/linux/livepatch.h | 13 +
> include/uapi/linux/livepatch.h | 25 +
> kernel/livepatch/core.c | 4 +-
> lib/livepatch/Makefile | 12 +
> lib/livepatch/test_klp_convert.h | 45 +
> lib/livepatch/test_klp_convert1.c | 121 +++
> lib/livepatch/test_klp_convert2.c | 110 +++
> lib/livepatch/test_klp_convert_data.c | 190 ++++
> lib/livepatch/test_klp_convert_keys.c | 91 ++
> lib/livepatch/test_klp_convert_keys_mod.c | 52 +
> lib/livepatch/test_klp_convert_mod_a.c | 31 +
> lib/livepatch/test_klp_convert_mod_b.c | 19 +
> lib/livepatch/test_klp_convert_mod_c.c | 36 +
> lib/livepatch/test_klp_convert_sections.c | 120 +++
> samples/livepatch/Makefile | 1 +
> .../livepatch/livepatch-annotated-sample.c | 93 ++
> scripts/Makefile | 1 +
> scripts/Makefile.modfinal | 33 +
> scripts/Makefile.modpost | 5 +
> scripts/livepatch/.gitignore | 1 +
> scripts/livepatch/Makefile | 5 +
> scripts/livepatch/elf.c | 817 ++++++++++++++++
> scripts/livepatch/elf.h | 74 ++
> scripts/livepatch/klp-convert.c | 893 ++++++++++++++++++
> scripts/livepatch/klp-convert.h | 47 +
> scripts/livepatch/list.h | 391 ++++++++
> scripts/mod/modpost.c | 28 +-
> scripts/mod/modpost.h | 1 +
> .../selftests/livepatch/test-livepatch.sh | 403 ++++++++
> 35 files changed, 3716 insertions(+), 12 deletions(-)
> create mode 100644 include/uapi/linux/livepatch.h
> create mode 100644 lib/livepatch/test_klp_convert.h
> create mode 100644 lib/livepatch/test_klp_convert1.c
> create mode 100644 lib/livepatch/test_klp_convert2.c
> create mode 100644 lib/livepatch/test_klp_convert_data.c
> create mode 100644 lib/livepatch/test_klp_convert_keys.c
> create mode 100644 lib/livepatch/test_klp_convert_keys_mod.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_a.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_b.c
> create mode 100644 lib/livepatch/test_klp_convert_mod_c.c
> create mode 100644 lib/livepatch/test_klp_convert_sections.c
> 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/klp-convert.h
> create mode 100644 scripts/livepatch/list.h
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-14 20:23 ` [PATCH v7 00/10] livepatch: klp-convert tool Marcos Paulo de Souza
@ 2023-03-17 20:29 ` Joe Lawrence
2023-03-17 23:20 ` Josh Poimboeuf
2023-03-20 20:15 ` Marcos Paulo de Souza
0 siblings, 2 replies; 29+ messages in thread
From: Joe Lawrence @ 2023-03-17 20:29 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Tue, Mar 14, 2023 at 05:23:56PM -0300, Marcos Paulo de Souza wrote:
> On Mon, Mar 06, 2023 at 09:08:14AM -0500, Joe Lawrence wrote:
> > Summary
> > -------
> >
> > 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 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, add support for symbol mapping in the form of a
> > symbols.klp 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. For example. see the saved_command_line
> > variable in lib/livepatch/test_klp_convert2.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. See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> > in lib/livepatch/test_klp_convert{1,2}.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 by
> > removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> > lib/livepatch/test_klp_convert{1,2}.c.
> >
> > Selftests have been added to exercise these klp-convert use-cases
> > through several tests.
> >
> >
> > Testing
> > -------
> >
> > The patchset selftests build and execute on x86_64, s390x, and ppc64le
> > for both default config (with added livepatch dependencies) and a larger
> > RHEL-9-ish config.
> >
> > Using the Intel's Linux Kernel Performance tests's make.cross,
> > klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> > ppc32 s390 arm64 arches.
> >
> >
> > Summary of changes in v7
> > ------------------------
> >
> > - rebase for v6.2
> > - combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
> > klp-convert annotation helpers")
> > - combine ("kbuild: Support for symbols.klp creation") with ("modpost:
> > Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
> > - klp-convert: add safe_snprintf() (-Wsign-compare)
> > - klp-convert: fix -Wsign-compare warnings
> > - klp-convert: use calloc() where appropriate
> > - klp-convert: copy ELF e_flags
> > - selftests: fix various build warnings
> > - klp-convert: WARN msg simplification, failed sanity checks, and sympos
> > comment [Marcos]
> > - klp-convert: fix elf_write_file() error paths [Petr]
>
> Thanks for the new version Joe. I've run the ksefltests on my x86 laptop, and it
> succeed as expected, so
>
> Tested-by: Marcos Paulo de Souza <mpdesouza@suse.com>
>
Thanks for the testing and reviews, Marcos.
The selftests are the first level of testing... we should probably
tackle a real or simulated CVE fix to see how well the tooling fits
larger livepatches.
One complication that I can envision is symbol positioning. Currently,
the klp-convert annotations are a direct mirror of the kernel's
<obj,symbol,pos> tuple. It should be possible to make this a bit more
user friendly for the livepatch developer if the annotations were
<obj,file,symbol>, as derived from the vmlinux / module.tmp.ko symbol
tables.
For example, the following code:
KLP_MODULE_RELOC(test_klp_convert_mod, test_klp_convert_mod_b.c) test_klp_convert_mod_relocs_b[] = {
KLP_SYMPOS(homonym_string),
KLP_SYMPOS(get_homonym_string),
};
could generate the following relocations:
Relocation section '.rela.klp.module_relocs.test_klp_convert_mod.test_klp_convert_mod_b.c' at offset 0x1dc0 contains 2 entries:
Offset Info Type Symbol's Value Symbol's Name + Addend
0000000000000000 0000003f00000001 R_X86_64_64 0000000000000000 homonym_string + 0
0000000000000008 0000004900000001 R_X86_64_64 0000000000000000 get_homonym_string + 0
for which klp-convert looks up in symbols.klp:
klp-convert-symbol-data.0.2
*vmlinux
...
*test_klp_convert_mod
-test_klp_convert_mod_a.c << added filenames to the format
test_klp_get_driver_name
driver_name
get_homonym_string << sympos = 1
homonym_string << sympos = 1
...
-test_klp_convert_mod_b.c
get_homonym_string << sympos = 2
homonym_string << sympos = 2
...
and then generates the usual klp-relocations as currently defined.
(Unfortunately full pathnames are not saved in the STT_FILE symbol table
entries, so there will be a few non-unique <obj,file,symbol> entries. I
believe the last time this was discussed, we found that there were a
relatively small number of such symbols.)
Have you tried retrofitting klp-convert into any real-world livepatch?
I'm curious as to your observations on the overall experience, or
thoughts on the sympos annotation style noted above.
Regards,
-- Joe
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-17 20:29 ` Joe Lawrence
@ 2023-03-17 23:20 ` Josh Poimboeuf
2023-03-20 19:23 ` Joe Lawrence
2023-04-11 10:06 ` Nicolai Stange
2023-03-20 20:15 ` Marcos Paulo de Souza
1 sibling, 2 replies; 29+ messages in thread
From: Josh Poimboeuf @ 2023-03-17 23:20 UTC (permalink / raw)
To: Joe Lawrence
Cc: Marcos Paulo de Souza, live-patching, linux-kernel, linux-kbuild,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> Have you tried retrofitting klp-convert into any real-world livepatch?
> I'm curious as to your observations on the overall experience, or
> thoughts on the sympos annotation style noted above.
On a related note, the patch creation process (of which klp-convert
would be part of) needs to be documented.
If I remember correctly, the proper safe usage of klp-convert requires a
kernel built with -flive-patching, plus some scripting and/or manual
processes.
If nobody knows how to safely use it then there wouldn't be much value
in merging it.
--
Josh
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-17 23:20 ` Josh Poimboeuf
@ 2023-03-20 19:23 ` Joe Lawrence
2023-04-11 10:06 ` Nicolai Stange
1 sibling, 0 replies; 29+ messages in thread
From: Joe Lawrence @ 2023-03-20 19:23 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Marcos Paulo de Souza, live-patching, linux-kernel, linux-kbuild,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On 3/17/23 19:20, Josh Poimboeuf wrote:
> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
>> Have you tried retrofitting klp-convert into any real-world livepatch?
>> I'm curious as to your observations on the overall experience, or
>> thoughts on the sympos annotation style noted above.
>
> On a related note, the patch creation process (of which klp-convert
> would be part of) needs to be documented.
>
> If I remember correctly, the proper safe usage of klp-convert requires a
> kernel built with -flive-patching, plus some scripting and/or manual
> processes.
>
> If nobody knows how to safely use it then there wouldn't be much value
> in merging it.
>
I took a trip though my inbox and dug up a 2020 discussion on
documenting livepatch and compiler considerations [1]. This led to the
suggestion of a greater, end-to-end livepatch author guide, for which
everyone agreed, but has since remained unwritten :/
If we have a miniconf @ LPC this year, maybe we can volunteer to write
sections, decide on an outline, or even better, have a rough draft to
review and discuss?
Aside: technically klp-convert doesn't require -flive-patching, but it's
probably strongly recommended in order to use it safely. And fwiw,
kpatch-build could leverage the tool should it desire one day. In the
meantime, if kpatch-build doesn't need/want to use it, perhaps
klp-convert should have its own CONFIG option? (Or something in modinfo
to key on.)
[1]
https://lore.kernel.org/live-patching/20200721161407.26806-1-joe.lawrence@redhat.com/
--
Joe
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-17 23:20 ` Josh Poimboeuf
2023-03-20 19:23 ` Joe Lawrence
@ 2023-04-11 10:06 ` Nicolai Stange
2023-05-02 23:38 ` Marcos Paulo de Souza
2023-05-03 19:54 ` Joe Lawrence
1 sibling, 2 replies; 29+ messages in thread
From: Nicolai Stange @ 2023-04-11 10:06 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Joe Lawrence, Marcos Paulo de Souza, live-patching, linux-kernel,
linux-kbuild, Miroslav Benes, Petr Mladek, Marcos Paulo de Souza,
Lukas Hruska
Josh Poimboeuf <jpoimboe@kernel.org> writes:
> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
>> Have you tried retrofitting klp-convert into any real-world livepatch?
>> I'm curious as to your observations on the overall experience, or
>> thoughts on the sympos annotation style noted above.
>
> On a related note, the patch creation process (of which klp-convert
> would be part of) needs to be documented.
>
> If I remember correctly, the proper safe usage of klp-convert requires a
> kernel built with -flive-patching, plus some scripting and/or manual
> processes.
Not always, I think: -flive-patching or IPA optimizations in general
aren't a concern in the context of data symbols. From a quick glance, it
seems like the selftests introduced as part of this patchset are
all restricted to this usecase.
> If nobody knows how to safely use it then there wouldn't be much value
> in merging it.
I tend to agree, but would put it a bit differently: the current
implementation of klp-convert features quite some convenience logic,
which, until the question of a documented livepatch preparation process
has been settled, is not known yet to ever be of any use.
For example, from [3/10]:
"For automatic resolution of livepatch relocations, a file called
symbols.klp 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."
For the source based approach to livepatch preparation we're using
internally, this is not really needed: the entity generating the source
-- be it klp-ccp or the author doing it manually -- needs to examine the
target objects long before link resp. klp-convert time for which symbols
can be referenced from the livepatch and how (i.e. determine a potential
sympos). I would expect it works similar for kpatch-build conceptually,
albeit kpatch-build probably doesn't rely on any external utility like
klp-convert for the .klp.* relas generation at all.
So with that, I agree that merging the klp-convert patchset in its
current form with those potentially unused convenience features,
presumably born out of certain assumptions about a manual livepatch
preparation process, indeed can be argued about, probably.
However, OTOH, there's currently no means whatsoever to create those
.klp.* relas (*) (**) and I would like to propose resorting to a more
minimal utility doing only that single thing: to stubbornly create
.klp.* relas out of certain "regular" ones using a very simple
transformation rule and nothing else beyond that. The "stripped"
klp-convert would have no knowledge of the symbols available in the
livepatched target objects at all, i.e. there would be no symbols.klp
file or alike anymore. Instead, it would simply walk through all of a
livepatch object's SHN_UNDEF symbols of format
".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" somewhen at
modpost time and
- rename the symbol to ".klp.sym.<foo-providing-mod>.some_foo,0" --
shortening the name should always be feasible as far as strtab is
concerned.
- turn the symbol's SHN_UNDEF into SHN_LIVEPATCH
- move any relocation (initially created by the compiler with source
based lp preparation approaches) against this symbol into a separate,
newly created rela section with flag SHF_RELA_LIVEPATCH set and whose
name is of format
.klp.rela.<loading-obj-name>.<livepatch-obj-dst-section-name>.
Furthermore, the new .klp.rela section's ->sh_info needs to be made to
refer to the destination section.
So, the only thing which would depend on the yet unspecified details of
the livepatch preparation process would be the creation of those
intermediate
".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" SHN_UNDEF
symbols to be processed by klp-convert. For source based livepatch
preparation approaches, counting in the selftests, this can be easily
controlled by means of asm("...") alias specifications at the respective
declarations like in e.g. extern int foo
asm("\".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0\"");
I imagine the first ones to benefit from having such a "stripped"
klp-convert available in the kernel tree would be new upstream selftests
for .klp.* rela coverage (like introduced with this here patchset
already) and for those some means of creating .klp.* relas would be
needed anyway. We (SUSE), and perhaps others as well, could integrate
this "stripped" klp-convert into our source based, production livepatch
preparation workflows right away, of course, and so we're obviously keen
on having it. Such a tool providing only the bare minimum would be
pretty much self-contained -- it would only need to hook into the
modpost Kbuild stage one way or the other -- and we could certainly
maintain it downstream out-of-tree, but that would potentially only
contribute to the current fragmentation around the livepatch creation
processes even more and there still wouldn't have a solution for the
upstream selftests.
What do you think, does it make sense to eventually have such a bare
minimum klp-convert merged in-tree, independently of the ongoing
discussion around the livepatch preparation processes, respectively (the
lack of) documentation around it? If yes, Lukas, now on CC, is
interested in this topic and would be willing to help out in any form
desired: either by contributing to Joe's work here or, if deemed more
feasible, to start out completely new from scratch -- dependent on your
opinion on the proposed, more minimal approach as well as on Joe's plans
around klp-convert.
Looking forward to hearing your feedback!
Thanks,
Nicolai
(*) We've been experimenting with building the relocation records
manually by various means, e.g. with GNU as' .reloc directive as an
example, but this all turned out impractical for various
reasons. Most noteworthy, because the records' offsets wouldn't get
adjusted properly when linking AFAIR.
(**) by some other means than directly with kpatch-build
--
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
(HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-04-11 10:06 ` Nicolai Stange
@ 2023-05-02 23:38 ` Marcos Paulo de Souza
2023-05-03 19:54 ` Joe Lawrence
1 sibling, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-05-02 23:38 UTC (permalink / raw)
To: Nicolai Stange
Cc: Josh Poimboeuf, Joe Lawrence, live-patching, linux-kernel,
linux-kbuild, Miroslav Benes, Petr Mladek, Marcos Paulo de Souza,
Lukas Hruska
On Tue, Apr 11, 2023 at 12:06:43PM +0200, Nicolai Stange wrote:
> Josh Poimboeuf <jpoimboe@kernel.org> writes:
>
> > On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> >> Have you tried retrofitting klp-convert into any real-world livepatch?
> >> I'm curious as to your observations on the overall experience, or
> >> thoughts on the sympos annotation style noted above.
> >
> > On a related note, the patch creation process (of which klp-convert
> > would be part of) needs to be documented.
> >
> > If I remember correctly, the proper safe usage of klp-convert requires a
> > kernel built with -flive-patching, plus some scripting and/or manual
> > processes.
>
> Not always, I think: -flive-patching or IPA optimizations in general
> aren't a concern in the context of data symbols. From a quick glance, it
> seems like the selftests introduced as part of this patchset are
> all restricted to this usecase.
<snip>
> What do you think, does it make sense to eventually have such a bare
> minimum klp-convert merged in-tree, independently of the ongoing
> discussion around the livepatch preparation processes, respectively (the
> lack of) documentation around it? If yes, Lukas, now on CC, is
> interested in this topic and would be willing to help out in any form
> desired: either by contributing to Joe's work here or, if deemed more
> feasible, to start out completely new from scratch -- dependent on your
> opinion on the proposed, more minimal approach as well as on Joe's plans
> around klp-convert.
>
> Looking forward to hearing your feedback!
So guys, any feedback? Ping :)
>
> Thanks,
>
> Nicolai
>
> (*) We've been experimenting with building the relocation records
> manually by various means, e.g. with GNU as' .reloc directive as an
> example, but this all turned out impractical for various
> reasons. Most noteworthy, because the records' offsets wouldn't get
> adjusted properly when linking AFAIR.
>
> (**) by some other means than directly with kpatch-build
>
> --
> SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> (HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-04-11 10:06 ` Nicolai Stange
2023-05-02 23:38 ` Marcos Paulo de Souza
@ 2023-05-03 19:54 ` Joe Lawrence
2023-05-09 20:34 ` Marcos Paulo de Souza
1 sibling, 1 reply; 29+ messages in thread
From: Joe Lawrence @ 2023-05-03 19:54 UTC (permalink / raw)
To: Nicolai Stange, Josh Poimboeuf, Marcos Paulo de Souza
Cc: live-patching, linux-kernel, linux-kbuild, Miroslav Benes,
Petr Mladek, Marcos Paulo de Souza, Lukas Hruska
On 4/11/23 06:06, Nicolai Stange wrote:
> Josh Poimboeuf <jpoimboe@kernel.org> writes:
>
>> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
>>> Have you tried retrofitting klp-convert into any real-world livepatch?
>>> I'm curious as to your observations on the overall experience, or
>>> thoughts on the sympos annotation style noted above.
>>
>> On a related note, the patch creation process (of which klp-convert
>> would be part of) needs to be documented.
>>
>> If I remember correctly, the proper safe usage of klp-convert requires a
>> kernel built with -flive-patching, plus some scripting and/or manual
>> processes.
>
> Not always, I think: -flive-patching or IPA optimizations in general
> aren't a concern in the context of data symbols. From a quick glance, it
> seems like the selftests introduced as part of this patchset are
> all restricted to this usecase.
>
IIRC there is nothing currently stopping klp-convert from converting
function symbol relocations. That may be dangerous when taking
optimizations like sibling functions (and their sharing of stack) into
consideration. This is about the point I stopped to turn and see what
the real use cases may be.
>> If nobody knows how to safely use it then there wouldn't be much value
>> in merging it.
>
> I tend to agree, but would put it a bit differently: the current
> implementation of klp-convert features quite some convenience logic,
> which, until the question of a documented livepatch preparation process
> has been settled, is not known yet to ever be of any use.
>
Good observation and perhaps something that Marcos could elaborate on
(pros and cons of klp-convert in his experiments).
> For example, from [3/10]:
>
> "For automatic resolution of livepatch relocations, a file called
> symbols.klp 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."
>
> For the source based approach to livepatch preparation we're using
> internally, this is not really needed: the entity generating the source
> -- be it klp-ccp or the author doing it manually -- needs to examine the
> target objects long before link resp. klp-convert time for which symbols
> can be referenced from the livepatch and how (i.e. determine a potential
> sympos). I would expect it works similar for kpatch-build conceptually,
> albeit kpatch-build probably doesn't rely on any external utility like
> klp-convert for the .klp.* relas generation at all.
>
Yes the conversion for kpatch-build the conversion is internal, though
split in parts:
The first step is during the binary comparison program
(create-diff-object) to place all the potential klp-relocations into a
temporary .kpatch_relocations section with info like dest, type,
objname, etc.
A second program, either create-klp-module [1] or the mostly deprecated
create-kpatch-module, iterates through the intermediate
.kpatch_relocation sections and converts to klp-relocation in the final
module output.
[1]
https://github.com/dynup/kpatch/blob/master/kpatch-build/create-klp-module.c#L158
> So with that, I agree that merging the klp-convert patchset in its
> current form with those potentially unused convenience features,
> presumably born out of certain assumptions about a manual livepatch
> preparation process, indeed can be argued about, probably.
>
>
> However, OTOH, there's currently no means whatsoever to create those
> .klp.* relas (*) (**) and I would like to propose resorting to a more
> minimal utility doing only that single thing: to stubbornly create
> .klp.* relas out of certain "regular" ones using a very simple
> transformation rule and nothing else beyond that. The "stripped"
> klp-convert would have no knowledge of the symbols available in the
> livepatched target objects at all, i.e. there would be no symbols.klp
> file or alike anymore. Instead, it would simply walk through all of a
> livepatch object's SHN_UNDEF symbols of format
> ".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" somewhen at
> modpost time and
> - rename the symbol to ".klp.sym.<foo-providing-mod>.some_foo,0" --
> shortening the name should always be feasible as far as strtab is
> concerned.
> - turn the symbol's SHN_UNDEF into SHN_LIVEPATCH
> - move any relocation (initially created by the compiler with source
> based lp preparation approaches) against this symbol into a separate,
> newly created rela section with flag SHF_RELA_LIVEPATCH set and whose
> name is of format
> .klp.rela.<loading-obj-name>.<livepatch-obj-dst-section-name>.
> Furthermore, the new .klp.rela section's ->sh_info needs to be made to
> refer to the destination section.
>
If I understand the idea, it would be similar to the second kpatch-build
step I mentioned above -- nothing fancy, just blindly convert symbols
marked SHN_UNDEF and a given naming convention.
FWIW, it may even be possible to retrofit kpatch-build to the format for
testing -- that would offer the ability to test many real-world CVE fixups.
> So, the only thing which would depend on the yet unspecified details of
> the livepatch preparation process would be the creation of those
> intermediate
> ".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0" SHN_UNDEF
> symbols to be processed by klp-convert. For source based livepatch
> preparation approaches, counting in the selftests, this can be easily
> controlled by means of asm("...") alias specifications at the respective
> declarations like in e.g. extern int foo
> asm("\".klp.sym.<loading-obj-name>.<foo-providing-mod>.some_foo,0\"");
>
>
> I imagine the first ones to benefit from having such a "stripped"
> klp-convert available in the kernel tree would be new upstream selftests
> for .klp.* rela coverage (like introduced with this here patchset
> already) and for those some means of creating .klp.* relas would be
> needed anyway. We (SUSE), and perhaps others as well, could integrate
> this "stripped" klp-convert into our source based, production livepatch
> preparation workflows right away, of course, and so we're obviously keen
> on having it. Such a tool providing only the bare minimum would be
> pretty much self-contained -- it would only need to hook into the
> modpost Kbuild stage one way or the other -- and we could certainly
> maintain it downstream out-of-tree, but that would potentially only
> contribute to the current fragmentation around the livepatch creation
> processes even more and there still wouldn't have a solution for the
> upstream selftests.
>
I think you summed it up that having an in-tree user, a test suite at
that, is a good motivation to maintain the conversion tool alongside.
Logistically it may easier to prototype and spin bugfixes faster locally
(esp. if it's supporting a quick CVE mitigation service), but hopefully
the rapid-dev/fix phase is only temporary and the normal upstream-first
development flow prevails.
> What do you think, does it make sense to eventually have such a bare
> minimum klp-convert merged in-tree, independently of the ongoing
> discussion around the livepatch preparation processes, respectively (the
> lack of) documentation around it? If yes, Lukas, now on CC, is
> interested in this topic and would be willing to help out in any form
> desired: either by contributing to Joe's work here or, if deemed more
> feasible, to start out completely new from scratch -- dependent on your
> opinion on the proposed, more minimal approach as well as on Joe's plans
> around klp-convert.
>
I like the incremental approach of minimizing the complexity of the
conversion tool. In fact, the hardest parts of this patchset (for me)
revolved around the kbuild integration to generate that symbols file :)
The next hardest where hypothetical problems with resolving to weird
read-only sections and then approaching compiler optimizations.
--
Joe
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-05-03 19:54 ` Joe Lawrence
@ 2023-05-09 20:34 ` Marcos Paulo de Souza
0 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-05-09 20:34 UTC (permalink / raw)
To: Joe Lawrence
Cc: Nicolai Stange, Josh Poimboeuf, live-patching, linux-kernel,
linux-kbuild, Miroslav Benes, Petr Mladek, Marcos Paulo de Souza,
Lukas Hruska
On Wed, May 03, 2023 at 03:54:47PM -0400, Joe Lawrence wrote:
> On 4/11/23 06:06, Nicolai Stange wrote:
> > Josh Poimboeuf <jpoimboe@kernel.org> writes:
> >
> >> On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> >>> Have you tried retrofitting klp-convert into any real-world livepatch?
> >>> I'm curious as to your observations on the overall experience, or
> >>> thoughts on the sympos annotation style noted above.
> >>
> >> On a related note, the patch creation process (of which klp-convert
> >> would be part of) needs to be documented.
> >>
> >> If I remember correctly, the proper safe usage of klp-convert requires a
> >> kernel built with -flive-patching, plus some scripting and/or manual
> >> processes.
> >
> > Not always, I think: -flive-patching or IPA optimizations in general
> > aren't a concern in the context of data symbols. From a quick glance, it
> > seems like the selftests introduced as part of this patchset are
> > all restricted to this usecase.
> >
>
> IIRC there is nothing currently stopping klp-convert from converting
> function symbol relocations. That may be dangerous when taking
> optimizations like sibling functions (and their sharing of stack) into
> consideration. This is about the point I stopped to turn and see what
> the real use cases may be.
>
> >> If nobody knows how to safely use it then there wouldn't be much value
> >> in merging it.
> >
> > I tend to agree, but would put it a bit differently: the current
> > implementation of klp-convert features quite some convenience logic,
> > which, until the question of a documented livepatch preparation process
> > has been settled, is not known yet to ever be of any use.
> >
>
> Good observation and perhaps something that Marcos could elaborate on
> (pros and cons of klp-convert in his experiments).
>
In my tests, I took the exact code generated by klp-ccp, and adapted it to not
rely on kallsyms anymore, removing the symbol lookups. For data symbols, I
changed it to be a extern variable instead of a pointer to it. For the function
symbols, also removed the pointer lookup, and left only the function prototype,
and it worked as expected.
I was quite surprised that it worked quite well. But I agree with
Nicolai that the tool itself could be shrunk into a smaller version. In our
usage, klp-ccp knows all unexported functions and to which modules they belong,
as it's currently used for the symbol lookup. I believe that kpatch-build also
has similar information so all the symbols.klp and other Kbuild machinery could be
avoided, making the tool responsible to only generate the klp relocations based
on the undefined symbols, just as proposed by Nicolai.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-17 20:29 ` Joe Lawrence
2023-03-17 23:20 ` Josh Poimboeuf
@ 2023-03-20 20:15 ` Marcos Paulo de Souza
1 sibling, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-03-20 20:15 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
On Fri, Mar 17, 2023 at 04:29:48PM -0400, Joe Lawrence wrote:
> On Tue, Mar 14, 2023 at 05:23:56PM -0300, Marcos Paulo de Souza wrote:
> > On Mon, Mar 06, 2023 at 09:08:14AM -0500, Joe Lawrence wrote:
> > > Summary
> > > -------
> > >
> > > 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 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, add support for symbol mapping in the form of a
> > > symbols.klp 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. For example. see the saved_command_line
> > > variable in lib/livepatch/test_klp_convert2.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. See the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections
> > > in lib/livepatch/test_klp_convert{1,2}.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 by
> > > removing the KLP_MODULE_RELOC / KLP_SYMPOS annotation sections in
> > > lib/livepatch/test_klp_convert{1,2}.c.
> > >
> > > Selftests have been added to exercise these klp-convert use-cases
> > > through several tests.
> > >
> > >
> > > Testing
> > > -------
> > >
> > > The patchset selftests build and execute on x86_64, s390x, and ppc64le
> > > for both default config (with added livepatch dependencies) and a larger
> > > RHEL-9-ish config.
> > >
> > > Using the Intel's Linux Kernel Performance tests's make.cross,
> > > klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> > > ppc32 s390 arm64 arches.
> > >
> > >
> > > Summary of changes in v7
> > > ------------------------
> > >
> > > - rebase for v6.2
> > > - combine ("livepatch: Add klp-convert tool") with ("livepatch: Add
> > > klp-convert annotation helpers")
> > > - combine ("kbuild: Support for symbols.klp creation") with ("modpost:
> > > Integrate klp-convert") to simplify Kbuild magic [Petr, Nicolas]
> > > - klp-convert: add safe_snprintf() (-Wsign-compare)
> > > - klp-convert: fix -Wsign-compare warnings
> > > - klp-convert: use calloc() where appropriate
> > > - klp-convert: copy ELF e_flags
> > > - selftests: fix various build warnings
> > > - klp-convert: WARN msg simplification, failed sanity checks, and sympos
> > > comment [Marcos]
> > > - klp-convert: fix elf_write_file() error paths [Petr]
> >
> > Thanks for the new version Joe. I've run the ksefltests on my x86 laptop, and it
> > succeed as expected, so
> >
> > Tested-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> >
>
> Thanks for the testing and reviews, Marcos.
>
> The selftests are the first level of testing... we should probably
> tackle a real or simulated CVE fix to see how well the tooling fits
> larger livepatches.
Our plan is to start testing new livepatches with klp-convert to ensure that it
works as expected, removing the need of kallsyms_lookup. Let's see how it goes
in the next few weeks.
>
> One complication that I can envision is symbol positioning. Currently,
> the klp-convert annotations are a direct mirror of the kernel's
> <obj,symbol,pos> tuple. It should be possible to make this a bit more
> user friendly for the livepatch developer if the annotations were
> <obj,file,symbol>, as derived from the vmlinux / module.tmp.ko symbol
> tables.
>
> For example, the following code:
>
> KLP_MODULE_RELOC(test_klp_convert_mod, test_klp_convert_mod_b.c) test_klp_convert_mod_relocs_b[] = {
> KLP_SYMPOS(homonym_string),
> KLP_SYMPOS(get_homonym_string),
> };
>
> could generate the following relocations:
>
> Relocation section '.rela.klp.module_relocs.test_klp_convert_mod.test_klp_convert_mod_b.c' at offset 0x1dc0 contains 2 entries:
> Offset Info Type Symbol's Value Symbol's Name + Addend
> 0000000000000000 0000003f00000001 R_X86_64_64 0000000000000000 homonym_string + 0
> 0000000000000008 0000004900000001 R_X86_64_64 0000000000000000 get_homonym_string + 0
>
> for which klp-convert looks up in symbols.klp:
>
> klp-convert-symbol-data.0.2
> *vmlinux
> ...
> *test_klp_convert_mod
> -test_klp_convert_mod_a.c << added filenames to the format
> test_klp_get_driver_name
> driver_name
> get_homonym_string << sympos = 1
> homonym_string << sympos = 1
> ...
> -test_klp_convert_mod_b.c
> get_homonym_string << sympos = 2
> homonym_string << sympos = 2
> ...
>
> and then generates the usual klp-relocations as currently defined.
This can get verbose very quickly, but I liked the idea. The sympos is much more
cryptic than specifying the file where the symbol lives on.
>
> (Unfortunately full pathnames are not saved in the STT_FILE symbol table
> entries, so there will be a few non-unique <obj,file,symbol> entries. I
> believe the last time this was discussed, we found that there were a
> relatively small number of such symbols.)
>
> Have you tried retrofitting klp-convert into any real-world livepatch?
> I'm curious as to your observations on the overall experience, or
> thoughts on the sympos annotation style noted above.
As I mentioned I'll start using klp-convert with new livepaches for testing
purposes in the next coming weeks. I'll reply in a few weeks about the
experience so far.
>
> Regards,
>
> -- Joe
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v7 00/10] livepatch: klp-convert tool
2023-03-06 14:08 [PATCH v7 00/10] livepatch: klp-convert tool Joe Lawrence
` (10 preceding siblings ...)
2023-03-14 20:23 ` [PATCH v7 00/10] livepatch: klp-convert tool Marcos Paulo de Souza
@ 2023-04-19 20:27 ` Marcos Paulo de Souza
11 siblings, 0 replies; 29+ messages in thread
From: Marcos Paulo de Souza @ 2023-04-19 20:27 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, linux-kernel, linux-kbuild, Josh Poimboeuf,
Miroslav Benes, Petr Mladek, Marcos Paulo de Souza
> Testing
> -------
>
> The patchset selftests build and execute on x86_64, s390x, and ppc64le
> for both default config (with added livepatch dependencies) and a larger
> RHEL-9-ish config.
>
> Using the Intel's Linux Kernel Performance tests's make.cross,
> klp-convert builds and processes livepatch .ko's for x86_64 ppc64le
> ppc32 s390 arm64 arches.
>
So I tested using real x86 livepatches, and it worked as expected. The test was
done by taking the livepatches, removing all externalized variables (those that
we need to call kallsyms_lookup), and compiling as usual and using
klp-convert to create the necessary relocation entries.
I'll keep testing with more livepatches for now on, and let you know if I find
any issue.
^ permalink raw reply [flat|nested] 29+ messages in thread