Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1280567 > unrolled thread

[RFC PATCH v2 0/6] (mostly) Arch-independent livepatch

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH v2 0/6] (mostly) Arch-independent livepatch Jessica Yu <jeyu@redhat.com> - 2015-12-01 05:30 +0100
    [RFC PATCH v2 2/6] module: preserve Elf information for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-12-01 05:30 +0100
      Re: module: preserve Elf information for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-12-01 09:50 +0100
      Re: module: preserve Elf information for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-12-01 22:10 +0100
      Re: [RFC PATCH v2 2/6] module: preserve Elf information for  livepatch modules Josh Poimboeuf <jpoimboe@redhat.com> - 2015-12-08 19:40 +0100
        Re: module: preserve Elf information for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-12-09 21:10 +0100
          Re: module: preserve Elf information for livepatch modules Josh Poimboeuf <jpoimboe@redhat.com> - 2015-12-10 15:40 +0100
    [RFC PATCH v2 6/6] Documentation: livepatch: outline the Elf format of a livepatch module Jessica Yu <jeyu@redhat.com> - 2015-12-01 05:30 +0100

#1280567 — [RFC PATCH v2 0/6] (mostly) Arch-independent livepatch

This patchset removes livepatch's need for architecture-specific relocation
code by leveraging existing code in the module loader to perform
arch-dependent work. Specifically, instead of duplicating code and
re-implementing what the apply_relocate_add() function in the module loader
already does in livepatch's klp_write_module_reloc(), we reuse
apply_relocate_add() to write relocations. The hope is that this will make
livepatch more easily portable to other architectures and greatly reduce
the amount of arch-specific code required to port livepatch to a particular
architecture.

Background: Why does livepatch need to write its own relocations?
==
A typical livepatch module contains patched versions of functions that can
reference non-exported global symbols and non-included local symbols.
Relocations referencing these types of symbols cannot be left in as-is
since the kernel module loader cannot resolve them and will therefore
reject the livepatch module. Furthermore, we cannot apply relocations that
affect modules not loaded yet at run time (e.g. a patch to a driver). The
current kpatch build system therefore solves this problem by embedding
special "dynrela" (dynamic reloc) sections in the resulting patch module
Elf output. Using these dynrela sections, livepatch can correctly resolve
symbols while taking into account its scope and what module the symbol
belongs to, and then manually apply the dynamic relocations.

Motivation: Why is having arch-dependent relocation code a problem?
==
The original motivation for this patchset stems from the increasing
roadblocks encountered while attempting to port livepatch to s390.
Specifically, there were problems dealing with s390 PLT and GOT relocation
types (R_390_{PLT,GOT}), which are handled differently from x86's
relocation types (which are much simpler to deal with, and a single
livepatch function (klp_write_module_reloc()) has been sufficient enough).
These s390 reloc types cannot be handled by simply performing a calculation
(as in the x86 case). For s390 modules with PLT/GOT relocations, the kernel
module loader allocates and fills in PLT+GOT table entries for every symbol
referenced by a PLT/GOT reloc in module core memory. So the problem of
porting livepatch to s390 became much more complicated than simply writing
an s390-specific klp_write_module_reloc() function. How can livepatch
handle these relocation types if the s390 module loader needs to allocate
and fill PLT/GOT entries ahead of time? The potential solutions were: 1)
have livepatch possibly allocate and maintain its own PLT/GOT tables for
every patch module (requiring even more arch-specific code), 2) modify the
s390 module loader heavily to accommodate livepatch modules (i.e. allocate
all the needed PLT/GOT entries for livepatch in advance but refrain from
applying relocations for to-be-patched modules), or 3) eliminate this
potential mess by leveraging module loader code to do all the relocation
work, letting livepatch off the hook completely. Solution #3 is what this
patchset implements.

How does this patchset remedy these problems?
==
Reusing the module loader code to perform livepatch relocations means that
livepatch no longer needs arch-specific reloc code and the aforementioned
problems with s390 PLT/GOT reloc types disappear (because we let the module
loader do all the relocation work for us). It will enable livepatch to be
more easily ported to other architectures.

Summary of proposed changes
==
This patch series enables livepatch to use the module loader's
apply_relocate_add() function to apply livepatch relocations (i.e. what
used to be dynrelas). apply_relocate_add() requires access to a patch
module's section headers, symbol table, reloc section indices, etc., and all
of these are accessible through the load_info struct used in the module
loader. Therefore we persist module Elf information (copied from load_info)
for livepatch modules.

The ELF-related changes enable livepatch to patch modules that are not yet
loaded (as well as patch vmlinux when kaslr is enabled). In order to use
apply_relocate_add(), we need real SHT_RELA sections to pass in. A
complication here is that relocations for not-yet-loaded modules should not
be applied when the patch module loads; they should only be applied once
the target module is loaded. Thus kpatch build scripts were modified to
output a livepatch module that contains special __klp_rela sections that
are managed by livepatch and are applied at the appropriate time (i.e. when
target module loads). They are marked with a special SHF_RELA_LIVEPATCH
section flag to indicate to the module loader that livepatch will handle
them. The SHN_LIVEPATCH shndx marks symbols that need to be resolved
once their respective target module loads. So, the module loader ignores
these symbols and does not attempt to resolve them. Finally, the
STB_LIVEPATCH_EXT symbol bind marks the scope of certain livepatch symbols,
so that livepatch can find the symbol in the right place. These ELF
constants were selected from OS-specific ranges according to the
definitions from glibc.

v2:
 - Copy only the minimum required Elf information for livepatch modules to
   make the call to apply_relocate_add(), not the entire load_info struct
   and the redundant copy of the module in memory
 - Add module->klp flag for simple identification of livepatch modules
 - s390: remove redundant vfree() and preserve mod_arch_specific if
   livepatch module
 - Use array format instead of a linked list for klp_reloc_secs
 - Add new documentation describing the format of a livepatch module in
   Documentation/livepatch

