Re: livepatch: reuse module loader code to write relocations
From: Jessica Yu
Date: Tue Feb 09 2016 - 20:22:46 EST
+++ Petr Mladek [09/02/16 15:01 +0100]:
On Wed 2016-02-03 20:11:09, Jessica Yu wrote:
Reuse module loader code to write relocations, thereby eliminating the need
for architecture specific relocation code in livepatch. Specifically, reuse
the apply_relocate_add() function in the module loader to write relocations
instead of duplicating functionality in livepatch's arch-dependent
klp_write_module_reloc() function.
In order to accomplish this, livepatch modules manage their own relocation
sections (marked with the SHF_RELA_LIVEPATCH section flag) and
livepatch-specific symbols (marked with SHN_LIVEPATCH symbol section
index). To apply livepatch relocation sections, livepatch symbols
referenced by relocs are resolved and then apply_relocate_add() is called
to apply those relocations.
In addition, remove x86 livepatch relocation code and the s390
klp_write_module_reloc() function stub. They are no longer needed since
relocation work has been offloaded to module loader.
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 7aa975d..c1fe57c 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -28,6 +28,9 @@
#include <linux/list.h>
#include <linux/kallsyms.h>
#include <linux/livepatch.h>
+#include <linux/elf.h>
+#include <linux/string.h>
+#include <linux/moduleloader.h>
#include <asm/cacheflush.h>
/**
@@ -87,6 +90,166 @@ static bool klp_is_object_loaded(struct klp_object *obj)
return !obj->name || obj->mod;
}
+/*
+ * Check if a livepatch symbol is formatted properly.
+ *
+ * See Documentation/livepatch/module-elf-format.txt for a
+ * detailed outline of requirements.
+ */
+static int klp_check_symbol_format(struct module *pmod, Elf_Sym *sym)
+{
+ size_t len;
+ char *s, *objname, *symname;
+
+ if (sym->st_shndx != SHN_LIVEPATCH)
+ return -EINVAL;
+
+ /*
+ * Livepatch symbol names must follow this format:
+ * .klp.sym.objname.symbol_name,sympos
+ */
+ s = pmod->strtab + sym->st_name;
+ /* [.klp.sym.]objname.symbol_name,sympos */
+ if (!s || strncmp(s, KLP_SYM_PREFIX, KLP_SYM_PREFIX_LEN))
+ return -EINVAL;
+
+ /* .klp.sym.[objname].symbol_name,sympos */
+ objname = s + KLP_SYM_PREFIX_LEN;
+ len = strcspn(objname, ".");
+ if (!(len > 0))
+ return -EINVAL;
+
+ /* .klp.sym.objname.symbol_name,[sympos] */
+ if (!strchr(s, ','))
+ return -EINVAL;
+
+ /* .klp.sym.objname.[symbol_name],sympos */
+ symname = objname + len + 1;
+ len = strcspn(symname, ",");
+ if (!(len > 0))
+ return -EINVAL;
+
+ return 0;
+}
+
+/*
+ * Check if a livepatch relocation section is formatted properly.
+ *
+ * See Documentation/livepatch/module-elf-format.txt for a
+ * detailed outline of requirements.
+ */
+static int klp_check_relasec_format(struct module *pmod, Elf_Shdr *relasec)
+{
+ char *secname;
+ size_t len;
+
+ secname = pmod->klp_info->secstrings + relasec->sh_name;
+ /* [.klp.rela.]objname.section_name */
+ if (!secname || strncmp(secname, KLP_RELASEC_PREFIX,
+ KLP_RELASEC_PREFIX_LEN))
+ return -EINVAL;
+
+ /* .klp.rela.[objname].section_name */
+ len = strcspn(secname + KLP_RELASEC_PREFIX_LEN, ".");
+ if (!(len > 0))
+ return -EINVAL;
+
+ return 0;
+}
+
+/*
+ * Check if obj->name matches the objname encoded in the rela
+ * section name (.klp.rela.[objname].section_name)
+ *
+ * Must pass klp_check_relasec_format() before calling this.
+ */
+static bool klp_relasec_matches_object(struct module *pmod, Elf_Shdr *relasec,
+ struct klp_object *obj)
+{
+ size_t len;
+ const char *obj_objname, *sec_objname, *secname;
+
+ secname = pmod->klp_info->secstrings + relasec->sh_name;
+ /* .klp.rela.[objname].section_name */
+ sec_objname = secname + KLP_RELASEC_PREFIX_LEN;
+ obj_objname = klp_is_module(obj) ? obj->name : "vmlinux";
+
+ /* Get length of the objname encoded in the section name */
+ len = strcspn(sec_objname, ".");
+
+ if (strlen(obj_objname) != len)
+ return false;
+
+ return strncmp(sec_objname, obj_objname, len) ? false : true;
+}
+
+/*
+ * klp_get_* helper functions
+ *
+ * klp_get_* functions extract different components of the name
+ * of a livepatch symbol. The full symbol name from the strtab
+ * is passed in as parameter @s, and @result is filled in with
+ * the extracted component.
+ *
+ * These functions assume a correctly formatted symbol and the
+ * klp_check_symbol_format() test *must* pass before calling any
+ * of these functions.
+ */
+
+/* .klp.sym.[objname].symbol_name,sympos */
+static int klp_get_sym_objname(char *s, char **result)
+{
+ size_t len;
+ char *objname, *objname_start;
+
+ /* .klp.sym.[objname].symbol_name,sympos */
+ objname_start = s + KLP_SYM_PREFIX_LEN;
+ len = strcspn(objname_start, ".");
+ objname = kstrndup(objname_start, len, GFP_KERNEL);
+ if (objname == NULL)
+ return -ENOMEM;
+
+ /* klp_find_object_symbol() treats NULL as vmlinux */
+ if (!strcmp(objname, "vmlinux")) {
+ *result = NULL;
+ kfree(objname);
+ } else
+ *result = objname;
+
+ return 0;
+}
+
+/* .klp.sym.objname.[symbol_name],sympos */
+static int klp_get_symbol_name(char *s, char **result)
+{
+ size_t len;
+ char *objname, *symname;
+
+ /* .klp.sym.[objname].symbol_name,sympos */
+ objname = s + KLP_SYM_PREFIX_LEN;
+ len = strcspn(objname, ".");
+
+ /* .klp.sym.objname.[symbol_name],sympos */
+ symname = objname + len + 1;
+ len = strcspn(symname, ",");
+
+ *result = kstrndup(symname, len, GFP_KERNEL);
+ if (*result == NULL)
+ return -ENOMEM;
+
+ return 0;
+}
+
+/* .klp.sym.objname.symbol_name,[sympos] */
+static int klp_get_sympos(char *s, unsigned long *result)
+{
+ char *sympos;
+
+ /* .klp.sym.symbol_name,[sympos] */
+ sympos = strchr(s, ',') + 1;
+ return kstrtol(sympos, 10, result);
+}
The usage of the helper function is nicely strightforward.
Also I like a lot all the comments with the [] brackets
that highlight what each check or search is for.
But I think that there is a lot of duplicated code in
the check_ and in the get_ functions. Also there is
a lot of strdup/free games.
I agree, I do not really like the repetition and all
the strdup/free's myself.
The length of the elemets is limited by definition.
The functions are called under klp_mutex.
Therefore it might be easier to maintain a two parse
function that would fill static buffers and return
error in case of wrong value.
I mean something like:
static char mod_name[MODULE_NAME_LEN + 1];
static char symbol_name[KSYM_SYMBOL_LEN + 1];
static int
klp_parse_symbol_format(struct module *pmod, Elf_Sym *sym,
char *mod_name, char *symbol_name,
int *sympos)
{
char *substr;
/*
* Livepatch symbol names must follow this format:
* .klp.sym.objname.symbol_name,sympos
*/
s = pmod->strtab + sym->st_name;
/* [.klp.sym.]objname.symbol_name,sympos */
if (!s || strncmp(s, KLP_SYM_PREFIX, KLP_SYM_PREFIX_LEN))
return -EINVAL;
/* .klp.sym.[objname].symbol_name,sympos */
substr = s + KLP_SYM_PREFIX_LEN;
len = strcspn(substr, ".");
if (!len || len > MODULE_NAME_LEN)
return -EINVAL;
strncpy(mod_name, substr, len);
mod_name[len] = '\0';
/* .klp.sym.objname.[symbol_name],sympos */
substr = substr + len + 1;
len = strcspn(substr, ",");
if (!len || len > KSYM_SYMBOL_LEN)
return -EINVAL;
strncpy(symbol_name, substr, len);
mod_name[len] = '\0';
/* .klp.sym.objname.symbol_name,[sympos] */
substr = substr + len + 1;
len = strlen(substr);
if (!len)
return -EINVAL;
return kstrtol(substr, 10, sympos);
}
How does that sound, please?
I like this idea, it is a lot cleaner and more concise
than using all those helper functions. :-)
Thanks,
Jessica