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

Groups > linux.kernel > #1266222 > unrolled thread

[RFC PATCH 0/5] Arch-independent livepatch

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/5] Arch-independent livepatch Jessica Yu <jeyu@redhat.com> - 2015-11-10 05:50 +0100
    [RFC PATCH 2/5] module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-10 05:50 +0100
      Re: [RFC PATCH 2/5] module: save load_info for livepatch modules Minfei Huang <mnfhuang@gmail.com> - 2015-11-11 09:10 +0100
      Re: [RFC PATCH 2/5] module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-11 15:20 +0100
        Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-12 06:40 +0100
          Re: module: save load_info for livepatch modules Petr Mladek <pmladek@suse.com> - 2015-11-12 11:30 +0100
            Re: module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-12 14:30 +0100
              Re: module: save load_info for livepatch modules Petr Mladek <pmladek@suse.com> - 2015-11-12 16:10 +0100
                Re: module: save load_info for livepatch modules Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 18:10 +0100
                  Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-12 23:20 +0100
                    Re: module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-13 13:30 +0100
                      Re: module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-13 13:50 +0100
                        Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-14 01:40 +0100
                      Re: module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-13 14:00 +0100
                        Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-14 03:20 +0100
              Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-13 01:30 +0100
      Re: [RFC PATCH 2/5] module: save load_info for livepatch modules Petr Mladek <pmladek@suse.com> - 2015-11-11 15:40 +0100
        Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-12 05:50 +0100
          Re: module: save load_info for livepatch modules Petr Mladek <pmladek@suse.com> - 2015-11-12 11:10 +0100
            Re: module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-12 15:20 +0100
              Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-13 07:40 +0100
                Re: module: save load_info for livepatch modules Miroslav Benes <mbenes@suse.cz> - 2015-11-13 14:10 +0100
            Re: module: save load_info for livepatch modules Jessica Yu <jeyu@redhat.com> - 2015-11-13 09:30 +0100
      Re: [RFC PATCH 2/5] module: save load_info for livepatch modules Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 18:20 +0100
      Re: [RFC PATCH 2/5] module: save load_info for livepatch modules Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 18:30 +0100
    [RFC PATCH 1/5] elf: add livepatch-specific elf constants Jessica Yu <jeyu@redhat.com> - 2015-11-10 05:50 +0100
      Re: [RFC PATCH 1/5] elf: add livepatch-specific elf constants Petr Mladek <pmladek@suse.com> - 2015-11-11 15:00 +0100
      Re: [RFC PATCH 1/5] elf: add livepatch-specific elf constants Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 16:40 +0100
      Re: [RFC PATCH 1/5] elf: add livepatch-specific elf constants Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 16:50 +0100
        Re: elf: add livepatch-specific elf constants Jessica Yu <jeyu@redhat.com> - 2015-11-13 08:00 +0100
    [RFC PATCH 4/5] samples: livepatch: init reloc list and mark as klp module Jessica Yu <jeyu@redhat.com> - 2015-11-10 05:50 +0100
      Re: [RFC PATCH 4/5] samples: livepatch: init reloc list and mark as  klp module Jiri Slaby <jslaby@suse.cz> - 2015-11-10 09:20 +0100
        Re: [RFC PATCH 4/5] samples: livepatch: init reloc list and mark as  klp module Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-10 15:00 +0100
          Re: samples: livepatch: init reloc list and mark as klp module Jessica Yu <jeyu@redhat.com> - 2015-11-10 19:40 +0100
      Re: [RFC PATCH 4/5] samples: livepatch: init reloc list and mark as  klp module Petr Mladek <pmladek@suse.com> - 2015-11-11 16:50 +0100
        Re: samples: livepatch: init reloc list and mark as klp module Jessica Yu <jeyu@redhat.com> - 2015-11-12 07:10 +0100
          Re: samples: livepatch: init reloc list and mark as klp module Miroslav Benes <mbenes@suse.cz> - 2015-11-12 11:50 +0100
    [RFC PATCH 5/5] livepatch: x86: remove unused relocation code Jessica Yu <jeyu@redhat.com> - 2015-11-10 05:50 +0100
      Re: [RFC PATCH 5/5] livepatch: x86: remove unused relocation code Petr Mladek <pmladek@suse.com> - 2015-11-11 16:50 +0100
        Re: [RFC PATCH 5/5] livepatch: x86: remove unused relocation code Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 19:10 +0100
    [RFC PATCH 3/5] livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-10 05:50 +0100
      Re: [RFC PATCH 3/5] livepatch: reuse module loader code to write  relocations Jiri Slaby <jslaby@suse.cz> - 2015-11-10 09:20 +0100
      Re: [RFC PATCH 3/5] livepatch: reuse module loader code to write  relocations Miroslav Benes <mbenes@suse.cz> - 2015-11-11 15:40 +0100
        Re: livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-11 21:10 +0100
          Re: livepatch: reuse module loader code to write relocations Miroslav Benes <mbenes@suse.cz> - 2015-11-12 16:30 +0100
            Re: livepatch: reuse module loader code to write relocations Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 18:50 +0100
              Re: livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-12 21:30 +0100
                Re: livepatch: reuse module loader code to write relocations Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 21:40 +0100
                  Re: livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-13 08:20 +0100
                  Re: livepatch: reuse module loader code to write relocations Miroslav Benes <mbenes@suse.cz> - 2015-11-13 15:00 +0100
            Re: livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-12 20:20 +0100
              Re: livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-12 21:40 +0100
      Re: [RFC PATCH 3/5] livepatch: reuse module loader code to write  relocations Petr Mladek <pmladek@suse.com> - 2015-11-11 16:30 +0100
        Re: livepatch: reuse module loader code to write relocations Jessica Yu <jeyu@redhat.com> - 2015-11-11 19:30 +0100
          Re: livepatch: reuse module loader code to write relocations Petr Mladek <pmladek@suse.com> - 2015-11-12 10:20 +0100
      Re: [RFC PATCH 3/5] livepatch: reuse module loader code to write  relocations Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 19:00 +0100
    Re: [RFC PATCH 0/5] Arch-independent livepatch Miroslav Benes <mbenes@suse.cz> - 2015-11-11 15:10 +0100
      Re: [RFC PATCH 0/5] Arch-independent livepatch Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-11 17:30 +0100

Page 1 of 3  [1] 2 3  Next page →

#1266222 — [RFC PATCH 0/5] 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 resolve 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 this struct for livepatch modules and it is
made available through module->info.

The ELF-related changes enable livepatch to patch modules that are not
loaded yet. 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 correspond to the modules being patched.
They are marked with a special SHF_RELA_LIVEPATCH section flag to indicate
to the module loader that it should ignore that reloc section and that
livepatch will handle them. The SHN_LIVEPATCH shndx marks symbols that will
have 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.

Jessica Yu (5):
  elf: add livepatch-specific elf constants
  module: save load_info for livepatch modules
  livepatch: reuse module loader code to write relocations
  samples: livepatch: init reloc list and mark as klp module
  livepatch: x86: remove unused relocation code

 arch/x86/kernel/Makefile             |   1 -
 arch/x86/kernel/livepatch.c          |  91 ------------------------------
 include/linux/livepatch.h            |  11 +++-
 include/linux/module.h               |  31 ++++++++++
 include/uapi/linux/elf.h             |   3 +
 kernel/livepatch/core.c              | 106 ++++++++++++++++++++++++-----------
 kernel/module.c                      |  36 +++++++-----
 samples/livepatch/livepatch-sample.c |   2 +
 8 files changed, 139 insertions(+), 142 deletions(-)
 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]

#1266224 — [RFC PATCH 2/5] module: save load_info for livepatch modules

In livepatch modules, preserve section, symbol, string information from
the load_info struct in the module loader. This information is used to
patch modules that are not loaded in memory yet; specifically it is used
to resolve remaining symbols and write relocations when the target
module loads.

Signed-off-by: Jessica Yu <jeyu@redhat.com>
---
 include/linux/module.h  | 25 +++++++++++++++++++++++++
 kernel/livepatch/core.c | 17 +++++++++++++++++
 kernel/module.c         | 36 ++++++++++++++++++++++--------------
 3 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 3a19c79..c8680b1 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -36,6 +36,20 @@ struct modversion_info {
 	char name[MODULE_NAME_LEN];
 };
 
+struct load_info {
+	Elf_Ehdr *hdr;
+	unsigned long len;
+	Elf_Shdr *sechdrs;
+	char *secstrings, *strtab;
+	unsigned long symoffs, stroffs;
+	struct _ddebug *debug;
+	unsigned int num_debug;
+	bool sig_ok;
+	struct {
+		unsigned int sym, str, mod, vers, info, pcpu;
+	} index;
+};
+
 struct module;
 
 struct module_kobject {
@@ -462,6 +476,8 @@ struct module {
 
 #ifdef CONFIG_LIVEPATCH
 	bool klp_alive;
+	/* save info to patch to-be-loaded modules */
+	struct load_info *info;
 #endif
 
 #ifdef CONFIG_MODULE_UNLOAD
@@ -635,6 +651,15 @@ static inline bool module_requested_async_probing(struct module *module)
 	return module && module->async_probe_requested;
 }
 
+#ifdef CONFIG_LIVEPATCH
+extern void klp_prepare_patch_module(struct module *mod,
+				     struct load_info *info);
+extern int
+apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
+		   unsigned int symindex, unsigned int relsec,
+		   struct module *me);
+#endif
+
 #else /* !CONFIG_MODULES... */
 
 /* Given an address, look for it in the exception tables. */
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 6e53441..087a8c7 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
 };
 
