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

Groups > linux.kernel > #1361874 > unrolled thread

Re: [PATCH v5 2/6] module: preserve Elf information for livepatch modules

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 2/6] module: preserve Elf information for livepatch  modules Josh Poimboeuf <jpoimboe@redhat.com> - 2016-03-21 15:10 +0100
    Re: module: preserve Elf information for livepatch modules Jessica Yu <jeyu@redhat.com> - 2016-03-22 19:00 +0100
      Re: module: preserve Elf information for livepatch modules Josh Poimboeuf <jpoimboe@redhat.com> - 2016-03-22 20:00 +0100

#1361874 — Re: [PATCH v5 2/6] module: preserve Elf information for livepatch modules

On Wed, Mar 16, 2016 at 03:47:04PM -0400, Jessica Yu wrote:
> For livepatch modules, copy Elf section, symbol, and string information
> from the load_info struct in the module loader. Persist copies of the
> original symbol table and string table.
> 
> Livepatch manages its own relocation sections in order to reuse module
> loader code to write relocations. Livepatch modules must preserve Elf
> information such as section indices in order to apply livepatch relocation
> sections using the module loader's apply_relocate_add() function.
> 
> In order to apply livepatch relocation sections, livepatch modules must
> keep a complete copy of their original symbol table in memory. Normally, a
> stripped down copy of a module's symbol table (containing only "core"
> symbols) is made available through module->core_symtab. But for livepatch
> modules, the symbol table copied into memory on module load must be exactly
> the same as the symbol table produced when the patch module was compiled.
> This is because the relocations in each livepatch relocation section refer
> to their respective symbols with their symbol indices, and the original
> symbol indices (and thus the symtab ordering) must be preserved in order
> for apply_relocate_add() to find the right symbol.
> 
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> ---
>  include/linux/module.h |  25 ++++++++++
>  kernel/module.c        | 123 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 146 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 2bb0c30..3daf2b3 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -330,6 +330,15 @@ struct mod_kallsyms {
>  	char *strtab;
>  };
>  
> +#ifdef CONFIG_LIVEPATCH
> +struct klp_modinfo {
> +	Elf_Ehdr hdr;
> +	Elf_Shdr *sechdrs;
> +	char *secstrings;
> +	unsigned int symndx;
> +};
> +#endif
> +
>  struct module {
>  	enum module_state state;
>  
> @@ -456,7 +465,11 @@ struct module {
>  #endif
>  
>  #ifdef CONFIG_LIVEPATCH
> +	bool klp; /* Is this a livepatch module? */
>  	bool klp_alive;
> +
> +	/* Elf information */
> +	struct klp_modinfo *klp_info;
>  #endif
>  
>  #ifdef CONFIG_MODULE_UNLOAD
> @@ -630,6 +643,18 @@ static inline bool module_requested_async_probing(struct module *module)
>  	return module && module->async_probe_requested;
>  }
>  
> +#ifdef CONFIG_LIVEPATCH
> +static inline bool is_livepatch_module(struct module *mod)
> +{
> +	return mod->klp;
> +}
> +#else /* !CONFIG_LIVEPATCH */
> +static inline bool is_livepatch_module(struct module *mod)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_LIVEPATCH */
> +
>  #else /* !CONFIG_MODULES... */
>  
>  /* Given an address, look for it in the exception tables. */
> diff --git a/kernel/module.c b/kernel/module.c
> index 87cfeb2..80b7fd9 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1971,6 +1971,82 @@ static void module_enable_nx(const struct module *mod) { }
>  static void module_disable_nx(const struct module *mod) { }
>  #endif
>  
> +#ifdef CONFIG_LIVEPATCH
> +/*
> + * Persist Elf information about a module. Copy the Elf header,
> + * section header table, section string table, and symtab section
> + * index from info to mod->klp_info.
> + */
> +static int copy_module_elf(struct module *mod, struct load_info *info)
> +{
> +	unsigned int size, symndx;
> +	int ret;
> +
> +	size = sizeof(*mod->klp_info);
> +	mod->klp_info = kmalloc(size, GFP_KERNEL);
> +	if (mod->klp_info == NULL)
> +		return -ENOMEM;
> +
> +	/* Elf header */
> +	size = sizeof(Elf_Ehdr);
> +	memcpy(&mod->klp_info->hdr, info->hdr, size);
> +
> +	/* Elf section header table */
> +	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
> +	mod->klp_info->sechdrs = kmalloc(size, GFP_KERNEL);
> +	if (mod->klp_info->sechdrs == NULL) {
> +		ret = -ENOMEM;
> +		goto free_info;
> +	}
> +	memcpy(mod->klp_info->sechdrs, info->sechdrs, size);
> +
> +	/* Elf section name string table */
> +	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
> +	mod->klp_info->secstrings = kmalloc(size, GFP_KERNEL);
> +	if (mod->klp_info->secstrings == NULL) {
> +		ret = -ENOMEM;
> +		goto free_sechdrs;
> +	}
> +	memcpy(mod->klp_info->secstrings, info->secstrings, size);
> +
> +	/* Elf symbol section index */
> +	symndx = info->index.sym;
> +	mod->klp_info->symndx = symndx;

nit: The 'symndx' local variable is superfluous.

> +
> +	/*
> +	 * For livepatch modules, core_symtab is a complete copy

s/core_symtab/core_kallsyms.symtab/ ?

> +	 * of the original symbol table. Adjust sh_addr to point
> +	 * to core_symtab since the copy of the symtab in module
> +	 * init memory is freed at the end of do_init_module().
> +	 */
> +	mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long) mod->core_kallsyms.symtab;

-- 
Josh

[toc] | [next] | [standalone]

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

+++ Josh Poimboeuf [21/03/16 09:06 -0500]:
>On Wed, Mar 16, 2016 at 03:47:04PM -0400, Jessica Yu wrote:
>> For livepatch modules, copy Elf section, symbol, and string information
>> from the load_info struct in the module loader. Persist copies of the
>> original symbol table and string table.
>>
>> Livepatch manages its own relocation sections in order to reuse module
>> loader code to write relocations. Livepatch modules must preserve Elf
>> information such as section indices in order to apply livepatch relocation
>> sections using the module loader's apply_relocate_add() function.
>>
>> In order to apply livepatch relocation sections, livepatch modules must
>> keep a complete copy of their original symbol table in memory. Normally, a
>> stripped down copy of a module's symbol table (containing only "core"
>> symbols) is made available through module->core_symtab. But for livepatch
>> modules, the symbol table copied into memory on module load must be exactly
>> the same as the symbol table produced when the patch module was compiled.
>> This is because the relocations in each livepatch relocation section refer
>> to their respective symbols with their symbol indices, and the original
>> symbol indices (and thus the symtab ordering) must be preserved in order
>> for apply_relocate_add() to find the right symbol.
>>
>> Signed-off-by: Jessica Yu <jeyu@redhat.com>
>> ---
>>  include/linux/module.h |  25 ++++++++++
>>  kernel/module.c        | 123 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 146 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 2bb0c30..3daf2b3 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -330,6 +330,15 @@ struct mod_kallsyms {
>>  	char *strtab;
>>  };
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +struct klp_modinfo {
>> +	Elf_Ehdr hdr;
>> +	Elf_Shdr *sechdrs;
>> +	char *secstrings;
>> +	unsigned int symndx;
>> +};
>> +#endif
>> +
>>  struct module {
>>  	enum module_state state;
>>
>> @@ -456,7 +465,11 @@ struct module {
>>  #endif
>>
>>  #ifdef CONFIG_LIVEPATCH
>> +	bool klp; /* Is this a livepatch module? */
>>  	bool klp_alive;
>> +
>> +	/* Elf information */
>> +	struct klp_modinfo *klp_info;
>>  #endif
>>
>>  #ifdef CONFIG_MODULE_UNLOAD
>> @@ -630,6 +643,18 @@ static inline bool module_requested_async_probing(struct module *module)
>>  	return module && module->async_probe_requested;
>>  }
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +static inline bool is_livepatch_module(struct module *mod)
>> +{
>> +	return mod->klp;
>> +}
>> +#else /* !CONFIG_LIVEPATCH */
>> +static inline bool is_livepatch_module(struct module *mod)
>> +{
>> +	return false;
>> +}
>> +#endif /* CONFIG_LIVEPATCH */
>> +
>>  #else /* !CONFIG_MODULES... */
>>
>>  /* Given an address, look for it in the exception tables. */
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 87cfeb2..80b7fd9 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -1971,6 +1971,82 @@ static void module_enable_nx(const struct module *mod) { }
>>  static void module_disable_nx(const struct module *mod) { }
>>  #endif
>>
>> +#ifdef CONFIG_LIVEPATCH
>> +/*
>> + * Persist Elf information about a module. Copy the Elf header,
>> + * section header table, section string table, and symtab section
>> + * index from info to mod->klp_info.
>> + */
>> +static int copy_module_elf(struct module *mod, struct load_info *info)
>> +{
>> +	unsigned int size, symndx;
>> +	int ret;
>> +
>> +	size = sizeof(*mod->klp_info);
>> +	mod->klp_info = kmalloc(size, GFP_KERNEL);
>> +	if (mod->klp_info == NULL)
>> +		return -ENOMEM;
>> +
>> +	/* Elf header */
>> +	size = sizeof(Elf_Ehdr);
>> +	memcpy(&mod->klp_info->hdr, info->hdr, size);
>> +
>> +	/* Elf section header table */
>> +	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
>> +	mod->klp_info->sechdrs = kmalloc(size, GFP_KERNEL);
>> +	if (mod->klp_info->sechdrs == NULL) {
>> +		ret = -ENOMEM;
>> +		goto free_info;
>> +	}
>> +	memcpy(mod->klp_info->sechdrs, info->sechdrs, size);
>> +
>> +	/* Elf section name string table */
>> +	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
>> +	mod->klp_info->secstrings = kmalloc(size, GFP_KERNEL);
>> +	if (mod->klp_info->secstrings == NULL) {
>> +		ret = -ENOMEM;
>> +		goto free_sechdrs;
>> +	}
>> +	memcpy(mod->klp_info->secstrings, info->secstrings, size);
>> +
>> +	/* Elf symbol section index */
>> +	symndx = info->index.sym;
>> +	mod->klp_info->symndx = symndx;

>
>nit: The 'symndx' local variable is superfluous.
>

The symndx variable is just there to keep the line below from reaching
an ugly length (although it is already quite long...)

>> +	mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long) mod->core_kallsyms.symtab;

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

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

On Tue, Mar 22, 2016 at 01:57:01PM -0400, Jessica Yu wrote:
> +++ Josh Poimboeuf [21/03/16 09:06 -0500]:
> >On Wed, Mar 16, 2016 at 03:47:04PM -0400, Jessica Yu wrote:
> >>For livepatch modules, copy Elf section, symbol, and string information
> >>from the load_info struct in the module loader. Persist copies of the
> >>original symbol table and string table.
> >>
> >>Livepatch manages its own relocation sections in order to reuse module
> >>loader code to write relocations. Livepatch modules must preserve Elf
> >>information such as section indices in order to apply livepatch relocation
> >>sections using the module loader's apply_relocate_add() function.
> >>
> >>In order to apply livepatch relocation sections, livepatch modules must
> >>keep a complete copy of their original symbol table in memory. Normally, a
> >>stripped down copy of a module's symbol table (containing only "core"
> >>symbols) is made available through module->core_symtab. But for livepatch
> >>modules, the symbol table copied into memory on module load must be exactly
> >>the same as the symbol table produced when the patch module was compiled.
> >>This is because the relocations in each livepatch relocation section refer
> >>to their respective symbols with their symbol indices, and the original
> >>symbol indices (and thus the symtab ordering) must be preserved in order
> >>for apply_relocate_add() to find the right symbol.
> >>
> >>Signed-off-by: Jessica Yu <jeyu@redhat.com>
> >>---
> >> include/linux/module.h |  25 ++++++++++
> >> kernel/module.c        | 123 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 2 files changed, 146 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/include/linux/module.h b/include/linux/module.h
> >>index 2bb0c30..3daf2b3 100644
> >>--- a/include/linux/module.h
> >>+++ b/include/linux/module.h
> >>@@ -330,6 +330,15 @@ struct mod_kallsyms {
> >> 	char *strtab;
> >> };
> >>
> >>+#ifdef CONFIG_LIVEPATCH
> >>+struct klp_modinfo {
> >>+	Elf_Ehdr hdr;
> >>+	Elf_Shdr *sechdrs;
> >>+	char *secstrings;
> >>+	unsigned int symndx;
> >>+};
> >>+#endif
> >>+
> >> struct module {
> >> 	enum module_state state;
> >>
> >>@@ -456,7 +465,11 @@ struct module {
> >> #endif
> >>
> >> #ifdef CONFIG_LIVEPATCH
> >>+	bool klp; /* Is this a livepatch module? */
> >> 	bool klp_alive;
> >>+
> >>+	/* Elf information */
> >>+	struct klp_modinfo *klp_info;
> >> #endif
> >>
> >> #ifdef CONFIG_MODULE_UNLOAD
> >>@@ -630,6 +643,18 @@ static inline bool module_requested_async_probing(struct module *module)
> >> 	return module && module->async_probe_requested;
> >> }
> >>
> >>+#ifdef CONFIG_LIVEPATCH
> >>+static inline bool is_livepatch_module(struct module *mod)
> >>+{
> >>+	return mod->klp;
> >>+}
> >>+#else /* !CONFIG_LIVEPATCH */
> >>+static inline bool is_livepatch_module(struct module *mod)
> >>+{
> >>+	return false;
> >>+}
> >>+#endif /* CONFIG_LIVEPATCH */
> >>+
> >> #else /* !CONFIG_MODULES... */
> >>
> >> /* Given an address, look for it in the exception tables. */
> >>diff --git a/kernel/module.c b/kernel/module.c
> >>index 87cfeb2..80b7fd9 100644
> >>--- a/kernel/module.c
> >>+++ b/kernel/module.c
> >>@@ -1971,6 +1971,82 @@ static void module_enable_nx(const struct module *mod) { }
> >> static void module_disable_nx(const struct module *mod) { }
> >> #endif
> >>
> >>+#ifdef CONFIG_LIVEPATCH
> >>+/*
> >>+ * Persist Elf information about a module. Copy the Elf header,
> >>+ * section header table, section string table, and symtab section
> >>+ * index from info to mod->klp_info.
> >>+ */
> >>+static int copy_module_elf(struct module *mod, struct load_info *info)
> >>+{
> >>+	unsigned int size, symndx;
> >>+	int ret;
> >>+
> >>+	size = sizeof(*mod->klp_info);
> >>+	mod->klp_info = kmalloc(size, GFP_KERNEL);
> >>+	if (mod->klp_info == NULL)
> >>+		return -ENOMEM;
> >>+
> >>+	/* Elf header */
> >>+	size = sizeof(Elf_Ehdr);
> >>+	memcpy(&mod->klp_info->hdr, info->hdr, size);
> >>+
> >>+	/* Elf section header table */
> >>+	size = sizeof(Elf_Shdr) * info->hdr->e_shnum;
> >>+	mod->klp_info->sechdrs = kmalloc(size, GFP_KERNEL);
> >>+	if (mod->klp_info->sechdrs == NULL) {
> >>+		ret = -ENOMEM;
> >>+		goto free_info;
> >>+	}
> >>+	memcpy(mod->klp_info->sechdrs, info->sechdrs, size);
> >>+
> >>+	/* Elf section name string table */
> >>+	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
> >>+	mod->klp_info->secstrings = kmalloc(size, GFP_KERNEL);
> >>+	if (mod->klp_info->secstrings == NULL) {
> >>+		ret = -ENOMEM;
> >>+		goto free_sechdrs;
> >>+	}
> >>+	memcpy(mod->klp_info->secstrings, info->secstrings, size);
> >>+
> >>+	/* Elf symbol section index */
> >>+	symndx = info->index.sym;
> >>+	mod->klp_info->symndx = symndx;
> 
> >
> >nit: The 'symndx' local variable is superfluous.
> >
> 
> The symndx variable is just there to keep the line below from reaching
> an ugly length (although it is already quite long...)
> 
> >>+	mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long) mod->core_kallsyms.symtab;

Oops, sorry, I missed that line!  It would be good to wrap it like:

	mod->klp_info->sechdrs[symndx].sh_addr = \
		(unsigned long) mod->core_kallsyms.symtab;

-- 
Josh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web