From: Eugene Loh <[email protected]>
When a module is loaded, its symbols' Elf_Sym information is stored
in a symtab. Further, type information is also captured. Since
Elf_Sym has no type field, historically the st_info field has been
hijacked for storing type: st_info was overwritten.
commit 5439c985c5a83a8419f762115afdf560ab72a452 ("module: Overwrite
st_size instead of st_info") changes that practice, as its one-liner
indicates. Unfortunately, this change overwrites symbol size,
information that a tool like DTrace expects to find.
Allocate a typetab array to store type information so that no Elf_Sym
field needs to be overwritten.
Fixes: 5439c985c5a8 ("module: Overwrite st_size instead of st_info")
Signed-off-by: Eugene Loh <[email protected]>
Reviewed-by: Nick Alcock <[email protected]>
---
include/linux/module.h | 1 +
kernel/module-internal.h | 2 +-
kernel/module.c | 21 ++++++++++++++-------
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index f5bc4c0..9c1bc21 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -315,6 +315,7 @@ struct mod_kallsyms {
Elf_Sym *symtab;
unsigned int num_symtab;
char *strtab;
+ char *typetab;
};
#ifdef CONFIG_LIVEPATCH
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 79c9be2..67828af 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -20,7 +20,7 @@ struct load_info {
unsigned long len;
Elf_Shdr *sechdrs;
char *secstrings, *strtab;
- unsigned long symoffs, stroffs;
+ unsigned long symoffs, stroffs, init_typeoff, core_typeoff;
struct _ddebug *debug;
unsigned int num_debug;
bool sig_ok;
diff --git a/kernel/module.c b/kernel/module.c
index 2ad1b52..c7c1bcd 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2647,6 +2647,8 @@ static void layout_symtab(struct module *mod, struct load_info *info)
info->symoffs = ALIGN(mod->core_layout.size, symsect->sh_addralign ?: 1);
info->stroffs = mod->core_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
mod->core_layout.size += strtab_size;
+ info->core_typeoff = mod->core_layout.size;
+ mod->core_layout.size += ndst * sizeof(char);
mod->core_layout.size = debug_align(mod->core_layout.size);
/* Put string table section at end of init part of module. */
@@ -2660,6 +2662,8 @@ static void layout_symtab(struct module *mod, struct load_info *info)
__alignof__(struct mod_kallsyms));
info->mod_kallsyms_init_off = mod->init_layout.size;
mod->init_layout.size += sizeof(struct mod_kallsyms);
+ info->init_typeoff = mod->init_layout.size;
+ mod->init_layout.size += nsrc * sizeof(char);
mod->init_layout.size = debug_align(mod->init_layout.size);
}
@@ -2683,20 +2687,23 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
mod->kallsyms->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
/* Make sure we get permanent strtab: don't use info->strtab. */
mod->kallsyms->strtab = (void *)info->sechdrs[info->index.str].sh_addr;
+ mod->kallsyms->typetab = mod->init_layout.base + info->init_typeoff;
- /* Set types up while we still have access to sections. */
- for (i = 0; i < mod->kallsyms->num_symtab; i++)
- mod->kallsyms->symtab[i].st_size
- = elf_type(&mod->kallsyms->symtab[i], info);
-
- /* Now populate the cut down core kallsyms for after init. */
+ /*
+ * Now populate the cut down core kallsyms for after init
+ * and set types up while we still have access to sections.
+ */
mod->core_kallsyms.symtab = dst = mod->core_layout.base + info->symoffs;
mod->core_kallsyms.strtab = s = mod->core_layout.base + info->stroffs;
+ mod->core_kallsyms.typetab = mod->core_layout.base + info->core_typeoff;
src = mod->kallsyms->symtab;
for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
+ mod->kallsyms->typetab[i] = elf_type(src + i, info);
if (i == 0 || is_livepatch_module(mod) ||
is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
info->index.pcpu)) {
+ mod->core_kallsyms.typetab[ndst] =
+ mod->kallsyms->typetab[i];
dst[ndst] = src[i];
dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
s += strlcpy(s, &mod->kallsyms->strtab[src[i].st_name],
@@ -4084,7 +4091,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
const Elf_Sym *sym = &kallsyms->symtab[symnum];
*value = kallsyms_symbol_value(sym);
- *type = sym->st_size;
+ *type = kallsyms->typetab[symnum];
strlcpy(name, kallsyms_symbol_name(kallsyms, symnum), KSYM_NAME_LEN);
strlcpy(module_name, mod->name, MODULE_NAME_LEN);
*exported = is_exported(name, *value, mod);
--
1.8.3.1
It's been 1.5 weeks. Could I get some feedback on this patch? Thanks.
On 02/25/2019 11:59 AM, [email protected] wrote:
> From: Eugene Loh <[email protected]>
>
> When a module is loaded, its symbols' Elf_Sym information is stored
> in a symtab. Further, type information is also captured. Since
> Elf_Sym has no type field, historically the st_info field has been
> hijacked for storing type: st_info was overwritten.
>
> commit 5439c985c5a83a8419f762115afdf560ab72a452 ("module: Overwrite
> st_size instead of st_info") changes that practice, as its one-liner
> indicates. Unfortunately, this change overwrites symbol size,
> information that a tool like DTrace expects to find.
>
> Allocate a typetab array to store type information so that no Elf_Sym
> field needs to be overwritten.
>
> Fixes: 5439c985c5a8 ("module: Overwrite st_size instead of st_info")
> Signed-off-by: Eugene Loh <[email protected]>
> Reviewed-by: Nick Alcock <[email protected]>
> ---
> include/linux/module.h | 1 +
> kernel/module-internal.h | 2 +-
> kernel/module.c | 21 ++++++++++++++-------
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index f5bc4c0..9c1bc21 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -315,6 +315,7 @@ struct mod_kallsyms {
> Elf_Sym *symtab;
> unsigned int num_symtab;
> char *strtab;
> + char *typetab;
> };
>
> #ifdef CONFIG_LIVEPATCH
> diff --git a/kernel/module-internal.h b/kernel/module-internal.h
> index 79c9be2..67828af 100644
> --- a/kernel/module-internal.h
> +++ b/kernel/module-internal.h
> @@ -20,7 +20,7 @@ struct load_info {
> unsigned long len;
> Elf_Shdr *sechdrs;
> char *secstrings, *strtab;
> - unsigned long symoffs, stroffs;
> + unsigned long symoffs, stroffs, init_typeoff, core_typeoff;
> struct _ddebug *debug;
> unsigned int num_debug;
> bool sig_ok;
> diff --git a/kernel/module.c b/kernel/module.c
> index 2ad1b52..c7c1bcd 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2647,6 +2647,8 @@ static void layout_symtab(struct module *mod, struct load_info *info)
> info->symoffs = ALIGN(mod->core_layout.size, symsect->sh_addralign ?: 1);
> info->stroffs = mod->core_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
> mod->core_layout.size += strtab_size;
> + info->core_typeoff = mod->core_layout.size;
> + mod->core_layout.size += ndst * sizeof(char);
> mod->core_layout.size = debug_align(mod->core_layout.size);
>
> /* Put string table section at end of init part of module. */
> @@ -2660,6 +2662,8 @@ static void layout_symtab(struct module *mod, struct load_info *info)
> __alignof__(struct mod_kallsyms));
> info->mod_kallsyms_init_off = mod->init_layout.size;
> mod->init_layout.size += sizeof(struct mod_kallsyms);
> + info->init_typeoff = mod->init_layout.size;
> + mod->init_layout.size += nsrc * sizeof(char);
> mod->init_layout.size = debug_align(mod->init_layout.size);
> }
>
> @@ -2683,20 +2687,23 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
> mod->kallsyms->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
> /* Make sure we get permanent strtab: don't use info->strtab. */
> mod->kallsyms->strtab = (void *)info->sechdrs[info->index.str].sh_addr;
> + mod->kallsyms->typetab = mod->init_layout.base + info->init_typeoff;
>
> - /* Set types up while we still have access to sections. */
> - for (i = 0; i < mod->kallsyms->num_symtab; i++)
> - mod->kallsyms->symtab[i].st_size
> - = elf_type(&mod->kallsyms->symtab[i], info);
> -
> - /* Now populate the cut down core kallsyms for after init. */
> + /*
> + * Now populate the cut down core kallsyms for after init
> + * and set types up while we still have access to sections.
> + */
> mod->core_kallsyms.symtab = dst = mod->core_layout.base + info->symoffs;
> mod->core_kallsyms.strtab = s = mod->core_layout.base + info->stroffs;
> + mod->core_kallsyms.typetab = mod->core_layout.base + info->core_typeoff;
> src = mod->kallsyms->symtab;
> for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
> + mod->kallsyms->typetab[i] = elf_type(src + i, info);
> if (i == 0 || is_livepatch_module(mod) ||
> is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
> info->index.pcpu)) {
> + mod->core_kallsyms.typetab[ndst] =
> + mod->kallsyms->typetab[i];
> dst[ndst] = src[i];
> dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
> s += strlcpy(s, &mod->kallsyms->strtab[src[i].st_name],
> @@ -4084,7 +4091,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
> const Elf_Sym *sym = &kallsyms->symtab[symnum];
>
> *value = kallsyms_symbol_value(sym);
> - *type = sym->st_size;
> + *type = kallsyms->typetab[symnum];
> strlcpy(name, kallsyms_symbol_name(kallsyms, symnum), KSYM_NAME_LEN);
> strlcpy(module_name, mod->name, MODULE_NAME_LEN);
> *exported = is_exported(name, *value, mod);
+++ Eugene Loh [07/03/19 15:37 -0800]:
>It's been 1.5 weeks. Could I get some feedback on this patch? Thanks.
Apologies, I was on vacation the past week.
Added some CC's from the last time we talked about overwriting the
st_size/st_info Elf_Sym fields, in case anyone else has comments.
>On 02/25/2019 11:59 AM, [email protected] wrote:
>>From: Eugene Loh <[email protected]>
>>
>>When a module is loaded, its symbols' Elf_Sym information is stored
>>in a symtab. Further, type information is also captured. Since
>>Elf_Sym has no type field, historically the st_info field has been
>>hijacked for storing type: st_info was overwritten.
>>
>>commit 5439c985c5a83a8419f762115afdf560ab72a452 ("module: Overwrite
>>st_size instead of st_info") changes that practice, as its one-liner
>>indicates. Unfortunately, this change overwrites symbol size,
>>information that a tool like DTrace expects to find.
>>
>>Allocate a typetab array to store type information so that no Elf_Sym
>>field needs to be overwritten.
>>
>>Fixes: 5439c985c5a8 ("module: Overwrite st_size instead of st_info")
>>Signed-off-by: Eugene Loh <[email protected]>
>>Reviewed-by: Nick Alcock <[email protected]>
>>---
>> include/linux/module.h | 1 +
>> kernel/module-internal.h | 2 +-
>> kernel/module.c | 21 ++++++++++++++-------
>> 3 files changed, 16 insertions(+), 8 deletions(-)
>>
>>diff --git a/include/linux/module.h b/include/linux/module.h
>>index f5bc4c0..9c1bc21 100644
>>--- a/include/linux/module.h
>>+++ b/include/linux/module.h
>>@@ -315,6 +315,7 @@ struct mod_kallsyms {
>> Elf_Sym *symtab;
>> unsigned int num_symtab;
>> char *strtab;
>>+ char *typetab;
>> };
>> #ifdef CONFIG_LIVEPATCH
>>diff --git a/kernel/module-internal.h b/kernel/module-internal.h
>>index 79c9be2..67828af 100644
>>--- a/kernel/module-internal.h
>>+++ b/kernel/module-internal.h
>>@@ -20,7 +20,7 @@ struct load_info {
>> unsigned long len;
>> Elf_Shdr *sechdrs;
>> char *secstrings, *strtab;
>>- unsigned long symoffs, stroffs;
>>+ unsigned long symoffs, stroffs, init_typeoff, core_typeoff;
Small nit: typeoff to typeoffs to match symoffs and stroffs.
Otherwise the rest looks fine to me. I guess it's about time we get
rid of the Elf_Sym field hijacking anyway :-)
Thanks!
Jessica
>> struct _ddebug *debug;
>> unsigned int num_debug;
>> bool sig_ok;
>>diff --git a/kernel/module.c b/kernel/module.c
>>index 2ad1b52..c7c1bcd 100644
>>--- a/kernel/module.c
>>+++ b/kernel/module.c
>>@@ -2647,6 +2647,8 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>> info->symoffs = ALIGN(mod->core_layout.size, symsect->sh_addralign ?: 1);
>> info->stroffs = mod->core_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
>> mod->core_layout.size += strtab_size;
>>+ info->core_typeoff = mod->core_layout.size;
>>+ mod->core_layout.size += ndst * sizeof(char);
>> mod->core_layout.size = debug_align(mod->core_layout.size);
>> /* Put string table section at end of init part of module. */
>>@@ -2660,6 +2662,8 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>> __alignof__(struct mod_kallsyms));
>> info->mod_kallsyms_init_off = mod->init_layout.size;
>> mod->init_layout.size += sizeof(struct mod_kallsyms);
>>+ info->init_typeoff = mod->init_layout.size;
>>+ mod->init_layout.size += nsrc * sizeof(char);
>> mod->init_layout.size = debug_align(mod->init_layout.size);
>> }
>>@@ -2683,20 +2687,23 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
>> mod->kallsyms->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
>> /* Make sure we get permanent strtab: don't use info->strtab. */
>> mod->kallsyms->strtab = (void *)info->sechdrs[info->index.str].sh_addr;
>>+ mod->kallsyms->typetab = mod->init_layout.base + info->init_typeoff;
>>- /* Set types up while we still have access to sections. */
>>- for (i = 0; i < mod->kallsyms->num_symtab; i++)
>>- mod->kallsyms->symtab[i].st_size
>>- = elf_type(&mod->kallsyms->symtab[i], info);
>>-
>>- /* Now populate the cut down core kallsyms for after init. */
>>+ /*
>>+ * Now populate the cut down core kallsyms for after init
>>+ * and set types up while we still have access to sections.
>>+ */
>> mod->core_kallsyms.symtab = dst = mod->core_layout.base + info->symoffs;
>> mod->core_kallsyms.strtab = s = mod->core_layout.base + info->stroffs;
>>+ mod->core_kallsyms.typetab = mod->core_layout.base + info->core_typeoff;
>> src = mod->kallsyms->symtab;
>> for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
>>+ mod->kallsyms->typetab[i] = elf_type(src + i, info);
>> if (i == 0 || is_livepatch_module(mod) ||
>> is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
>> info->index.pcpu)) {
>>+ mod->core_kallsyms.typetab[ndst] =
>>+ mod->kallsyms->typetab[i];
>> dst[ndst] = src[i];
>> dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
>> s += strlcpy(s, &mod->kallsyms->strtab[src[i].st_name],
>>@@ -4084,7 +4091,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
>> const Elf_Sym *sym = &kallsyms->symtab[symnum];
>> *value = kallsyms_symbol_value(sym);
>>- *type = sym->st_size;
>>+ *type = kallsyms->typetab[symnum];
>> strlcpy(name, kallsyms_symbol_name(kallsyms, symnum), KSYM_NAME_LEN);
>> strlcpy(module_name, mod->name, MODULE_NAME_LEN);
>> *exported = is_exported(name, *value, mod);
>