+/*
+ * Save necessary information from info in order to be able to
+ * patch modules that might be loaded later
+ */
+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
+{
+	Elf_Shdr *symsect;
+
+	symsect = info->sechdrs + info->index.sym;
+	/* update sh_addr to point to symtab */
+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
+
+	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
+	memcpy(mod->info, info, sizeof(*info));
+
+}
+
 static int __init klp_init(void)
 {
 	int ret;
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..8ae3ca5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -318,20 +318,6 @@ int unregister_module_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_module_notifier);
 
-struct load_info {
-	Elf_Ehdr *hdr;
-	unsigned long len;
-	Elf_Shdr *sechdrs;
-	char *secstrings, *strtab;
-	unsigned long symoffs, stroffs;
-	struct _ddebug *debug;
-	unsigned int num_debug;
-	bool sig_ok;
-	struct {
-		unsigned int sym, str, mod, vers, info, pcpu;
-	} index;
-};
-
 /* We require a truly strong try_module_get(): 0 means failure due to
    ongoing or failed initialization etc. */
 static inline int strong_try_module_get(struct module *mod)
@@ -2137,6 +2123,11 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
 			       (long)sym[i].st_value);
 			break;
 
+#ifdef CONFIG_LIVEPATCH
+		case SHN_LIVEPATCH:
+			break;
+#endif
+
 		case SHN_UNDEF:
 			ksym = resolve_symbol_wait(mod, info, name);
 			/* Ok if resolved.  */
@@ -2185,6 +2176,11 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
 		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
 			continue;
 
+#ifdef CONFIG_LIVEPATCH
+		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
+			continue;
+#endif
+
 		if (info->sechdrs[i].sh_type == SHT_REL)
 			err = apply_relocate(info->sechdrs, info->strtab,
 					     info->index.sym, i, mod);
@@ -3530,8 +3526,20 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err < 0)
 		goto bug_cleanup;
 
+#ifdef CONFIG_LIVEPATCH
+	/*
+	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
+		klp_prepare_patch_module(mod, info);
+	else
+		free_copy(info);
+#else
 	/* Get rid of temporary copy. */
 	free_copy(info);
+#endif
 
 	/* Done! */
 	trace_module_load(mod);
-- 
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]

#1267028 — Re: [RFC PATCH 2/5] module: save load_info for livepatch modules

On 11/09/15 at 11:45pm, Jessica Yu wrote:
> In livepatch modules, preserve section, symbol, string information from
> the load_info struct in the module loader. This information is used to
> patch modules that are not loaded in memory yet; specifically it is used
> to resolve remaining symbols and write relocations when the target
> module loads.
> 
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> ---
>  include/linux/module.h  | 25 +++++++++++++++++++++++++
>  kernel/livepatch/core.c | 17 +++++++++++++++++
>  kernel/module.c         | 36 ++++++++++++++++++++++--------------
>  3 files changed, 64 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3a19c79..c8680b1 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -36,6 +36,20 @@ struct modversion_info {
>  	char name[MODULE_NAME_LEN];
>  };
>  
> +struct load_info {
> +	Elf_Ehdr *hdr;
> +	unsigned long len;
> +	Elf_Shdr *sechdrs;
> +	char *secstrings, *strtab;
> +	unsigned long symoffs, stroffs;
> +	struct _ddebug *debug;
> +	unsigned int num_debug;
> +	bool sig_ok;
> +	struct {
> +		unsigned int sym, str, mod, vers, info, pcpu;
> +	} index;
> +};
> +
>  struct module;
>  
>  struct module_kobject {
> @@ -462,6 +476,8 @@ struct module {
>  
>  #ifdef CONFIG_LIVEPATCH
>  	bool klp_alive;
> +	/* save info to patch to-be-loaded modules */
> +	struct load_info *info;
>  #endif
>  
>  #ifdef CONFIG_MODULE_UNLOAD
> @@ -635,6 +651,15 @@ static inline bool module_requested_async_probing(struct module *module)
>  	return module && module->async_probe_requested;
>  }
>  
> +#ifdef CONFIG_LIVEPATCH
> +extern void klp_prepare_patch_module(struct module *mod,
> +				     struct load_info *info);
> +extern int
> +apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
> +		   unsigned int symindex, unsigned int relsec,
> +		   struct module *me);
> +#endif
> +
>  #else /* !CONFIG_MODULES... */
>  
>  /* Given an address, look for it in the exception tables. */
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 6e53441..087a8c7 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
>  	.priority = INT_MIN+1, /* called late but before ftrace notifier */
>  };
>  
> +/*
> + * Save necessary information from info in order to be able to
> + * patch modules that might be loaded later
> + */
> +void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> +{
> +	Elf_Shdr *symsect;
> +
> +	symsect = info->sechdrs + info->index.sym;
> +	/* update sh_addr to point to symtab */
> +	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
> +
> +	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);

We should test the value of mod->info, since kernel may fail to allocate
the memory.

Thanks
Minfei

> +	memcpy(mod->info, info, sizeof(*info));
> +
> +}
> +
>  static int __init klp_init(void)
>  {
>  	int ret;
> diff --git a/kernel/module.c b/kernel/module.c
> index 8f051a1..8ae3ca5 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -318,20 +318,6 @@ int unregister_module_notifier(struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL(unregister_module_notifier);
>  
> -struct load_info {
> -	Elf_Ehdr *hdr;
> -	unsigned long len;
> -	Elf_Shdr *sechdrs;
> -	char *secstrings, *strtab;
> -	unsigned long symoffs, stroffs;
> -	struct _ddebug *debug;
> -	unsigned int num_debug;
> -	bool sig_ok;
> -	struct {
> -		unsigned int sym, str, mod, vers, info, pcpu;
> -	} index;
> -};
> -
>  /* We require a truly strong try_module_get(): 0 means failure due to
>     ongoing or failed initialization etc. */
>  static inline int strong_try_module_get(struct module *mod)
> @@ -2137,6 +2123,11 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
>  			       (long)sym[i].st_value);
>  			break;
>  
> +#ifdef CONFIG_LIVEPATCH
> +		case SHN_LIVEPATCH:
> +			break;
> +#endif
> +
>  		case SHN_UNDEF:
>  			ksym = resolve_symbol_wait(mod, info, name);
>  			/* Ok if resolved.  */
> @@ -2185,6 +2176,11 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
>  		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
>  			continue;
>  
> +#ifdef CONFIG_LIVEPATCH
> +		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
> +			continue;
> +#endif
> +
>  		if (info->sechdrs[i].sh_type == SHT_REL)
>  			err = apply_relocate(info->sechdrs, info->strtab,
>  					     info->index.sym, i, mod);
> @@ -3530,8 +3526,20 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  	if (err < 0)
>  		goto bug_cleanup;
>  
> +#ifdef CONFIG_LIVEPATCH
> +	/*
> +	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
> +		klp_prepare_patch_module(mod, info);
> +	else
> +		free_copy(info);
> +#else
>  	/* Get rid of temporary copy. */
>  	free_copy(info);
> +#endif
>  
>  	/* Done! */
>  	trace_module_load(mod);
> -- 
> 2.4.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe live-patching" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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]

#1267211 — Re: [RFC PATCH 2/5] module: save load_info for livepatch modules

On Mon, 9 Nov 2015, Jessica Yu wrote:

> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3a19c79..c8680b1 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h

[...]

> +#ifdef CONFIG_LIVEPATCH
> +extern void klp_prepare_patch_module(struct module *mod,
> +				     struct load_info *info);
> +extern int
> +apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
> +		   unsigned int symindex, unsigned int relsec,
> +		   struct module *me);
> +#endif
> +
>  #else /* !CONFIG_MODULES... */

apply_relocate_add() is already in include/linux/moduleloader.h (guarded 
by CONFIG_MODULES_USE_ELF_RELA), so maybe we can just include that where 
we need it. As for the klp_prepare_patch_module() wouldn't it be better to 
have it in our livepatch.h and include that in kernel/module.c?

>  /* Given an address, look for it in the exception tables. */
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 6e53441..087a8c7 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
>  	.priority = INT_MIN+1, /* called late but before ftrace notifier */
>  };
>  
> +/*
> + * Save necessary information from info in order to be able to
> + * patch modules that might be loaded later
> + */
> +void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> +{
> +	Elf_Shdr *symsect;
> +
> +	symsect = info->sechdrs + info->index.sym;
> +	/* update sh_addr to point to symtab */
> +	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
> +
> +	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	memcpy(mod->info, info, sizeof(*info));
> +
> +}

What about arch-specific 'struct mod_arch_specific'? We need to preserve 
it somewhere as well for s390x and other non-x86 architectures.

> +#ifdef CONFIG_LIVEPATCH
> +	/*
> +	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
> +		klp_prepare_patch_module(mod, info);
> +	else
> +		free_copy(info);
> +#else
>  	/* Get rid of temporary copy. */
>  	free_copy(info);
> +#endif

Maybe I am missing something but isn't it necessary to call vfree() on 
info somewhere in the end?

Regards,
Miroslav
--
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]

#1267645 — Re: module: save load_info for livepatch modules

+++ Miroslav Benes [11/11/15 15:17 +0100]:
>On Mon, 9 Nov 2015, Jessica Yu wrote:
>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 3a19c79..c8680b1 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>
>[...]
>
>> +#ifdef CONFIG_LIVEPATCH
>> +extern void klp_prepare_patch_module(struct module *mod,
>> +				     struct load_info *info);
>> +extern int
>> +apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
>> +		   unsigned int symindex, unsigned int relsec,
>> +		   struct module *me);
>> +#endif
>> +
>>  #else /* !CONFIG_MODULES... */
>
>apply_relocate_add() is already in include/linux/moduleloader.h (guarded
>by CONFIG_MODULES_USE_ELF_RELA), so maybe we can just include that where
>we need it. As for the klp_prepare_patch_module() wouldn't it be better to
>have it in our livepatch.h and include that in kernel/module.c?

Yeah, Petr pointed this out as well :-) I will just include
moduleloader.h for the apply_relocate_add() declaration.

It also looks like we have some disagreement over where to put
klp_prepare_patch_module(), either in livepatch/core.c (and add the
function declaration in livepatch.h, and have module.c include
livepatch.h) or in kernel/module.c, keeping the
klp_prepare_patch_module() declaration in module.h. Maybe Rusty can
provide some input.

>>  /* Given an address, look for it in the exception tables. */
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index 6e53441..087a8c7 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
>>  	.priority = INT_MIN+1, /* called late but before ftrace notifier */
>>  };
>>
>> +/*
>> + * Save necessary information from info in order to be able to
>> + * patch modules that might be loaded later
>> + */
>> +void klp_prepare_patch_module(struct module *mod, struct load_info *info)
>> +{
>> +	Elf_Shdr *symsect;
>> +
>> +	symsect = info->sechdrs + info->index.sym;
>> +	/* update sh_addr to point to symtab */
>> +	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
>> +
>> +	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +	memcpy(mod->info, info, sizeof(*info));
>> +
>> +}
>
>What about arch-specific 'struct mod_arch_specific'? We need to preserve
>it somewhere as well for s390x and other non-x86 architectures.

Ah! Thank you for catching this, I overlooked this important detail.
Yes, we do need to save the arch-specific struct. We would be in
trouble for s390 relocs if we didn't. I am trying to think of a way to
save this information for s390, since s390's module_finalize() frees
mod->arch.syminfo, which we definitely need in order for the call to
apply_relocate_add() to work. Maybe we can add an extra call right
before module_finalize() that will do some livepatch-specific
processing and copy this information (this would be in
post_relocation() in kernel/module.c). Perhaps this patchset cannot be
entirely free of arch-specific code after all :-( Still thinking.

>> +#ifdef CONFIG_LIVEPATCH
>> +	/*
>> +	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
>> +		klp_prepare_patch_module(mod, info);
>> +	else
>> +		free_copy(info);
>> +#else
>>  	/* Get rid of temporary copy. */
>>  	free_copy(info);
>> +#endif
>
>Maybe I am missing something but isn't it necessary to call vfree() on
>info somewhere in the end?

So free_copy() will call vfree(info->hdr), except in livepatch modules
we want to keep all the elf section information stored there, so we
avoid calling free_copy(), As for the info struct itself, if you look
at the init_module and finit_module syscall definitions in
kernel/module.c, you will see that info is actually a local function
variable, simply passed in to the call to load_module(), and will be
automatically deallocated when the syscall returns. :-) No need to
explicitly free info.

Thanks for the comments,
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]

#1267795 — Re: module: save load_info for livepatch modules

On Thu 2015-11-12 00:33:12, Jessica Yu wrote:
> +++ Miroslav Benes [11/11/15 15:17 +0100]:
> >On Mon, 9 Nov 2015, Jessica Yu wrote:
> >
> >>diff --git a/include/linux/module.h b/include/linux/module.h
> >>index 3a19c79..c8680b1 100644
> >>--- a/include/linux/module.h
> >>+++ b/include/linux/module.h
> >
> >[...]
> >
> >>+#ifdef CONFIG_LIVEPATCH
> >>+extern void klp_prepare_patch_module(struct module *mod,
> >>+				     struct load_info *info);
> >>+extern int
> >>+apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
> >>+		   unsigned int symindex, unsigned int relsec,
> >>+		   struct module *me);
> >>+#endif
> >>+
> >> #else /* !CONFIG_MODULES... */
> >
> >apply_relocate_add() is already in include/linux/moduleloader.h (guarded
> >by CONFIG_MODULES_USE_ELF_RELA), so maybe we can just include that where
> >we need it. As for the klp_prepare_patch_module() wouldn't it be better to
> >have it in our livepatch.h and include that in kernel/module.c?
> 
> Yeah, Petr pointed this out as well :-) I will just include
> moduleloader.h for the apply_relocate_add() declaration.
> 
> It also looks like we have some disagreement over where to put
> klp_prepare_patch_module(), either in livepatch/core.c (and add the
> function declaration in livepatch.h, and have module.c include
> livepatch.h) or in kernel/module.c, keeping the
> klp_prepare_patch_module() declaration in module.h. Maybe Rusty can
> provide some input.
> 
> >> /* Given an address, look for it in the exception tables. */
> >>diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> >>index 6e53441..087a8c7 100644
> >>--- a/kernel/livepatch/core.c
> >>+++ b/kernel/livepatch/core.c
> >>@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
> >> 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
> >> };
> >>
> >>+/*
> >>+ * Save necessary information from info in order to be able to
> >>+ * patch modules that might be loaded later
> >>+ */
> >>+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> >>+{
> >>+	Elf_Shdr *symsect;
> >>+
> >>+	symsect = info->sechdrs + info->index.sym;
> >>+	/* update sh_addr to point to symtab */
> >>+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
> >>+
> >>+	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
> >>+	memcpy(mod->info, info, sizeof(*info));
> >>+
> >>+}
> >
> >What about arch-specific 'struct mod_arch_specific'? We need to preserve
> >it somewhere as well for s390x and other non-x86 architectures.
> 
> Ah! Thank you for catching this, I overlooked this important detail.
> Yes, we do need to save the arch-specific struct. We would be in
> trouble for s390 relocs if we didn't. I am trying to think of a way to
> save this information for s390, since s390's module_finalize() frees
> mod->arch.syminfo, which we definitely need in order for the call to
> apply_relocate_add() to work. Maybe we can add an extra call right
> before module_finalize() that will do some livepatch-specific
> processing and copy this information (this would be in
> post_relocation() in kernel/module.c). Perhaps this patchset cannot be
> entirely free of arch-specific code after all :-( Still thinking.

I think about adding a flag somewhere, e.g. mod->preserve_relocs.
It might be set in simplify_symbols() when SHN_LIVEPATCH is found.
It might be checked when freeing the needed structures in both
the generic and arch-specific code.

> >>+#ifdef CONFIG_LIVEPATCH
> >>+	/*
> >>+	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
> >>+		klp_prepare_patch_module(mod, info);
> >>+	else
> >>+		free_copy(info);
> >>+#else
> >> 	/* Get rid of temporary copy. */
> >> 	free_copy(info);
> >>+#endif
> >
> >Maybe I am missing something but isn't it necessary to call vfree() on
> >info somewhere in the end?
> 
> So free_copy() will call vfree(info->hdr), except in livepatch modules
> we want to keep all the elf section information stored there, so we
> avoid calling free_copy(), As for the info struct itself, if you look
> at the init_module and finit_module syscall definitions in
> kernel/module.c, you will see that info is actually a local function
> variable, simply passed in to the call to load_module(), and will be
> automatically deallocated when the syscall returns. :-) No need to
> explicitly free info.

We still have to free the copied or preserved structures when
the module is unloaded.

Thank you,
Petr
--
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]

#1267909 — Re: module: save load_info for livepatch modules

On Thu, 12 Nov 2015, Petr Mladek wrote:

> On Thu 2015-11-12 00:33:12, Jessica Yu wrote:
> > +++ Miroslav Benes [11/11/15 15:17 +0100]:
> > >On Mon, 9 Nov 2015, Jessica Yu wrote:
> > >
> > >>diff --git a/include/linux/module.h b/include/linux/module.h
> > >>index 3a19c79..c8680b1 100644
> > >>--- a/include/linux/module.h
> > >>+++ b/include/linux/module.h
> > >
> > >[...]
> > >
> > >>+#ifdef CONFIG_LIVEPATCH
> > >>+extern void klp_prepare_patch_module(struct module *mod,
> > >>+				     struct load_info *info);
> > >>+extern int
> > >>+apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
> > >>+		   unsigned int symindex, unsigned int relsec,
> > >>+		   struct module *me);
> > >>+#endif
> > >>+
> > >> #else /* !CONFIG_MODULES... */
> > >
> > >apply_relocate_add() is already in include/linux/moduleloader.h (guarded
> > >by CONFIG_MODULES_USE_ELF_RELA), so maybe we can just include that where
> > >we need it. As for the klp_prepare_patch_module() wouldn't it be better to
> > >have it in our livepatch.h and include that in kernel/module.c?
> > 
> > Yeah, Petr pointed this out as well :-) I will just include
> > moduleloader.h for the apply_relocate_add() declaration.
> > 
> > It also looks like we have some disagreement over where to put
> > klp_prepare_patch_module(), either in livepatch/core.c (and add the
> > function declaration in livepatch.h, and have module.c include
> > livepatch.h) or in kernel/module.c, keeping the
> > klp_prepare_patch_module() declaration in module.h. Maybe Rusty can
> > provide some input.

Yes, there are several ways how to do it. Maybe the best would be some 
generic way in kernel/module.c. I am not sure if there will be another 
user of this in the future but nevertheless. It would also allow us to 
somehow solve the issues mentioned below. Thus, klp_prepare_patch_module 
is inappropriate name and it should be for example just preserve_load_info 
(or more general if needed) and it should be in kernel/module.c.

