Re: [PATCH] kallsyms: store type information in its own array

[Eugene Loh]

It's been 1.5 weeks.  Could I get some feedback on this patch? Thanks.


On 02/25/2019 11:59 AM, eugene....@oracle.com wrote:

From: Eugene Loh <eugene....@oracle.com>

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 <eugene....@oracle.com>
Reviewed-by: Nick Alcock <nick.alc...@oracle.com>
---
  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);