Jessica Yu (6):
  Elf: add livepatch-specific Elf constants
  module: preserve Elf information for livepatch modules
  module: s390: keep mod_arch_specific for livepatch modules
  livepatch: reuse module loader code to write relocations
  samples: livepatch: init reloc section array and mark as klp module
  Documentation: livepatch: outline the Elf format of a livepatch module

 Documentation/livepatch/patch-module-format.txt | 117 ++++++++++++++++++++++++
 arch/s390/kernel/module.c                       |  13 ++-
 arch/x86/include/asm/livepatch.h                |   2 -
 arch/x86/kernel/Makefile                        |   1 -
 arch/x86/kernel/livepatch.c                     |  91 ------------------
 include/linux/livepatch.h                       |  30 +++---
 include/linux/module.h                          |  15 +++
 include/uapi/linux/elf.h                        |  17 ++--
 kernel/livepatch/core.c                         |  94 ++++++++++---------
 kernel/module.c                                 |  98 +++++++++++++++++++-
 samples/livepatch/livepatch-sample.c            |   6 ++
 11 files changed, 319 insertions(+), 165 deletions(-)
 create mode 100644 Documentation/livepatch/patch-module-format.txt
 delete mode 100644 arch/x86/kernel/livepatch.c

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1280568 — [RFC PATCH v2 2/6] module: preserve Elf information for livepatch modules

For livepatch modules, copy Elf section, symbol, and string information
from the load_info struct in the module loader.

Livepatch uses special relocation sections in order to be able to patch
modules that are not yet loaded, as well as apply patches to the kernel
when the addresses of symbols cannot be determined at compile time (for
example, when kaslr is enabled). Livepatch modules must preserve Elf
information such as section indices in order to apply the remaining
relocation sections at the appropriate time (i.e. when the target module
loads).