> > >> /* Given an address, look for it in the exception tables. */
> > >>diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > >>index 6e53441..087a8c7 100644
> > >>--- a/kernel/livepatch/core.c
> > >>+++ b/kernel/livepatch/core.c
> > >>@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
> > >> 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
> > >> };
> > >>
> > >>+/*
> > >>+ * Save necessary information from info in order to be able to
> > >>+ * patch modules that might be loaded later
> > >>+ */
> > >>+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> > >>+{
> > >>+	Elf_Shdr *symsect;
> > >>+
> > >>+	symsect = info->sechdrs + info->index.sym;
> > >>+	/* update sh_addr to point to symtab */
> > >>+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
> > >>+
> > >>+	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
> > >>+	memcpy(mod->info, info, sizeof(*info));
> > >>+
> > >>+}
> > >
> > >What about arch-specific 'struct mod_arch_specific'? We need to preserve
> > >it somewhere as well for s390x and other non-x86 architectures.
> > 
> > Ah! Thank you for catching this, I overlooked this important detail.
> > Yes, we do need to save the arch-specific struct. We would be in
> > trouble for s390 relocs if we didn't. I am trying to think of a way to
> > save this information for s390, since s390's module_finalize() frees
> > mod->arch.syminfo, which we definitely need in order for the call to
> > apply_relocate_add() to work. Maybe we can add an extra call right
> > before module_finalize() that will do some livepatch-specific
> > processing and copy this information (this would be in
> > post_relocation() in kernel/module.c). Perhaps this patchset cannot be
> > entirely free of arch-specific code after all :-( Still thinking.

Well, mod_arch_specific is defined as each architecture needs. So for x86 
it is empty. It is arch-agnostic in this way and we can deal with it as 
"a black box". We just need it not to be freed in module_finalize. And...

> I think about adding a flag somewhere, e.g. mod->preserve_relocs.
> It might be set in simplify_symbols() when SHN_LIVEPATCH is found.
> It might be checked when freeing the needed structures in both
> the generic and arch-specific code.

...that is the reason why some sort of flag seems to be necessary. It 
could be set when livepatch is set in modinfo. We would use it for 
preserving both load_info and mod_arch_specific struct (in some form) and 
for...

> > >>+#ifdef CONFIG_LIVEPATCH
> > >>+	/*
> > >>+	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
> > >>+		klp_prepare_patch_module(mod, info);
> > >>+	else
> > >>+		free_copy(info);
> > >>+#else
> > >> 	/* Get rid of temporary copy. */
> > >> 	free_copy(info);
> > >>+#endif
> > >
> > >Maybe I am missing something but isn't it necessary to call vfree() on
> > >info somewhere in the end?
> > 
> > So free_copy() will call vfree(info->hdr), except in livepatch modules
> > we want to keep all the elf section information stored there, so we
> > avoid calling free_copy(), As for the info struct itself, if you look
> > at the init_module and finit_module syscall definitions in
> > kernel/module.c, you will see that info is actually a local function
> > variable, simply passed in to the call to load_module(), and will be
> > automatically deallocated when the syscall returns. :-) No need to
> > explicitly free info.
> 
> We still have to free the copied or preserved structures when
> the module is unloaded.

...freeing what we allocated. We need to free info->hdr somewhere if not 
here and also mod_arch_specific struct where the patch module is removed. 
This would unfortunately lead to changes in arch-specific code in 
module.c. For example in arch/s390/kernel/module.c there is vfree call on 
part of mod_arch_specific in module_finalize. We would call it only if the 
flag mentioned above is not set and at the same time we would need to call 
it when the patch module is being removed.

Hm, this is (again) getting complicated and ugly. Is there someone who can 
simplify things? Josh? :)

Miroslav
--
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]

#1268015 — Re: module: save load_info for livepatch modules

On Thu 2015-11-12 14:22:28, Miroslav Benes wrote:
> On Thu, 12 Nov 2015, Petr Mladek wrote:
> > > >Maybe I am missing something but isn't it necessary to call vfree() on
> > > >info somewhere in the end?
> > > 
> > > So free_copy() will call vfree(info->hdr), except in livepatch modules
> > > we want to keep all the elf section information stored there, so we
> > > avoid calling free_copy(), As for the info struct itself, if you look
> > > at the init_module and finit_module syscall definitions in
> > > kernel/module.c, you will see that info is actually a local function
> > > variable, simply passed in to the call to load_module(), and will be
> > > automatically deallocated when the syscall returns. :-) No need to
> > > explicitly free info.
> > 
> > We still have to free the copied or preserved structures when
> > the module is unloaded.
> 
> ...freeing what we allocated. We need to free info->hdr somewhere if not 
> here and also mod_arch_specific struct where the patch module is removed. 
> This would unfortunately lead to changes in arch-specific code in 
> module.c. For example in arch/s390/kernel/module.c there is vfree call on 
> part of mod_arch_specific in module_finalize. We would call it only if the 
> flag mentioned above is not set and at the same time we would need to call 
> it when the patch module is being removed.

Sigh, I am afraid that the flag is not enough. IMHO, we need to split
the load finalizing functions into two pieces. One will be always
called when the module load is finalized. The other part will free
the load_info. It will be called either when the load is finalized or
when the module is unloaded, depending on if we want to preserve
the load_info.

Sigh, it is getting complicated. But let's see how it looks in reality.

Best Regards,
Petr
--
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]

#1268114 — Re: module: save load_info for livepatch modules

On Thu, Nov 12, 2015 at 04:03:45PM +0100, Petr Mladek wrote:
> On Thu 2015-11-12 14:22:28, Miroslav Benes wrote:
> > On Thu, 12 Nov 2015, Petr Mladek wrote:
> > > > >Maybe I am missing something but isn't it necessary to call vfree() on
> > > > >info somewhere in the end?
> > > > 
> > > > So free_copy() will call vfree(info->hdr), except in livepatch modules
> > > > we want to keep all the elf section information stored there, so we
> > > > avoid calling free_copy(), As for the info struct itself, if you look
> > > > at the init_module and finit_module syscall definitions in
> > > > kernel/module.c, you will see that info is actually a local function
> > > > variable, simply passed in to the call to load_module(), and will be
> > > > automatically deallocated when the syscall returns. :-) No need to
> > > > explicitly free info.
> > > 
> > > We still have to free the copied or preserved structures when
> > > the module is unloaded.
> > 
> > ...freeing what we allocated. We need to free info->hdr somewhere if not 
> > here and also mod_arch_specific struct where the patch module is removed. 
> > This would unfortunately lead to changes in arch-specific code in 
> > module.c. For example in arch/s390/kernel/module.c there is vfree call on 
> > part of mod_arch_specific in module_finalize. We would call it only if the 
> > flag mentioned above is not set and at the same time we would need to call 
> > it when the patch module is being removed.
> 
> Sigh, I am afraid that the flag is not enough. IMHO, we need to split
> the load finalizing functions into two pieces. One will be always
> called when the module load is finalized. The other part will free
> the load_info. It will be called either when the load is finalized or
> when the module is unloaded, depending on if we want to preserve
> the load_info.
> 
> Sigh, it is getting complicated. But let's see how it looks in reality.

At the other end of the spectrum, we could do the simplest thing
possible: _always_ save the data (even if CONFIG_LIVEPATCH is disabled).

(gdb) print sizeof(*info)
$3 = 96
(gdb) p sizeof(*info->hdr)
$4 = 64
s390 mod_arch_syminfo struct: 24 bytes by my reckoning.

So between info, info->hdr, and s390 mod_arch_syminfo, we're talking
about 184 bytes on s390 and 160 bytes on x86_64.  That seems like
peanuts compared to the size of a typical module.  The benefit is that
the code would be simpler because we don't have any special cases and
the structs would automatically get freed with the module struct when
the module gets unloaded.

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

#1268300 — Re: module: save load_info for livepatch modules

+++ Josh Poimboeuf [12/11/15 11:05 -0600]:
>On Thu, Nov 12, 2015 at 04:03:45PM +0100, Petr Mladek wrote:
>> On Thu 2015-11-12 14:22:28, Miroslav Benes wrote:
>> > On Thu, 12 Nov 2015, Petr Mladek wrote:
>> > > > >Maybe I am missing something but isn't it necessary to call vfree() on
>> > > > >info somewhere in the end?
>> > > >
>> > > > So free_copy() will call vfree(info->hdr), except in livepatch modules
>> > > > we want to keep all the elf section information stored there, so we
>> > > > avoid calling free_copy(), As for the info struct itself, if you look
>> > > > at the init_module and finit_module syscall definitions in
>> > > > kernel/module.c, you will see that info is actually a local function
>> > > > variable, simply passed in to the call to load_module(), and will be
>> > > > automatically deallocated when the syscall returns. :-) No need to
>> > > > explicitly free info.
>> > >
>> > > We still have to free the copied or preserved structures when
>> > > the module is unloaded.
>> >
>> > ...freeing what we allocated. We need to free info->hdr somewhere if not
>> > here and also mod_arch_specific struct where the patch module is removed.
>> > This would unfortunately lead to changes in arch-specific code in
>> > module.c. For example in arch/s390/kernel/module.c there is vfree call on
>> > part of mod_arch_specific in module_finalize. We would call it only if the
>> > flag mentioned above is not set and at the same time we would need to call
>> > it when the patch module is being removed.
>>
>> Sigh, I am afraid that the flag is not enough. IMHO, we need to split
>> the load finalizing functions into two pieces. One will be always
>> called when the module load is finalized. The other part will free
>> the load_info. It will be called either when the load is finalized or
>> when the module is unloaded, depending on if we want to preserve
>> the load_info.
>>
>> Sigh, it is getting complicated. But let's see how it looks in reality.
>
>At the other end of the spectrum, we could do the simplest thing
>possible: _always_ save the data (even if CONFIG_LIVEPATCH is disabled).
>
>(gdb) print sizeof(*info)
>$3 = 96
>(gdb) p sizeof(*info->hdr)
>$4 = 64
>s390 mod_arch_syminfo struct: 24 bytes by my reckoning.
>
>So between info, info->hdr, and s390 mod_arch_syminfo, we're talking
>about 184 bytes on s390 and 160 bytes on x86_64.  That seems like
>peanuts compared to the size of a typical module.  The benefit is that
>the code would be simpler because we don't have any special cases and
>the structs would automatically get freed with the module struct when
>the module gets unloaded.

