Re: module: preserve Elf information for livepatch modules

[Jessica Yu]

+++ 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@xxxxxxxxxx>
> > ---
> >  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;