Signed-off-by: Jessica Yu <jeyu@redhat.com>
---
 include/linux/module.h |  9 +++++
 kernel/module.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 3a19c79..9b46256 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -425,6 +425,14 @@ struct module {
 
 	/* Notes attributes */
 	struct module_notes_attrs *notes_attrs;
+
+	/* Elf information (optionally saved) */
+	Elf_Ehdr *hdr;
+	Elf_Shdr *sechdrs;
+	char *secstrings;
+	struct {
+		unsigned int sym, str, mod, vers, info, pcpu;
+	} index;
 #endif
 
 	/* The command line arguments (may be mangled).  People like
@@ -461,6 +469,7 @@ struct module {
 #endif
 
 #ifdef CONFIG_LIVEPATCH
+	bool klp; /* Is this a livepatch module? */
 	bool klp_alive;
 #endif
 
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..433c2d6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1984,6 +1984,13 @@ static void unset_module_core_ro_nx(struct module *mod) { }
 static void unset_module_init_ro_nx(struct module *mod) { }
 #endif
 
+static void free_module_elf(struct module *mod)
+{
+	kfree(mod->hdr);
+	kfree(mod->sechdrs);
+	kfree(mod->secstrings);
+}
+
 void __weak module_memfree(void *module_region)
 {
 	vfree(module_region);
@@ -2022,6 +2029,9 @@ static void free_module(struct module *mod)
 	/* Free any allocated parameters. */
 	destroy_params(mod->kp, mod->num_kp);
 
+	/* Free Elf information if it was saved */
+	free_module_elf(mod);
+
 	/* Now we can delete it from the lists */
 	mutex_lock(&module_mutex);
 	/* Unlink carefully: kallsyms could be walking list. */
@@ -2137,6 +2147,10 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
 			       (long)sym[i].st_value);
 			break;
 
+		case SHN_LIVEPATCH:
+			/* klp symbols are resolved by livepatch */
+			break;
+
 		case SHN_UNDEF:
 			ksym = resolve_symbol_wait(mod, info, name);
 			/* Ok if resolved.  */
@@ -2185,6 +2199,10 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
 		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
 			continue;
 
+		/* klp relocation sections are applied by livepatch */
+		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
+			continue;
+
 		if (info->sechdrs[i].sh_type == SHT_REL)
 			err = apply_relocate(info->sechdrs, info->strtab,
 					     info->index.sym, i, mod);
@@ -2393,6 +2411,11 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
 {
 	const Elf_Shdr *sechdrs = info->sechdrs;
 
+	if (ELF_ST_BIND(sym->st_info) == STB_LIVEPATCH_EXT)
+		return 'K';
+	if (sym->st_shndx == SHN_LIVEPATCH)
+		return 'k';
+
 	if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
 		if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT)
 			return 'v';
@@ -2475,7 +2498,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
 
 	/* Compute total space required for the core symbols' strtab. */
 	for (ndst = i = 0; i < nsrc; i++) {
-		if (i == 0 ||
+		if (i == 0 || mod->klp ||
 		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
 			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
 			ndst++;
@@ -2517,7 +2540,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
 	mod->core_strtab = s = mod->module_core + info->stroffs;
 	src = mod->symtab;
 	for (ndst = i = 0; i < mod->num_symtab; i++) {
-		if (i == 0 ||
+		if (i == 0 || mod->klp ||
 		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
 			dst[ndst] = src[i];
 			dst[ndst++].st_name = s - mod->core_strtab;
@@ -2638,6 +2661,64 @@ static int elf_header_check(struct load_info *info)
 	return 0;
 }
 
+/*
+ * copy_module_elf - preserve Elf information about a module
+ */
+static int copy_module_elf(struct module *mod, struct load_info *info)
+{
+	unsigned int size;
+	int ret = 0;
+	Elf_Shdr *symsect;
+
+	/* Elf header */
+	size = sizeof(Elf_Ehdr);
+	mod->hdr = kzalloc(size, GFP_KERNEL);
+	if (mod->hdr == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	memcpy(mod->hdr, info->hdr, size);
+
+	/* Elf section header table */
+	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
+	mod->sechdrs = kzalloc(size, GFP_KERNEL);
+	if (mod->sechdrs == NULL) {
+		ret = -ENOMEM;
+		goto free_hdr;
+	}
+	memcpy(mod->sechdrs, info->sechdrs, size);
+
+	/* Elf section name string table */
+	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
+	mod->secstrings = kzalloc(size, GFP_KERNEL);
+	if (mod->secstrings == NULL) {
+		ret = -ENOMEM;
+		goto free_sechdrs;
+	}
+	memcpy(mod->secstrings, info->secstrings, size);
+
+	/* Elf section indices */
+	memcpy(&mod->index, &info->index, sizeof(info->index));
+
+	/*
+	 * Update symtab's sh_addr to point to a valid
+	 * symbol table, as the temporary symtab in module
+	 * init memory will be freed
+	 */
+	symsect = mod->sechdrs + mod->index.sym;
+	symsect->sh_addr = (unsigned long)mod->core_symtab;
+
+	return ret;
+
+free_sechdrs:
+	kfree(mod->sechdrs);
+free_hdr:
+	kfree(mod->hdr);
+out:
+	return ret;
+}
+
+
 #define COPY_CHUNK_SIZE (16*PAGE_SIZE)
 
 static int copy_chunked_from_user(void *dst, const void __user *usrc, unsigned long len)
@@ -2866,6 +2947,9 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 			"is unknown, you have been warned.\n", mod->name);
 	}
 
+	if (get_modinfo(info, "livepatch"))
+		mod->klp = true;
+
 	/* Set up license info based on the info section */
 	set_license(mod, get_modinfo(info, "license"));
 
@@ -3530,6 +3614,16 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err < 0)
 		goto bug_cleanup;
 
+	/*
+	 * Save sechdrs, indices, and other data from info
+	 * in order to patch to-be-loaded modules.
+	 * Do not call free_copy() for livepatch modules.
+	 */
+	if (mod->klp)
+		err = copy_module_elf(mod, info);
+	if (err < 0)
+		goto bug_cleanup;
+
 	/* Get rid of temporary copy. */
 	free_copy(info);
 
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280654 — Re: module: preserve Elf information for livepatch modules

+++ Jessica Yu [30/11/15 23:21 -0500]:
>For livepatch modules, copy Elf section, symbol, and string information
>from the load_info struct in the module loader.
>
>Livepatch uses special relocation sections in order to be able to patch
>modules that are not yet loaded, as well as apply patches to the kernel
>when the addresses of symbols cannot be determined at compile time (for
>example, when kaslr is enabled). Livepatch modules must preserve Elf
>information such as section indices in order to apply the remaining
>relocation sections at the appropriate time (i.e. when the target module
>loads).
>
>Signed-off-by: Jessica Yu <jeyu@redhat.com>
>---
> include/linux/module.h |  9 +++++
> kernel/module.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 105 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/module.h b/include/linux/module.h
>index 3a19c79..9b46256 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -425,6 +425,14 @@ struct module {
>
> 	/* Notes attributes */
> 	struct module_notes_attrs *notes_attrs;
>+
>+	/* Elf information (optionally saved) */
>+	Elf_Ehdr *hdr;
>+	Elf_Shdr *sechdrs;
>+	char *secstrings;
>+	struct {
>+		unsigned int sym, str, mod, vers, info, pcpu;
>+	} index;
> #endif
>
> 	/* The command line arguments (may be mangled).  People like
>@@ -461,6 +469,7 @@ struct module {
> #endif
>
> #ifdef CONFIG_LIVEPATCH
>+	bool klp; /* Is this a livepatch module? */

Gah. I believe this field should be outside the #ifdef. 

Jessica
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281216 — Re: module: preserve Elf information for livepatch modules

+++ Jessica Yu [30/11/15 23:21 -0500]:
>For livepatch modules, copy Elf section, symbol, and string information
>from the load_info struct in the module loader.
>
>Livepatch uses special relocation sections in order to be able to patch
>modules that are not yet loaded, as well as apply patches to the kernel
>when the addresses of symbols cannot be determined at compile time (for
>example, when kaslr is enabled). Livepatch modules must preserve Elf
>information such as section indices in order to apply the remaining
>relocation sections at the appropriate time (i.e. when the target module
>loads).
>
>Signed-off-by: Jessica Yu <jeyu@redhat.com>
>---
> include/linux/module.h |  9 +++++
> kernel/module.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 105 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/module.h b/include/linux/module.h
>index 3a19c79..9b46256 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -425,6 +425,14 @@ struct module {
>
> 	/* Notes attributes */
> 	struct module_notes_attrs *notes_attrs;
>+
>+	/* Elf information (optionally saved) */
>+	Elf_Ehdr *hdr;
>+	Elf_Shdr *sechdrs;
>+	char *secstrings;
>+	struct {
>+		unsigned int sym, str, mod, vers, info, pcpu;
>+	} index;
> #endif

This particular patch unforunately breaks !CONFIG_KALLSYMS kernel
builds, as I've just discovered. These fields should move out of the
CONFIG_KALLSYMS block. And..

> 	/* The command line arguments (may be mangled).  People like
>@@ -461,6 +469,7 @@ struct module {
> #endif
>
> #ifdef CONFIG_LIVEPATCH
>+	bool klp; /* Is this a livepatch module? */
> 	bool klp_alive;
> #endif
>
>diff --git a/kernel/module.c b/kernel/module.c
>index 8f051a1..433c2d6 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -1984,6 +1984,13 @@ static void unset_module_core_ro_nx(struct module *mod) { }
> static void unset_module_init_ro_nx(struct module *mod) { }
> #endif
>
>+static void free_module_elf(struct module *mod)
>+{
>+	kfree(mod->hdr);
>+	kfree(mod->sechdrs);
>+	kfree(mod->secstrings);
>+}
>+
> void __weak module_memfree(void *module_region)
> {
> 	vfree(module_region);
>@@ -2022,6 +2029,9 @@ static void free_module(struct module *mod)
> 	/* Free any allocated parameters. */
> 	destroy_params(mod->kp, mod->num_kp);
>
>+	/* Free Elf information if it was saved */
>+	free_module_elf(mod);
>+
> 	/* Now we can delete it from the lists */
> 	mutex_lock(&module_mutex);
> 	/* Unlink carefully: kallsyms could be walking list. */
>@@ -2137,6 +2147,10 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
> 			       (long)sym[i].st_value);
> 			break;
>
>+		case SHN_LIVEPATCH:
>+			/* klp symbols are resolved by livepatch */
>+			break;
>+
> 		case SHN_UNDEF:
> 			ksym = resolve_symbol_wait(mod, info, name);
> 			/* Ok if resolved.  */
>@@ -2185,6 +2199,10 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
> 		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
> 			continue;
>
>+		/* klp relocation sections are applied by livepatch */
>+		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
>+			continue;
>+
> 		if (info->sechdrs[i].sh_type == SHT_REL)
> 			err = apply_relocate(info->sechdrs, info->strtab,
> 					     info->index.sym, i, mod);
>@@ -2393,6 +2411,11 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
> {
> 	const Elf_Shdr *sechdrs = info->sechdrs;
>
>+	if (ELF_ST_BIND(sym->st_info) == STB_LIVEPATCH_EXT)
>+		return 'K';
>+	if (sym->st_shndx == SHN_LIVEPATCH)
>+		return 'k';
>+
> 	if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
> 		if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT)
> 			return 'v';
>@@ -2475,7 +2498,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>
> 	/* Compute total space required for the core symbols' strtab. */
> 	for (ndst = i = 0; i < nsrc; i++) {
>-		if (i == 0 ||
>+		if (i == 0 || mod->klp ||
> 		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
> 			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
> 			ndst++;
>@@ -2517,7 +2540,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
> 	mod->core_strtab = s = mod->module_core + info->stroffs;
> 	src = mod->symtab;
> 	for (ndst = i = 0; i < mod->num_symtab; i++) {
>-		if (i == 0 ||
>+		if (i == 0 || mod->klp ||
> 		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
> 			dst[ndst] = src[i];
> 			dst[ndst++].st_name = s - mod->core_strtab;
>@@ -2638,6 +2661,64 @@ static int elf_header_check(struct load_info *info)
> 	return 0;
> }
>
>+/*
>+ * copy_module_elf - preserve Elf information about a module
>+ */
>+static int copy_module_elf(struct module *mod, struct load_info *info)
>+{
>+	unsigned int size;
>+	int ret = 0;
>+	Elf_Shdr *symsect;
>+
>+	/* Elf header */
>+	size = sizeof(Elf_Ehdr);
>+	mod->hdr = kzalloc(size, GFP_KERNEL);
>+	if (mod->hdr == NULL) {
>+		ret = -ENOMEM;
>+		goto out;
>+	}
>+	memcpy(mod->hdr, info->hdr, size);
>+
>+	/* Elf section header table */
>+	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
>+	mod->sechdrs = kzalloc(size, GFP_KERNEL);
>+	if (mod->sechdrs == NULL) {
>+		ret = -ENOMEM;
>+		goto free_hdr;
>+	}
>+	memcpy(mod->sechdrs, info->sechdrs, size);
>+
>+	/* Elf section name string table */
>+	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
>+	mod->secstrings = kzalloc(size, GFP_KERNEL);
>+	if (mod->secstrings == NULL) {
>+		ret = -ENOMEM;
>+		goto free_sechdrs;
>+	}
>+	memcpy(mod->secstrings, info->secstrings, size);
>+
>+	/* Elf section indices */
>+	memcpy(&mod->index, &info->index, sizeof(info->index));
>+
>+	/*
>+	 * Update symtab's sh_addr to point to a valid
>+	 * symbol table, as the temporary symtab in module
>+	 * init memory will be freed
>+	 */
>+	symsect = mod->sechdrs + mod->index.sym;
>+	symsect->sh_addr = (unsigned long)mod->core_symtab;

..instead of relying on CONFIG_KALLSYMS being set, check for
CONFIG_KALLSYMS before the sh_addr assignment above (mod->core_symtab
only exists when CONFIG_KALLSYMS is set). Then we can leave
{free,copy}_module_elf() outside any #ifdef blocks.

Jessica
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1286763 — Re: [RFC PATCH v2 2/6] module: preserve Elf information for livepatch modules

On Mon, Nov 30, 2015 at 11:21:15PM -0500, Jessica Yu wrote:
> For livepatch modules, copy Elf section, symbol, and string information
> from the load_info struct in the module loader.
> 
> Livepatch uses special relocation sections in order to be able to patch
> modules that are not yet loaded, as well as apply patches to the kernel
> when the addresses of symbols cannot be determined at compile time (for
> example, when kaslr is enabled). Livepatch modules must preserve Elf
> information such as section indices in order to apply the remaining
> relocation sections at the appropriate time (i.e. when the target module
> loads).
> 
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> ---
>  include/linux/module.h |  9 +++++
>  kernel/module.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 105 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3a19c79..9b46256 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -425,6 +425,14 @@ struct module {
>  
>  	/* Notes attributes */
>  	struct module_notes_attrs *notes_attrs;
> +
> +	/* Elf information (optionally saved) */
> +	Elf_Ehdr *hdr;

I would rename "hdr" to "elf_hdr" to make its purpose clearer.

> +	Elf_Shdr *sechdrs;
> +	char *secstrings;

Probably a good idea to add underscores to the names ("sec_hdrs" and
"sec_strings") to be consistent with most of the other fields in the
struct.

> +	struct {
> +		unsigned int sym, str, mod, vers, info, pcpu;
> +	} index;

I might be contradicting myself from what I said before.  But I'm
thinking we should put all these fields inside a CONFIG_LIVEPATCH ifdef.
Then below, there could be two versions of copy_module_elf(), the real
one for LIVEPATCH and and an empty one for !LIVEPATCH.  And the same
story for free_module_elf().

>  #endif
>  
>  	/* The command line arguments (may be mangled).  People like
> @@ -461,6 +469,7 @@ struct module {
>  #endif
>  
>  #ifdef CONFIG_LIVEPATCH
> +	bool klp; /* Is this a livepatch module? */
>  	bool klp_alive;
>  #endif
>  
> diff --git a/kernel/module.c b/kernel/module.c
> index 8f051a1..433c2d6 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1984,6 +1984,13 @@ static void unset_module_core_ro_nx(struct module *mod) { }
>  static void unset_module_init_ro_nx(struct module *mod) { }
>  #endif
>  
> +static void free_module_elf(struct module *mod)
> +{
> +	kfree(mod->hdr);
> +	kfree(mod->sechdrs);
> +	kfree(mod->secstrings);
> +}
> +
>  void __weak module_memfree(void *module_region)
>  {
>  	vfree(module_region);
> @@ -2022,6 +2029,9 @@ static void free_module(struct module *mod)
>  	/* Free any allocated parameters. */
>  	destroy_params(mod->kp, mod->num_kp);
>  
> +	/* Free Elf information if it was saved */
> +	free_module_elf(mod);
> +
>  	/* Now we can delete it from the lists */
>  	mutex_lock(&module_mutex);
>  	/* Unlink carefully: kallsyms could be walking list. */
> @@ -2137,6 +2147,10 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
>  			       (long)sym[i].st_value);
>  			break;
>  
> +		case SHN_LIVEPATCH:
> +			/* klp symbols are resolved by livepatch */
> +			break;
> +
>  		case SHN_UNDEF:
>  			ksym = resolve_symbol_wait(mod, info, name);
>  			/* Ok if resolved.  */
> @@ -2185,6 +2199,10 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
>  		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
>  			continue;
>  
> +		/* klp relocation sections are applied by livepatch */
> +		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
> +			continue;
> +
>  		if (info->sechdrs[i].sh_type == SHT_REL)
>  			err = apply_relocate(info->sechdrs, info->strtab,
>  					     info->index.sym, i, mod);
> @@ -2393,6 +2411,11 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
>  {
>  	const Elf_Shdr *sechdrs = info->sechdrs;
>  
> +	if (ELF_ST_BIND(sym->st_info) == STB_LIVEPATCH_EXT)
> +		return 'K';
> +	if (sym->st_shndx == SHN_LIVEPATCH)
> +		return 'k';
> +
>  	if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
>  		if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT)
>  			return 'v';
> @@ -2475,7 +2498,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>  
>  	/* Compute total space required for the core symbols' strtab. */
>  	for (ndst = i = 0; i < nsrc; i++) {
> -		if (i == 0 ||
> +		if (i == 0 || mod->klp ||
>  		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
>  			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
>  			ndst++;

Instead of accessing mod->klp directly, how about an
'is_livepatch_module(mod)' function.  There could be two versions, with
the !LIVEPATCH version always returning false and the LIVEPATCH version
checking mod->klp.  Then mod->klp itself can stay inside the LIVEPATCH
ifdef in the module struct.

> @@ -2517,7 +2540,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
>  	mod->core_strtab = s = mod->module_core + info->stroffs;
>  	src = mod->symtab;
>  	for (ndst = i = 0; i < mod->num_symtab; i++) {
> -		if (i == 0 ||
> +		if (i == 0 || mod->klp ||
>  		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
>  			dst[ndst] = src[i];
>  			dst[ndst++].st_name = s - mod->core_strtab;
> @@ -2638,6 +2661,64 @@ static int elf_header_check(struct load_info *info)
>  	return 0;
>  }
>  
> +/*
> + * copy_module_elf - preserve Elf information about a module
> + */
> +static int copy_module_elf(struct module *mod, struct load_info *info)
> +{
> +	unsigned int size;
> +	int ret = 0;

No need to initialize ret to zero here since it's never used
uninitalized.

> +	Elf_Shdr *symsect;
> +
> +	/* Elf header */
> +	size = sizeof(Elf_Ehdr);
> +	mod->hdr = kzalloc(size, GFP_KERNEL);

No need to zero the memory here with kzalloc() since it will all be
memcpy()'d anyway.  kmalloc() can be used instead (and the same for the
other kzalloc()s below).

> +	if (mod->hdr == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	memcpy(mod->hdr, info->hdr, size);
> +
> +	/* Elf section header table */
> +	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
> +	mod->sechdrs = kzalloc(size, GFP_KERNEL);
> +	if (mod->sechdrs == NULL) {
> +		ret = -ENOMEM;
> +		goto free_hdr;
> +	}
> +	memcpy(mod->sechdrs, info->sechdrs, size);
> +
> +	/* Elf section name string table */
> +	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
> +	mod->secstrings = kzalloc(size, GFP_KERNEL);
> +	if (mod->secstrings == NULL) {
> +		ret = -ENOMEM;
> +		goto free_sechdrs;
> +	}
> +	memcpy(mod->secstrings, info->secstrings, size);
> +
> +	/* Elf section indices */
> +	memcpy(&mod->index, &info->index, sizeof(info->index));
> +
> +	/*
> +	 * Update symtab's sh_addr to point to a valid
> +	 * symbol table, as the temporary symtab in module
> +	 * init memory will be freed
> +	 */
> +	symsect = mod->sechdrs + mod->index.sym;
> +	symsect->sh_addr = (unsigned long)mod->core_symtab;
> +
> +	return ret;
> +
> +free_sechdrs:
> +	kfree(mod->sechdrs);
> +free_hdr:
> +	kfree(mod->hdr);
> +out:
> +	return ret;
> +}
> +
> +
>  #define COPY_CHUNK_SIZE (16*PAGE_SIZE)
>  
>  static int copy_chunked_from_user(void *dst, const void __user *usrc, unsigned long len)
> @@ -2866,6 +2947,9 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
>  			"is unknown, you have been warned.\n", mod->name);
>  	}
>  
> +	if (get_modinfo(info, "livepatch"))
> +		mod->klp = true;
> +

Similar to the is_livepatch_module() function I suggested, this can be
put in a function so that mod->klp can be abstracted away for the
!LIVEPATCH case.  Maybe there should be a check_livepatch_modinfo()
function:

1. the !LIVEPATCH version of the function could return an error if
modinfo has "livepatch"

2. the LIVEPATCH version could simply set mod->klp = true.

>  	/* Set up license info based on the info section */
>  	set_license(mod, get_modinfo(info, "license"));
>  
> @@ -3530,6 +3614,16 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  	if (err < 0)
>  		goto bug_cleanup;
>  
> +	/*
> +	 * Save sechdrs, indices, and other data from info
> +	 * in order to patch to-be-loaded modules.
> +	 * Do not call free_copy() for livepatch modules.

I think the last line of this comment isn't right, since free_copy() is
called below regardless.

> +	 */
> +	if (mod->klp)
> +		err = copy_module_elf(mod, info);
> +	if (err < 0)
> +		goto bug_cleanup;

Not strictly necessary, but I think it would be a little cleaner to only
check the err if copy_module_elf() was called.

> +
>  	/* Get rid of temporary copy. */
>  	free_copy(info);

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1287808 — Re: module: preserve Elf information for livepatch modules

+++ Josh Poimboeuf [08/12/15 12:32 -0600]:
>On Mon, Nov 30, 2015 at 11:21:15PM -0500, Jessica Yu wrote:
>> For livepatch modules, copy Elf section, symbol, and string information
>> from the load_info struct in the module loader.
>>
>> Livepatch uses special relocation sections in order to be able to patch
>> modules that are not yet loaded, as well as apply patches to the kernel
>> when the addresses of symbols cannot be determined at compile time (for
>> example, when kaslr is enabled). Livepatch modules must preserve Elf
>> information such as section indices in order to apply the remaining
>> relocation sections at the appropriate time (i.e. when the target module
>> loads).
>>
>> Signed-off-by: Jessica Yu <jeyu@redhat.com>
>> ---
>>  include/linux/module.h |  9 +++++
>>  kernel/module.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 105 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 3a19c79..9b46256 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -425,6 +425,14 @@ struct module {
>>
>>  	/* Notes attributes */
>>  	struct module_notes_attrs *notes_attrs;
>> +
>> +	/* Elf information (optionally saved) */
>> +	Elf_Ehdr *hdr;
>
>I would rename "hdr" to "elf_hdr" to make its purpose clearer.
>
>> +	Elf_Shdr *sechdrs;
>> +	char *secstrings;
>
>Probably a good idea to add underscores to the names ("sec_hdrs" and
>"sec_strings") to be consistent with most of the other fields in the
>struct.
>
>> +	struct {
>> +		unsigned int sym, str, mod, vers, info, pcpu;
>> +	} index;
>
>I might be contradicting myself from what I said before.  But I'm
>thinking we should put all these fields inside a CONFIG_LIVEPATCH ifdef.
>Then below, there could be two versions of copy_module_elf(), the real
>one for LIVEPATCH and and an empty one for !LIVEPATCH.  And the same
>story for free_module_elf().

I think in the v1 discussion we were leaning more towards making this
generic. We could potentially just have the Elf module fields
available in the generic case, independent of whether CONFIG_LIVEPATCH
is set, whereas the mod->klp field should probably be only available
when LIVEPATCH is set. I think this makes sense since the Elf fields
aren't dependent on livepatch (although livepatch would be the only
user of these fields at the moment). I don't know if there would be
any users in the future that would be interested in using this Elf
information. Thoughts on this?

>>  #endif
>>
>>  	/* The command line arguments (may be mangled).  People like
>> @@ -461,6 +469,7 @@ struct module {
>>  #endif
>>
>>  #ifdef CONFIG_LIVEPATCH
>> +	bool klp; /* Is this a livepatch module? */
>>  	bool klp_alive;
>>  #endif
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 8f051a1..433c2d6 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -1984,6 +1984,13 @@ static void unset_module_core_ro_nx(struct module *mod) { }
>>  static void unset_module_init_ro_nx(struct module *mod) { }
>>  #endif
>>
>> +static void free_module_elf(struct module *mod)
>> +{
>> +	kfree(mod->hdr);
>> +	kfree(mod->sechdrs);
>> +	kfree(mod->secstrings);
>> +}
>> +
>>  void __weak module_memfree(void *module_region)
>>  {
>>  	vfree(module_region);
>> @@ -2022,6 +2029,9 @@ static void free_module(struct module *mod)
>>  	/* Free any allocated parameters. */
>>  	destroy_params(mod->kp, mod->num_kp);
>>
>> +	/* Free Elf information if it was saved */
>> +	free_module_elf(mod);
>> +
>>  	/* Now we can delete it from the lists */
>>  	mutex_lock(&module_mutex);
>>  	/* Unlink carefully: kallsyms could be walking list. */
>> @@ -2137,6 +2147,10 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
>>  			       (long)sym[i].st_value);
>>  			break;
>>
>> +		case SHN_LIVEPATCH:
>> +			/* klp symbols are resolved by livepatch */
>> +			break;
>> +
>>  		case SHN_UNDEF:
>>  			ksym = resolve_symbol_wait(mod, info, name);
>>  			/* Ok if resolved.  */
>> @@ -2185,6 +2199,10 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
>>  		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
>>  			continue;
>>
>> +		/* klp relocation sections are applied by livepatch */
>> +		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
>> +			continue;
>> +
>>  		if (info->sechdrs[i].sh_type == SHT_REL)
>>  			err = apply_relocate(info->sechdrs, info->strtab,
>>  					     info->index.sym, i, mod);
>> @@ -2393,6 +2411,11 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
>>  {
>>  	const Elf_Shdr *sechdrs = info->sechdrs;
>>
>> +	if (ELF_ST_BIND(sym->st_info) == STB_LIVEPATCH_EXT)
>> +		return 'K';
>> +	if (sym->st_shndx == SHN_LIVEPATCH)
>> +		return 'k';
>> +
>>  	if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
>>  		if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT)
>>  			return 'v';
>> @@ -2475,7 +2498,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>>
>>  	/* Compute total space required for the core symbols' strtab. */
>>  	for (ndst = i = 0; i < nsrc; i++) {
>> -		if (i == 0 ||
>> +		if (i == 0 || mod->klp ||
>>  		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
>>  			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
>>  			ndst++;
>
>Instead of accessing mod->klp directly, how about an
>'is_livepatch_module(mod)' function.  There could be two versions, with
>the !LIVEPATCH version always returning false and the LIVEPATCH version
>checking mod->klp.  Then mod->klp itself can stay inside the LIVEPATCH
>ifdef in the module struct.

OK, that sounds good. Makes more sense for the klp field to exist only
in the LIVEPATCH case anyway.

>> @@ -2517,7 +2540,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
>>  	mod->core_strtab = s = mod->module_core + info->stroffs;
>>  	src = mod->symtab;
>>  	for (ndst = i = 0; i < mod->num_symtab; i++) {
>> -		if (i == 0 ||
>> +		if (i == 0 || mod->klp ||
>>  		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
>>  			dst[ndst] = src[i];
>>  			dst[ndst++].st_name = s - mod->core_strtab;
>> @@ -2638,6 +2661,64 @@ static int elf_header_check(struct load_info *info)
>>  	return 0;
>>  }
>>
>> +/*
>> + * copy_module_elf - preserve Elf information about a module
>> + */
>> +static int copy_module_elf(struct module *mod, struct load_info *info)
>> +{
>> +	unsigned int size;
>> +	int ret = 0;
>
>No need to initialize ret to zero here since it's never used
>uninitalized.
>
>> +	Elf_Shdr *symsect;
>> +
>> +	/* Elf header */
>> +	size = sizeof(Elf_Ehdr);
>> +	mod->hdr = kzalloc(size, GFP_KERNEL);
>
>No need to zero the memory here with kzalloc() since it will all be
>memcpy()'d anyway.  kmalloc() can be used instead (and the same for the
>other kzalloc()s below).
>
>> +	if (mod->hdr == NULL) {
>> +		ret = -ENOMEM;
>> +		goto out;
>> +	}
>> +	memcpy(mod->hdr, info->hdr, size);
>> +
>> +	/* Elf section header table */
>> +	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
>> +	mod->sechdrs = kzalloc(size, GFP_KERNEL);
>> +	if (mod->sechdrs == NULL) {
>> +		ret = -ENOMEM;
>> +		goto free_hdr;
>> +	}
>> +	memcpy(mod->sechdrs, info->sechdrs, size);
>> +
>> +	/* Elf section name string table */
>> +	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
>> +	mod->secstrings = kzalloc(size, GFP_KERNEL);
>> +	if (mod->secstrings == NULL) {
>> +		ret = -ENOMEM;
>> +		goto free_sechdrs;
>> +	}
>> +	memcpy(mod->secstrings, info->secstrings, size);
>> +
>> +	/* Elf section indices */
>> +	memcpy(&mod->index, &info->index, sizeof(info->index));
>> +
>> +	/*
>> +	 * Update symtab's sh_addr to point to a valid
>> +	 * symbol table, as the temporary symtab in module
>> +	 * init memory will be freed
>> +	 */
>> +	symsect = mod->sechdrs + mod->index.sym;
>> +	symsect->sh_addr = (unsigned long)mod->core_symtab;
>> +
>> +	return ret;
>> +
>> +free_sechdrs:
>> +	kfree(mod->sechdrs);
>> +free_hdr:
>> +	kfree(mod->hdr);
>> +out:
>> +	return ret;
>> +}
>> +
>> +
>>  #define COPY_CHUNK_SIZE (16*PAGE_SIZE)
>>
>>  static int copy_chunked_from_user(void *dst, const void __user *usrc, unsigned long len)
>> @@ -2866,6 +2947,9 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
>>  			"is unknown, you have been warned.\n", mod->name);
>>  	}
>>
>> +	if (get_modinfo(info, "livepatch"))
>> +		mod->klp = true;
>> +
>
>Similar to the is_livepatch_module() function I suggested, this can be
>put in a function so that mod->klp can be abstracted away for the
>!LIVEPATCH case.  Maybe there should be a check_livepatch_modinfo()
>function:
>
>1. the !LIVEPATCH version of the function could return an error if
>modinfo has "livepatch"
>
>2. the LIVEPATCH version could simply set mod->klp = true.
>
>>  	/* Set up license info based on the info section */
>>  	set_license(mod, get_modinfo(info, "license"));
>>
>> @@ -3530,6 +3614,16 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>  	if (err < 0)
>>  		goto bug_cleanup;
>>
>> +	/*
>> +	 * Save sechdrs, indices, and other data from info
>> +	 * in order to patch to-be-loaded modules.
>> +	 * Do not call free_copy() for livepatch modules.
>
>I think the last line of this comment isn't right, since free_copy() is
>called below regardless.

Yes you're right, forgot to update that comment.

>> +	 */
>> +	if (mod->klp)
>> +		err = copy_module_elf(mod, info);
>> +	if (err < 0)
>> +		goto bug_cleanup;
>
>Not strictly necessary, but I think it would be a little cleaner to only
>check the err if copy_module_elf() was called.
>
>> +
>>  	/* Get rid of temporary copy. */
>>  	free_copy(info);
>
>-- 
>Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288537 — Re: module: preserve Elf information for livepatch modules

On Wed, Dec 09, 2015 at 03:05:23PM -0500, Jessica Yu wrote:
> +++ Josh Poimboeuf [08/12/15 12:32 -0600]:
> >On Mon, Nov 30, 2015 at 11:21:15PM -0500, Jessica Yu wrote:
> >>For livepatch modules, copy Elf section, symbol, and string information
> >>from the load_info struct in the module loader.
> >>
> >>Livepatch uses special relocation sections in order to be able to patch
> >>modules that are not yet loaded, as well as apply patches to the kernel
> >>when the addresses of symbols cannot be determined at compile time (for
> >>example, when kaslr is enabled). Livepatch modules must preserve Elf
> >>information such as section indices in order to apply the remaining
> >>relocation sections at the appropriate time (i.e. when the target module
> >>loads).
> >>
> >>Signed-off-by: Jessica Yu <jeyu@redhat.com>
> >>---
> >> include/linux/module.h |  9 +++++
> >> kernel/module.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >> 2 files changed, 105 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/include/linux/module.h b/include/linux/module.h
> >>index 3a19c79..9b46256 100644
> >>--- a/include/linux/module.h
> >>+++ b/include/linux/module.h
> >>@@ -425,6 +425,14 @@ struct module {
> >>
> >> 	/* Notes attributes */
> >> 	struct module_notes_attrs *notes_attrs;
> >>+
> >>+	/* Elf information (optionally saved) */
> >>+	Elf_Ehdr *hdr;
> >
> >I would rename "hdr" to "elf_hdr" to make its purpose clearer.
> >
> >>+	Elf_Shdr *sechdrs;
> >>+	char *secstrings;
> >
> >Probably a good idea to add underscores to the names ("sec_hdrs" and
> >"sec_strings") to be consistent with most of the other fields in the
> >struct.
> >
> >>+	struct {
> >>+		unsigned int sym, str, mod, vers, info, pcpu;
> >>+	} index;
> >
> >I might be contradicting myself from what I said before.  But I'm
> >thinking we should put all these fields inside a CONFIG_LIVEPATCH ifdef.
> >Then below, there could be two versions of copy_module_elf(), the real
> >one for LIVEPATCH and and an empty one for !LIVEPATCH.  And the same
> >story for free_module_elf().
> 
> I think in the v1 discussion we were leaning more towards making this
> generic. We could potentially just have the Elf module fields
> available in the generic case, independent of whether CONFIG_LIVEPATCH
> is set, whereas the mod->klp field should probably be only available
> when LIVEPATCH is set. I think this makes sense since the Elf fields
> aren't dependent on livepatch (although livepatch would be the only
> user of these fields at the moment). I don't know if there would be
> any users in the future that would be interested in using this Elf
> information. Thoughts on this?

IIRC, I think I made the suggestion to always save the elf fields
because otherwise it was looking like we were going to need a lot of
spaghetti code.

But if we can find a way to wrap the elf fields in LIVEPATCH while
keeping the code simple, then there's no real downside and I think
that's the way to go.  If somebody else wants to use the fields later,
then they can remove or change the ifdefs as needed.

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280569 — [RFC PATCH v2 6/6] Documentation: livepatch: outline the Elf format of a livepatch module

Document the special Elf sections and constants livepatch modules use.

Signed-off-by: Jessica Yu <jeyu@redhat.com>
---
 Documentation/livepatch/patch-module-format.txt | 117 ++++++++++++++++++++++++
 1 file changed, 117 insertions(+)
 create mode 100644 Documentation/livepatch/patch-module-format.txt

diff --git a/Documentation/livepatch/patch-module-format.txt b/Documentation/livepatch/patch-module-format.txt
new file mode 100644
index 0000000..6118e5d
--- /dev/null
+++ b/Documentation/livepatch/patch-module-format.txt
@@ -0,0 +1,116 @@
+---------------------------
+Livepatch module Elf format
+---------------------------
+
+This document outlines the special Elf constants and sections livepatch
+uses to patch both modules and the kernel (vmlinux).
+
+--------------------------
+1. Livepatch modinfo field
+--------------------------
+
+Livepatch modules can be identified by users by using the 'modinfo' command
+and looking for the presence of the "livepatch" field. This field is also
+used by the kernel module loader to identify livepatch modules.
+
+Example modinfo output:
+
+% modinfo kpatch-meminfo.ko
+filename:		kpatch-meminfo.ko
+livepatch:		Y
+license:		GPL
+depends:
+vermagic:		4.3.0+ SMP mod_unload
+
+--------------------
+2. Livepatch symbols
+--------------------
+
+These are symbols marked with SHN_LIVEPATCH and are manually resolved by
+livepatch. This is useful in cases where we cannot immediately know the
+address of a symbol because the to-be-patched module is not loaded yet.
+livepatch modules keep these symbols in their original symbol tables, and
+the symbol table is made accessible through module->core_symtab.
+
+-----------------------------------
+3. Livepatch "external" symbol bind
+-----------------------------------
+
+The STB_LIVEPATCH_EXT symbol bind is used to help livepatch resolve global
+symbols referenced by klp relocations. After the module is copied into
+memory the module loader actually overwrites each symbol's bind with a
+character (see add_kallsyms() in kernel/module.c), so STB_LIVEPATCH_EXT
+symbols are represented with a capital 'K'.
+
+-----------------------------------
+4. "__klp_rela" relocation sections
+-----------------------------------
+
+A livepatch module uses special Elf relocation sections to apply
+relocations both for regular vmlinux patches as well as those that should
+be applied as soon as the to-be-patched module is loaded. For example, if a
+patch module patches a driver that is not currently loaded, livepatch will
+apply its corresponding klp relocation section(s) to the driver once it
+loads.
+
+The names of these livepatch relocation sections are formatted
+"__klp_rela_${objname}", where ${objname} is the name of the "object" being
+patched (e.g. vmlinux or name of module). Each object within a patch module
+may have multiple klp sections (e.g. patches to multiple functions within
+the same object). There is a 1-1 correspondence between a klp relocation
+section and the target section (usually the text section for a function) to
+which the relocation(s) apply.
+
+Here's a sample readelf output for a livepatch module that patches vmlinux and
+modules 9p, btrfs, ext4:
+  ...
+  [29] __klp_rela_9p.text.caches_show RELA                    0000000000000000 002d58 0000c0 18 AIo 64   9  8
+  [30] __klp_rela_btrfs.text.btrfs_feature_attr_show RELA     0000000000000000 002e18 000060 18 AIo 64  11  8
+  ...
+  [34] __klp_rela_ext4.text.ext4_attr_store RELA              0000000000000000 002fd8 0000d8 18 AIo 64  13  8
+  [35] __klp_rela_ext4.text.ext4_attr_show RELA               0000000000000000 0030b0 000150 18 AIo 64  15  8
+  [36] __klp_rela_vmlinux.text.cmdline_proc_show RELA         0000000000000000 003200 000018 18 AIo 64  17  8
+  [37] __klp_rela_vmlinux.text.meminfo_proc_show RELA         0000000000000000 003218 0000f0 18 AIo 64  19  8
+  ...
+
+klp relocation sections are SHT_RELA sections but with a few special
+characteristics. Notice that they are marked SHF_ALLOC ("A") so that they
+will not be discarded when the module is loaded into memory, as well as
+with the SHF_RELA_LIVEPATCH flag ("o" - for OS-specific) so the module
+loader can identify them and avoid treating them as regular SHT_RELA
+sections, since they are manually managed by livepatch.
+
+---------------------------------------------------------------
+4.1 How klp relocation sections are represented and initialized
+---------------------------------------------------------------
+
+Livepatch modules must initialize a klp_patch structure to pass in to
+klp_register_patch(). A klp_patch struct contains an array of klp_objects,
+and each klp_object contains an array of klp_reloc_sec structs that
+represent the klp relocation sections that must be applied to that object.
+Each klp_reloc_sec struct is allocated and initialized by the patch module
+code before the call to klp_register_patch().
+
+The klp_reloc_sec structure contains useful metadata about a klp relocation
+section such as its section index and name. Since Elf information is
+preserved for livepatch modules (see Section 5), a klp relocation section
+can be applied simply by passing in the saved section index to
+apply_relocate_add() (in the module loader code), which then uses it to
+access the actual Elf relocation section and apply the relocations.
+
+--------------------------------------------------------
+5. How a livepatch module accesses its symbol table and
+its klp relocation sections
+--------------------------------------------------------
+
+The kernel module loader checks whether the module being loaded is a
+livepatch module. If so, it then makes a copy of the module's Elf header,
+section headers, section name string table, and some noteworthy section
+indices (for example, the symtab's section index). It adjusts the symtab's
+sh_addr to point to mod->core_symtab, since the original mod->symtab lies
+in init memory and gets freed once the module finishes initializing. For
+livepatch modules, the core_symtab will be an exact copy of its original
+symbol table (where normally, only "core" symbols are included in this
+symbol table. See is_core_symbol() in kernel/module.c). livepatch requires
+that the symbols retain their original indices in the symbol table so that
+the klp relocation sections can be applied correctly.
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web