I think I agree with Josh on this one (except, I would always save
load_info if it is a livepatch module, instead of for every module in the
!CONFIG_LIVEPATCH case, and we can just check modinfo to see if it is
a livepatch module).

If the tradeoff here is between simplicity and readibility of code vs.
saving some extra space (and by the looks of it, not a lot), I think I
would choose having clear code over saving some bytes of memory. Hard
coding checks and edge cases imo might cause confusion and trouble
down the road.

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

#1268905 — Re: module: save load_info for livepatch modules

On Thu, 12 Nov 2015, Jessica Yu wrote:

> +++ Josh Poimboeuf [12/11/15 11:05 -0600]:
> > On Thu, Nov 12, 2015 at 04:03:45PM +0100, Petr Mladek wrote:
> > > On Thu 2015-11-12 14:22:28, Miroslav Benes wrote:
> > > > On Thu, 12 Nov 2015, Petr Mladek wrote:
> > > > > > >Maybe I am missing something but isn't it necessary to call vfree()
> > > on
> > > > > > >info somewhere in the end?
> > > > > >
> > > > > > So free_copy() will call vfree(info->hdr), except in livepatch
> > > modules
> > > > > > we want to keep all the elf section information stored there, so we
> > > > > > avoid calling free_copy(), As for the info struct itself, if you
> > > look
> > > > > > at the init_module and finit_module syscall definitions in
> > > > > > kernel/module.c, you will see that info is actually a local function
> > > > > > variable, simply passed in to the call to load_module(), and will be
> > > > > > automatically deallocated when the syscall returns. :-) No need to
> > > > > > explicitly free info.
> > > > >
> > > > > We still have to free the copied or preserved structures when
> > > > > the module is unloaded.
> > > >
> > > > ...freeing what we allocated. We need to free info->hdr somewhere if not
> > > > here and also mod_arch_specific struct where the patch module is
> > > removed.
> > > > This would unfortunately lead to changes in arch-specific code in
> > > > module.c. For example in arch/s390/kernel/module.c there is vfree call
> > > on
> > > > part of mod_arch_specific in module_finalize. We would call it only if
> > > the
> > > > flag mentioned above is not set and at the same time we would need to
> > > call
> > > > it when the patch module is being removed.
> > > 
> > > Sigh, I am afraid that the flag is not enough. IMHO, we need to split
> > > the load finalizing functions into two pieces. One will be always
> > > called when the module load is finalized. The other part will free
> > > the load_info. It will be called either when the load is finalized or
> > > when the module is unloaded, depending on if we want to preserve
> > > the load_info.
> > > 
> > > Sigh, it is getting complicated. But let's see how it looks in reality.
> > 
> > At the other end of the spectrum, we could do the simplest thing
> > possible: _always_ save the data (even if CONFIG_LIVEPATCH is disabled).
> > 
> > (gdb) print sizeof(*info)
> > $3 = 96
> > (gdb) p sizeof(*info->hdr)
> > $4 = 64
> > s390 mod_arch_syminfo struct: 24 bytes by my reckoning.
> > 
> > So between info, info->hdr, and s390 mod_arch_syminfo, we're talking
> > about 184 bytes on s390 and 160 bytes on x86_64.  That seems like
> > peanuts compared to the size of a typical module.  The benefit is that
> > the code would be simpler because we don't have any special cases and
> > the structs would automatically get freed with the module struct when
> > the module gets unloaded.

Agreed. mod_arch_specific contains more things on certain architectures, 
but compared to the size of a module it is still not much.

> 
> I think I agree with Josh on this one (except, I would always save
> load_info if it is a livepatch module, instead of for every module in the
> !CONFIG_LIVEPATCH case, and we can just check modinfo to see if it is
> a livepatch module).
> 
> If the tradeoff here is between simplicity and readibility of code vs.
> saving some extra space (and by the looks of it, not a lot), I think I
> would choose having clear code over saving some bytes of memory. Hard
> coding checks and edge cases imo might cause confusion and trouble
> down the road.

I agree this seems like the best approach. So if we preserve 
mod_arch_syminfo (in case of s390) we should free it not in 
module_finalize, but somewhere in free_module... where 
module_arch_cleanup() is called... and also module_arch_freeing_init() is 
called there too. And what you find there for s390 is 

	vfree(mod->arch.syminfo);
	mod->arch.syminfo = NULL;

Well, it does nothing here, because mod->arch.syminfo is already NULL. It 
was freed in module_finalize. So we can even remove this code from 
module_finalize and all should be fine. At least for s390.

As for load_info, I don't have a strong opinion whether to keep it for all 
modules or for livepatch modules only.

Miroslav
--
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]

#1268930 — Re: module: save load_info for livepatch modules

On Fri, 13 Nov 2015, Miroslav Benes wrote:

> As for load_info, I don't have a strong opinion whether to keep it for all 
> modules or for livepatch modules only.

I have. We cannot keep it, even for livepatch modules...

In info->hdr there is a temporary copy of the whole module (see 
init_module syscall and the first parts of load_module). In load_module 
a final struct module * is created with parts of info->hdr copied (I'll 
get to that later). So if we saved info->hdr for later purposes we would 
just have two copies of the same module in the memory. The original one 
with !SHF_ALLOC sections and everything in vmalloc area, and the new
final copy with SHF_ALLOC sections only. This is not good.

If this is correct (and I think it is after some staring into the code) we 
need to do something different. We should build the info we need for 
delayed relocations from the final copy (or refactor the existing 
module code).

The second problem... dynrela sections need to be marked with SHF_ALLOC 
flag, right? Perhaps it would be better not to do it and copy also 
SHF_RELA_LIVEPATCH sections. It is equivalent but not hidden somewhere 
else (in userspace "kpatch-build" tool).

Miroslav
--
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]

#1269379 — Re: module: save load_info for livepatch modules

+++ Miroslav Benes [13/11/15 13:46 +0100]:
>On Fri, 13 Nov 2015, Miroslav Benes wrote:
>
>> As for load_info, I don't have a strong opinion whether to keep it for all
>> modules or for livepatch modules only.
>
>I have. We cannot keep it, even for livepatch modules...
>
>In info->hdr there is a temporary copy of the whole module (see
>init_module syscall and the first parts of load_module). In load_module
>a final struct module * is created with parts of info->hdr copied (I'll
>get to that later). So if we saved info->hdr for later purposes we would
>just have two copies of the same module in the memory. The original one
>with !SHF_ALLOC sections and everything in vmalloc area, and the new
>final copy with SHF_ALLOC sections only. This is not good.
>
>If this is correct (and I think it is after some staring into the code) we
>need to do something different. We should build the info we need for
>delayed relocations from the final copy (or refactor the existing
>module code).
>
>The second problem... dynrela sections need to be marked with SHF_ALLOC
>flag, right? Perhaps it would be better not to do it and copy also
>SHF_RELA_LIVEPATCH sections. It is equivalent but not hidden somewhere
>else (in userspace "kpatch-build" tool).

Hm, OK. I understand your concern about leaving a redundant copy of
the module in memory and I agree that we need to do better. I think I
have a solution.

I'm looking at exactly what components we need to make the calls to
apply_relocate_add() work. It's quite simple, I think we only need to
keep the following:

1. A copy of the module's elf section headers i.e. info->sechdrs.
This should be easy to copy. memcpy [info->hdr->e_shnum *
sizeof(Elf_Shdr)] bytes from info->sechdrs. We can maybe put
this in a new field called module->sechdrs.

2. A copy of each __klp_rela section.
If we don't keep info, the current code will discard/not copy the rela
sections over to module core memory since they are !SHF_ALLOC. In
kpatch-build, it is very easy to simply |= the SHF_ALLOC flag with
each __klp_rela section and they will automatically get copied over to
module core memory, and their sh_addr's automatically get reassigned
as well. Thus the klp rela sections will be accessible at
sechdrs[index_of_klpsec].sh_addr. I think this is the easiest solution.

3. A copy of the symbol table. 
Notice that module already has a "symtab" field. In kernels configured
with CONFIG_KALLSYMS, it points to a trimmed down symtab (the
mod->core_symtab) in module core memory. This symtab is not normally
complete; only "core" symbols are kept in it. See add_kallsyms()
(called in post_relocations()) for how core symbols are copied into
this symtab. Then, after the symbols have been copied, module->symtab
is reassigned to point to this core_symtab in do_init_module(). Since
CONFIG_LIVEPATCH requires CONFIG_KALLSYMS, I think we can assume that
mod->symtab will be pointing to mod->core_symtab at the end of the
module load process, since mod->symtab gets assigned to core_symtab in
do_init_module() if CONFIG_KALLSYMS is set.

