| 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: <20151112171438.GE4038@treble.hsd1.ky.comcast.net>
Date: Thu, 12 Nov 2015 11:14:38 -0600
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Jessica Yu <jeyu@...hat.com>
Cc: Rusty Russell <rusty@...tcorp.com.au>,
Seth Jennings <sjenning@...hat.com>,
Jiri Kosina <jikos@...nel.org>,
Vojtech Pavlik <vojtech@...e.com>,
Miroslav Benes <mbenes@...e.cz>, linux-api@...r.kernel.org,
live-patching@...r.kernel.org, x86@...nel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/5] module: save load_info for livepatch modules
On Mon, Nov 09, 2015 at 11:45:52PM -0500, 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@...hat.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
> +
The kernel coding style generally frowns on ifdefs inside functions.
IMO it's fine to just remove the ifdef completely here.
> 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
> +
Same here, I think the ifdef can be removed.
> 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
Same here, IMO. (And as Miroslav suggested, klp_prepare_patch_module()
can be renamed to something else (save_info()?) which exists even in the
!CONFIG_LIVEPATCH case.
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists