| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160322185516.c24hegtjytxjie37@treble.redhat.com>
Date: Tue, 22 Mar 2016 13:55:16 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Jessica Yu <jeyu@...hat.com>
Cc: Rusty Russell <rusty@...tcorp.com.au>,
Petr Mladek <pmladek@...e.com>, Jiri Kosina <jikos@...nel.org>,
Jonathan Corbet <corbet@....net>,
Miroslav Benes <mbenes@...e.cz>, linux-api@...r.kernel.org,
live-patching@...r.kernel.org, x86@...nel.org,
linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: 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@...hat.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
Powered by blists - more mailing lists