So for livepatch, what we can do is make sure every symbol in a
livepatch module gets copied into this core symtab. It is important we
keep every symbol since apply_relocate_add() will be using the
original symbol indices. We can implement this by adding a check in
add_kallsyms() to see if we're dealing with a livepatch module. If
yes, just copy all the symbols over.

Then, we will also update Elf_Shdr corresponding to the symbol table
section (sechdrs[symindex].sh_addr) to make sure its sh_addr points to
mod->symtab, so apply_relocate_add() will be able to use it.

4. A copy of mod_arch_specific
I think we discussed this in another email somewhere, but we need to
keep a copy if this somewhere as well. 

So to summarize, keep a copy of sechdrs in module->sechdrs, keep a
copy of mod_arch_specific, mark klp rela sections with SHF_ALLOC,
re-use module->symtab by making sure every symbol gets considered a
"core" symbol and gets copied over. And of course any memory we
allocate (sechdrs, arch stuff) we will free in perhaps free_module()
somewhere.

I haven't implemented it yet but I think it will work, and we don't
need to keep load_info in this scheme. What do you think?

Thanks,
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]

#1268931 — Re: module: save load_info for livepatch modules

On Fri, 13 Nov 2015, Miroslav Benes wrote:

> I agree this seems like the best approach. So if we preserve 
> mod_arch_syminfo (in case of s390) we should free it not in 
> module_finalize, but somewhere in free_module... where 
> module_arch_cleanup() is called... and also module_arch_freeing_init() is 
> called there too. And what you find there for s390 is 
> 
> 	vfree(mod->arch.syminfo);
> 	mod->arch.syminfo = NULL;
> 
> Well, it does nothing here, because mod->arch.syminfo is already NULL. It 
> was freed in module_finalize. So we can even remove this code from 
> module_finalize and all should be fine. At least for s390.

Which is not true because module_arch_freeing_init is also called from 
do_init_module, called from load_module. So we should move it to 
module_arch_cleanup.

That code is like a maze without Ariadne's thread.

Miroslav
--
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]

#1269390 — Re: module: save load_info for livepatch modules

+++ Miroslav Benes [13/11/15 13:56 +0100]:
>On Fri, 13 Nov 2015, Miroslav Benes wrote:
>
>> I agree this seems like the best approach. So if we preserve
>> mod_arch_syminfo (in case of s390) we should free it not in
>> module_finalize, but somewhere in free_module... where
>> module_arch_cleanup() is called... and also module_arch_freeing_init() is
>> called there too. And what you find there for s390 is
>>
>> 	vfree(mod->arch.syminfo);
>> 	mod->arch.syminfo = NULL;
>>
>> Well, it does nothing here, because mod->arch.syminfo is already NULL. It
>> was freed in module_finalize. So we can even remove this code from
>> module_finalize and all should be fine. At least for s390.
>
>Which is not true because module_arch_freeing_init is also called from
>do_init_module, called from load_module. So we should move it to
>module_arch_cleanup.
>
>That code is like a maze without Ariadne's thread.

Heh, I agree with that sentiment.

I am slightly confused about the s390 code, and whether the authors
originally intended for that double vfree() to happen in both
module_finalize() and module_arch_freeing_init() (called from
do_init_module). Seems like a mistake. If module load succeeds,
do_init_module calls module_arch_freeing_init(). And if load_module
fails halfway through, both module_deallocate() and free_module() will
also call module_arch_freeing_init(). I feel like that vfree should
only happen once in module_arch_freeing_init() and not in
module_finalize(). If we can remove the double vfree() code from
module_finalize(), we can copy the mod_arch_specific safely before the
call to do_init_module().

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]

#1268513 — Re: module: save load_info for livepatch modules

+++ Miroslav Benes [12/11/15 14:22 +0100]:
>On Thu, 12 Nov 2015, Petr Mladek wrote:
>
>> On Thu 2015-11-12 00:33:12, Jessica Yu wrote:
>> > +++ Miroslav Benes [11/11/15 15:17 +0100]:
>> > >On Mon, 9 Nov 2015, Jessica Yu wrote:
>> > >
>> > >>diff --git a/include/linux/module.h b/include/linux/module.h
>> > >>index 3a19c79..c8680b1 100644
>> > >>--- a/include/linux/module.h
>> > >>+++ b/include/linux/module.h
>> > >
>> > >[...]
>> > >
>> > >>+#ifdef CONFIG_LIVEPATCH
>> > >>+extern void klp_prepare_patch_module(struct module *mod,
>> > >>+				     struct load_info *info);
>> > >>+extern int
>> > >>+apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
>> > >>+		   unsigned int symindex, unsigned int relsec,
>> > >>+		   struct module *me);
>> > >>+#endif
>> > >>+
>> > >> #else /* !CONFIG_MODULES... */
>> > >
>> > >apply_relocate_add() is already in include/linux/moduleloader.h (guarded
>> > >by CONFIG_MODULES_USE_ELF_RELA), so maybe we can just include that where
>> > >we need it. As for the klp_prepare_patch_module() wouldn't it be better to
>> > >have it in our livepatch.h and include that in kernel/module.c?
>> >
>> > Yeah, Petr pointed this out as well :-) I will just include
>> > moduleloader.h for the apply_relocate_add() declaration.
>> >
>> > It also looks like we have some disagreement over where to put
>> > klp_prepare_patch_module(), either in livepatch/core.c (and add the
>> > function declaration in livepatch.h, and have module.c include
>> > livepatch.h) or in kernel/module.c, keeping the
>> > klp_prepare_patch_module() declaration in module.h. Maybe Rusty can
>> > provide some input.
>
>Yes, there are several ways how to do it. Maybe the best would be some
>generic way in kernel/module.c. I am not sure if there will be another
>user of this in the future but nevertheless. It would also allow us to
>somehow solve the issues mentioned below. Thus, klp_prepare_patch_module
>is inappropriate name and it should be for example just preserve_load_info
>(or more general if needed) and it should be in kernel/module.c.

A more generic way sounds good. I think Petr is leaning towards this
too, i.e. have a generic function named copy_module_info() in
module.c, instead of klp_prepare_patch_module().

>> > >> /* Given an address, look for it in the exception tables. */
>> > >>diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> > >>index 6e53441..087a8c7 100644
>> > >>--- a/kernel/livepatch/core.c
>> > >>+++ b/kernel/livepatch/core.c
>> > >>@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
>> > >> 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
>> > >> };
>> > >>
>> > >>+/*
>> > >>+ * Save necessary information from info in order to be able to
>> > >>+ * patch modules that might be loaded later
>> > >>+ */
>> > >>+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
>> > >>+{
>> > >>+	Elf_Shdr *symsect;
>> > >>+
>> > >>+	symsect = info->sechdrs + info->index.sym;
>> > >>+	/* update sh_addr to point to symtab */
>> > >>+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
>> > >>+
>> > >>+	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
>> > >>+	memcpy(mod->info, info, sizeof(*info));
>> > >>+
>> > >>+}
>> > >
>> > >What about arch-specific 'struct mod_arch_specific'? We need to preserve
>> > >it somewhere as well for s390x and other non-x86 architectures.
>> >
>> > Ah! Thank you for catching this, I overlooked this important detail.
>> > Yes, we do need to save the arch-specific struct. We would be in
>> > trouble for s390 relocs if we didn't. I am trying to think of a way to
>> > save this information for s390, since s390's module_finalize() frees
>> > mod->arch.syminfo, which we definitely need in order for the call to
>> > apply_relocate_add() to work. Maybe we can add an extra call right
>> > before module_finalize() that will do some livepatch-specific
>> > processing and copy this information (this would be in
>> > post_relocation() in kernel/module.c). Perhaps this patchset cannot be
>> > entirely free of arch-specific code after all :-( Still thinking.
>
>Well, mod_arch_specific is defined as each architecture needs. So for x86
>it is empty. It is arch-agnostic in this way and we can deal with it as
>"a black box". We just need it not to be freed in module_finalize. And...
>
>> I think about adding a flag somewhere, e.g. mod->preserve_relocs.
>> It might be set in simplify_symbols() when SHN_LIVEPATCH is found.
>> It might be checked when freeing the needed structures in both
>> the generic and arch-specific code.
>
>...that is the reason why some sort of flag seems to be necessary. It
>could be set when livepatch is set in modinfo. We would use it for
>preserving both load_info and mod_arch_specific struct (in some form) and
>for...
>
>> > >>+#ifdef CONFIG_LIVEPATCH
>> > >>+	/*
>> > >>+	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
>> > >>+		klp_prepare_patch_module(mod, info);
>> > >>+	else
>> > >>+		free_copy(info);
>> > >>+#else
>> > >> 	/* Get rid of temporary copy. */
>> > >> 	free_copy(info);
>> > >>+#endif
>> > >
>> > >Maybe I am missing something but isn't it necessary to call vfree() on
>> > >info somewhere in the end?
>> >
>> > So free_copy() will call vfree(info->hdr), except in livepatch modules
>> > we want to keep all the elf section information stored there, so we
>> > avoid calling free_copy(), As for the info struct itself, if you look
>> > at the init_module and finit_module syscall definitions in
>> > kernel/module.c, you will see that info is actually a local function
>> > variable, simply passed in to the call to load_module(), and will be
>> > automatically deallocated when the syscall returns. :-) No need to
>> > explicitly free info.
>>
>> We still have to free the copied or preserved structures when
>> the module is unloaded.
>
>...freeing what we allocated. We need to free info->hdr somewhere if not
>here and also mod_arch_specific struct where the patch module is removed.

Right, I intended to free the preserved/copied structures in patch_exit():
https://github.com/flaming-toast/kpatch/blob/no_dynrela_redux/kmod/patch/livepatch-patch-hook.c#L322

But now that I'm thinking about it, perhaps it is better and clearer
to have the freeing be done in free_module()?

>This would unfortunately lead to changes in arch-specific code in
>module.c. For example in arch/s390/kernel/module.c there is vfree call on
>part of mod_arch_specific in module_finalize. We would call it only if the
>flag mentioned above is not set and at the same time we would need to call
>it when the patch module is being removed.

Yup..this is what I meant when I said I was concerned that this
patchset might end up needing arch-specific code. :-\

Hard coding a flag check doesn't seem very portable or modular (here,
it would be a specific case to s390). If we do require arch code, how
about using a small arch-specific livepatch function to do the
copying, maybe call it klp_copy_arch_info()?

Actually, if we're going the generic route, we can just call it
copy_arch_info(). Maybe we can put the call and definition in
arch/../kernel/module.c, and it will copy the mod_arch_specific struct
(plus do whatever else that's needed). In the case of s390, we need to
additionally copy the mod_arch_syminfo array. Then, we can just leave
the vfree alone.

So in this scheme, I'd imagine we'd have copy_module_info() +
copy_arch_info(), called from the module loader if the module is a
livepatch module. However I am not yet entirely sure where to put the
call to copy_arch_info(), maybe within module_finalize()? 
Then, we could have the corresponding free_module_info() and
free_arch_info() functions, maybe called from free_module() instead of
patch_exit(). Does this sound too complicated? Would it work?

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

#1267222 — Re: [RFC PATCH 2/5] module: save load_info for livepatch modules

On Mon 2015-11-09 23:45:52, Jessica Yu wrote:
> In livepatch modules, preserve section, symbol, string information from
> the load_info struct in the module loader. This information is used to
> patch modules that are not loaded in memory yet; specifically it is used
> to resolve remaining symbols and write relocations when the target
> module loads.
> 
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> ---
>  include/linux/module.h  | 25 +++++++++++++++++++++++++
>  kernel/livepatch/core.c | 17 +++++++++++++++++
>  kernel/module.c         | 36 ++++++++++++++++++++++--------------
>  3 files changed, 64 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3a19c79..c8680b1 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
[...]
> @@ -635,6 +651,15 @@ static inline bool module_requested_async_probing(struct module *module)
>  	return module && module->async_probe_requested;
>  }
>  
> +#ifdef CONFIG_LIVEPATCH
> +extern void klp_prepare_patch_module(struct module *mod,
> +				     struct load_info *info);
> +extern int
> +apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
> +		   unsigned int symindex, unsigned int relsec,
> +		   struct module *me);
> +#endif

This function is already declared in moduleloader.h.
It is implemted only when CONFIG_MODULES_USE_ELF_RELA is defined.

I guess that we want to include moduleloader.h in livepatch.

> +
>  #else /* !CONFIG_MODULES... */
>  
>  /* Given an address, look for it in the exception tables. */
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 6e53441..087a8c7 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
>  	.priority = INT_MIN+1, /* called late but before ftrace notifier */
>  };
>  
> +/*
> + * Save necessary information from info in order to be able to
> + * patch modules that might be loaded later
> + */
> +void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> +{
> +	Elf_Shdr *symsect;
> +
> +	symsect = info->sechdrs + info->index.sym;
> +	/* update sh_addr to point to symtab */
> +	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;

Is livepatch the only user of this value? By other words, is this safe?

> +	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	memcpy(mod->info, info, sizeof(*info));
> +
> +}

It is strange that this funtion is defined in livepatch/core.c
but declared in module.h. I would move the definition to
module.c.

>  static int __init klp_init(void)
>  {
>  	int ret;
> diff --git a/kernel/module.c b/kernel/module.c
> index 8f051a1..8ae3ca5 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -318,20 +318,6 @@ int unregister_module_notifier(struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL(unregister_module_notifier);
>  
> -struct load_info {
> -	Elf_Ehdr *hdr;
> -	unsigned long len;
> -	Elf_Shdr *sechdrs;
> -	char *secstrings, *strtab;
> -	unsigned long symoffs, stroffs;
> -	struct _ddebug *debug;
> -	unsigned int num_debug;
> -	bool sig_ok;
> -	struct {
> -		unsigned int sym, str, mod, vers, info, pcpu;
> -	} index;
> -};
> -
>  /* We require a truly strong try_module_get(): 0 means failure due to
>     ongoing or failed initialization etc. */
>  static inline int strong_try_module_get(struct module *mod)
> @@ -2137,6 +2123,11 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
>  			       (long)sym[i].st_value);
>  			break;
>  
> +#ifdef CONFIG_LIVEPATCH
> +		case SHN_LIVEPATCH:
> +			break;
> +#endif

IMHO, even a kernel compiled without CONFIG_LIVEPATCH should handle livepatch
modules with grace. It means to reject loading.

>  		case SHN_UNDEF:
>  			ksym = resolve_symbol_wait(mod, info, name);
>  			/* Ok if resolved.  */
> @@ -2185,6 +2176,11 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
>  		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
>  			continue;
>  
> +#ifdef CONFIG_LIVEPATCH
> +		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
> +			continue;
> +#endif
> +
>  		if (info->sechdrs[i].sh_type == SHT_REL)
>  			err = apply_relocate(info->sechdrs, info->strtab,
>  					     info->index.sym, i, mod);
> @@ -3530,8 +3526,20 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  	if (err < 0)
>  		goto bug_cleanup;
>  
> +#ifdef CONFIG_LIVEPATCH
> +	/*
> +	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
> +		klp_prepare_patch_module(mod, info);
> +	else
> +		free_copy(info);
> +#else

I would move this #else one line above and get rid of the
double free_copy(info); But it is a matter of taste.


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

Best Regards,
Petr
--
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]

#1267633 — Re: module: save load_info for livepatch modules

+++ Petr Mladek [11/11/15 15:31 +0100]:
>On Mon 2015-11-09 23:45:52, Jessica Yu wrote:
>> In livepatch modules, preserve section, symbol, string information from
>> the load_info struct in the module loader. This information is used to
>> patch modules that are not loaded in memory yet; specifically it is used
>> to resolve remaining symbols and write relocations when the target
>> module loads.
>>
>> Signed-off-by: Jessica Yu <jeyu@redhat.com>
>> ---
>>  include/linux/module.h  | 25 +++++++++++++++++++++++++
>>  kernel/livepatch/core.c | 17 +++++++++++++++++
>>  kernel/module.c         | 36 ++++++++++++++++++++++--------------
>>  3 files changed, 64 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 3a19c79..c8680b1 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>[...]
>> @@ -635,6 +651,15 @@ static inline bool module_requested_async_probing(struct module *module)
>>  	return module && module->async_probe_requested;
>>  }
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +extern void klp_prepare_patch_module(struct module *mod,
>> +				     struct load_info *info);
>> +extern int
>> +apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab,
>> +		   unsigned int symindex, unsigned int relsec,
>> +		   struct module *me);
>> +#endif
>
>This function is already declared in moduleloader.h.
>It is implemted only when CONFIG_MODULES_USE_ELF_RELA is defined.
>
>I guess that we want to include moduleloader.h in livepatch.
>
>> +
>>  #else /* !CONFIG_MODULES... */
>>
>>  /* Given an address, look for it in the exception tables. */
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index 6e53441..087a8c7 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
>>  	.priority = INT_MIN+1, /* called late but before ftrace notifier */
>>  };
>>
>> +/*
>> + * Save necessary information from info in order to be able to
>> + * patch modules that might be loaded later
>> + */
>> +void klp_prepare_patch_module(struct module *mod, struct load_info *info)
>> +{
>> +	Elf_Shdr *symsect;
>> +
>> +	symsect = info->sechdrs + info->index.sym;
>> +	/* update sh_addr to point to symtab */
>> +	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
>
>Is livepatch the only user of this value? By other words, is this safe?

I think it is safe to say yes. klp_prepare_patch_module() is only
called at the very end of load_module(), right before
do_init_module(). Normally, at that point, info->hdr will have already
been freed by free_copy() along with the elf section information
associated with it. But if we have a livepatch module, we don't free.
So we should be the very last user, and there should be nobody
utilizing the memory associated with the load_info struct anymore at
that point.

>> +	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +	memcpy(mod->info, info, sizeof(*info));
>> +
>> +}
>
>It is strange that this funtion is defined in livepatch/core.c
>but declared in module.h. I would move the definition to
>module.c.

Right, I was trying to keep all the livepatch-related functions
together in livepatch/core.c. but I can move it to module.c if it
makes more sense/Rusty doesn't object to it :-)

>>  static int __init klp_init(void)
>>  {
>>  	int ret;
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 8f051a1..8ae3ca5 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -318,20 +318,6 @@ int unregister_module_notifier(struct notifier_block *nb)
>>  }
>>  EXPORT_SYMBOL(unregister_module_notifier);
>>
>> -struct load_info {
>> -	Elf_Ehdr *hdr;
>> -	unsigned long len;
>> -	Elf_Shdr *sechdrs;
>> -	char *secstrings, *strtab;
>> -	unsigned long symoffs, stroffs;
>> -	struct _ddebug *debug;
>> -	unsigned int num_debug;
>> -	bool sig_ok;
>> -	struct {
>> -		unsigned int sym, str, mod, vers, info, pcpu;
>> -	} index;
>> -};
>> -
>>  /* We require a truly strong try_module_get(): 0 means failure due to
>>     ongoing or failed initialization etc. */
>>  static inline int strong_try_module_get(struct module *mod)
>> @@ -2137,6 +2123,11 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
>>  			       (long)sym[i].st_value);
>>  			break;
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +		case SHN_LIVEPATCH:
>> +			break;
>> +#endif
>
>IMHO, even a kernel compiled without CONFIG_LIVEPATCH should handle livepatch
>modules with grace. It means to reject loading.

I think even right now, without considering this patchset, we don't
reject modules "gracefully" when we load a livepatch module without
CONFIG_LIVEPATCH. The module loader will complain and reject the
livepatch module, saying something like "Unknown symbol
klp_register_patch." This behavior is the same with or without
this patch series applied. If we want to add a bit more logic to
gracefully reject patch modules, perhaps that should be a different
patch altogether, as I think it is unrelated to the goal of this one :-)

>>  		case SHN_UNDEF:
>>  			ksym = resolve_symbol_wait(mod, info, name);
>>  			/* Ok if resolved.  */
>> @@ -2185,6 +2176,11 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
>>  		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
>>  			continue;
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
>> +			continue;
>> +#endif
>> +
>>  		if (info->sechdrs[i].sh_type == SHT_REL)
>>  			err = apply_relocate(info->sechdrs, info->strtab,
>>  					     info->index.sym, i, mod);
>> @@ -3530,8 +3526,20 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>  	if (err < 0)
>>  		goto bug_cleanup;
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +	/*
>> +	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
>> +		klp_prepare_patch_module(mod, info);
>> +	else
>> +		free_copy(info);
>> +#else
>
>I would move this #else one line above and get rid of the
>double free_copy(info); But it is a matter of taste.

Maybe I'm missing something, but I think we do need the double
free_copy(), because in the CONFIG_LIVEPATCH case, we still want to
call free_copy() for non-livepatch modules. And we want to avoid
calling free_copy() for livepatch modules (hence the extra else).

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

Thanks for the comments,
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]

#1267777 — Re: module: save load_info for livepatch modules

On Wed 2015-11-11 23:44:08, Jessica Yu wrote:
> +++ Petr Mladek [11/11/15 15:31 +0100]:
> >On Mon 2015-11-09 23:45:52, Jessica Yu wrote:
> >>diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> >>index 6e53441..087a8c7 100644
> >>--- a/kernel/livepatch/core.c
> >>+++ b/kernel/livepatch/core.c
> >>@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
> >> 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
> >> };
> >>
> >>+/*
> >>+ * Save necessary information from info in order to be able to
> >>+ * patch modules that might be loaded later
> >>+ */
> >>+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> >>+{
> >>+	Elf_Shdr *symsect;
> >>+
> >>+	symsect = info->sechdrs + info->index.sym;
> >>+	/* update sh_addr to point to symtab */
> >>+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
> >
> >Is livepatch the only user of this value? By other words, is this safe?
> 
> I think it is safe to say yes. klp_prepare_patch_module() is only
> called at the very end of load_module(), right before
> do_init_module(). Normally, at that point, info->hdr will have already
> been freed by free_copy() along with the elf section information
> associated with it. But if we have a livepatch module, we don't free.
> So we should be the very last user, and there should be nobody
> utilizing the memory associated with the load_info struct anymore at
> that point.

I see. It looks safe at this point. But still I wonder if it would be
possible to calculate this later in the livepatch code. It will allow
to potentially use the info structure also by other subsystem.

BTW: Where is "sh_addr" value used, please? I see it used only
in the third patch as info->sechdrs[relindex].sh_addr. But it is
an array. I am not sure if it is the same variable.


> >>+	mod->info = kzalloc(sizeof(*info), GFP_KERNEL);
> >>+	memcpy(mod->info, info, sizeof(*info));
> >>+
> >>+}
> >
> >It is strange that this funtion is defined in livepatch/core.c
> >but declared in module.h. I would move the definition to
> >module.c.
> 
> Right, I was trying to keep all the livepatch-related functions
> together in livepatch/core.c. but I can move it to module.c if it
> makes more sense/Rusty doesn't object to it :-)

Sure. I think that we could use some generic name, e.g. copy_module_info().

> >> static int __init klp_init(void)
> >> {
> >> 	int ret;
> >>diff --git a/kernel/module.c b/kernel/module.c
> >>index 8f051a1..8ae3ca5 100644
> >>--- a/kernel/module.c
> >>+++ b/kernel/module.c
> >>@@ -2137,6 +2123,11 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
> >> 			       (long)sym[i].st_value);
> >> 			break;
> >>
> >>+#ifdef CONFIG_LIVEPATCH
> >>+		case SHN_LIVEPATCH:
> >>+			break;
> >>+#endif
> >
> >IMHO, even a kernel compiled without CONFIG_LIVEPATCH should handle livepatch
> >modules with grace. It means to reject loading.
> 
> I think even right now, without considering this patchset, we don't
> reject modules "gracefully" when we load a livepatch module without
> CONFIG_LIVEPATCH. The module loader will complain and reject the
> livepatch module, saying something like "Unknown symbol
> klp_register_patch." This behavior is the same with or without
> this patch series applied. If we want to add a bit more logic to
> gracefully reject patch modules, perhaps that should be a different
> patch altogether, as I think it is unrelated to the goal of this one :-)

Yup, the module load would fail anyway because of the missing symbol.
But I think that we should fail on the first error occurence.

In each case, IMHO, we should not do the "default:" action for this
section even when complied without CONFIG_LIVEPATCH.


> >> 		case SHN_UNDEF:
> >> 			ksym = resolve_symbol_wait(mod, info, name);
> >> 			/* Ok if resolved.  */
> >>@@ -2185,6 +2176,11 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
> >> 		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
> >> 			continue;
> >>
> >>+#ifdef CONFIG_LIVEPATCH
> >>+		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
> >>+			continue;
> >>+#endif

I guess that if we do not trigger the error above, and do
not have the check here, we will try to call apply_relocate() below.
I guess that it will fail. If we are lucky it will print "Unknown
relocation". I think that we could do better.

> >>+
> >> 		if (info->sechdrs[i].sh_type == SHT_REL)
> >> 			err = apply_relocate(info->sechdrs, info->strtab,
> >> 					     info->index.sym, i, mod);
> >>@@ -3530,8 +3526,20 @@ static int load_module(struct load_info *info, const char __user *uargs,
> >> 	if (err < 0)
> >> 		goto bug_cleanup;
> >>
> >>+#ifdef CONFIG_LIVEPATCH
> >>+	/*
> >>+	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
> >>+		klp_prepare_patch_module(mod, info);
> >>+	else
> >>+		free_copy(info);
> >>+#else
> >
> >I would move this #else one line above and get rid of the
> >double free_copy(info); But it is a matter of taste.
> 
> Maybe I'm missing something, but I think we do need the double
> free_copy(), because in the CONFIG_LIVEPATCH case, we still want to
> call free_copy() for non-livepatch modules. And we want to avoid
> calling free_copy() for livepatch modules (hence the extra else).

Ah, this was just a cosmetic change. I meant to use something like:

#ifdef CONFIG_LIVEPATCH
	/*
	 * 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 (get_modinfo((struct load_info *)info, "livepatch"))
		klp_prepare_patch_module(mod, info);
	else
#endif
		/* Get rid of temporary copy. */
		free_copy(info);


It is a matter of taste. Maybe, your variant was better in the end.

Thank you,
Petr
--
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]

#1267966 — Re: module: save load_info for livepatch modules

On Thu, 12 Nov 2015, Petr Mladek wrote:

> On Wed 2015-11-11 23:44:08, Jessica Yu wrote:
> > +++ Petr Mladek [11/11/15 15:31 +0100]:
> > >On Mon 2015-11-09 23:45:52, Jessica Yu wrote:
> > >>diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > >>index 6e53441..087a8c7 100644
> > >>--- a/kernel/livepatch/core.c
> > >>+++ b/kernel/livepatch/core.c
> > >>@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {
> > >> 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
> > >> };
> > >>
> > >>+/*
> > >>+ * Save necessary information from info in order to be able to
> > >>+ * patch modules that might be loaded later
> > >>+ */
> > >>+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
> > >>+{
> > >>+	Elf_Shdr *symsect;
> > >>+
> > >>+	symsect = info->sechdrs + info->index.sym;
> > >>+	/* update sh_addr to point to symtab */
> > >>+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;
> > >
> > >Is livepatch the only user of this value? By other words, is this safe?
> > 
> > I think it is safe to say yes. klp_prepare_patch_module() is only
> > called at the very end of load_module(), right before
> > do_init_module(). Normally, at that point, info->hdr will have already
> > been freed by free_copy() along with the elf section information
> > associated with it. But if we have a livepatch module, we don't free.
> > So we should be the very last user, and there should be nobody
> > utilizing the memory associated with the load_info struct anymore at
> > that point.
> 
> I see. It looks safe at this point. But still I wonder if it would be
> possible to calculate this later in the livepatch code. It will allow
> to potentially use the info structure also by other subsystem.
> 
> BTW: Where is "sh_addr" value used, please? I see it used only
> in the third patch as info->sechdrs[relindex].sh_addr. But it is
> an array. I am not sure if it is the same variable.

Jessica, why do we need to update sh_addr for symtab? It is not clear to 
me.

Miroslav
--
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]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web