module: make it clearer when we're handling kallsyms symbols vs exported symbols
PATCH v2 1/1 module: make it clearer when we're handling kallsyms symbols vs exported symbols
The module loader internally works with both exported symbols represented as struct kernel_symbol, as well as Elf symbols from a module's symbol table. It's hard to distinguish sometimes which type of symbol we're handling given that some helper function names are not consistent or helpful. Take get_ksymbol() for instance - are we looking for an exported symbol or a kallsyms symbol here? Or symname() and kernel_symbol_name() - which function handles an exported symbol and which one an Elf symbol?
Clean up and unify the function naming scheme a bit to make it clear which kind of symbol we're handling. This change only affects static functions internal to the module loader.
---
v2: renamed kallsyms_find_* funcs to find_kallsyms_* to follow the
already existing <verb>_<noun> naming convention in module.c
kernel/module.c | 78 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 34 deletions(-)
kernel/module.c +44 -34
@@ -495,9 +495,9 @@ struct find_symbol_arg {const struct kernel_symbol *sym; }; -static bool check_symbol (const struct symsearch *syms, -struct module *owner, -unsigned int symnum,void *data) +static bool check_exported_symbol (const struct symsearch *syms, +struct module *owner, +unsigned int symnum,void *data) {struct find_symbol_arg *fsa = data;
@@ -555,9 +555,9 @@ static int cmp_name(const void *va, const void *vb)return strcmp (a,kernel_symbol_name (b)); } -static bool find_symbol_in_section (const struct symsearch *syms, -struct module *owner, -void *data) +static bool find_exported_symbol_in_section (const struct symsearch *syms, +struct module *owner, +void *data) {struct find_symbol_arg *fsa = data;struct kernel_symbol *sym;
@@ -565,13 +565,14 @@ static bool find_symbol_in_section(const struct symsearch *syms, sym =bsearch (fsa->name, syms->start, syms->stop - syms->start,sizeof (struct kernel_symbol ), cmp_name); -if (sym !=NULL &&check_symbol (syms, owner, sym - syms->start, data)) +if (sym !=NULL &&check_exported_symbol (syms, owner, + sym - syms->start, data))return true;return false; } -/* Find a symbol and return it, along with, (optional) crc and +/* Find an exported symbol and return it, along with, (optional) crc and * (optional) module which owns it. Needs preempt disabled or module_mutex. */ const struct kernel_symbol *find_symbol(const char *name, struct module **owner,
@@ -585,7 +586,7 @@ const struct kernel_symbol *find_symbol(const char *name, fsa.gplok = gplok; fsa.warn = warn; -if (each_symbol_section (find_symbol_in_section, &fsa)) { +if (each_symbol_section (find_exported_symbol_in_section, &fsa)) {if (owner) *owner = fsa.owner;if (crc)
@@ -2198,7 +2199,7 @@ EXPORT_SYMBOL_GPL(__symbol_get);* * You must hold the module_mutex. */ -static int verify_export_symbols (struct module *mod) +static int verify_exported_symbols (struct module *mod) {unsigned int i;struct module *owner;
@@ -2519,10 +2520,10 @@ static void free_modinfo(struct module *mod)#ifdef CONFIG_KALLSYMS -/* lookup symbol in given range of kernel_symbols */ -static const struct kernel_symbol *lookup_symbol (const char *name, -const struct kernel_symbol *start, -const struct kernel_symbol *stop) +/* Lookup exported symbol in given range of kernel_symbols */ +static const struct kernel_symbol *lookup_exported_symbol (const char *name, +const struct kernel_symbol *start, +const struct kernel_symbol *stop) {return bsearch (name, start, stop - start,sizeof (struct kernel_symbol ), cmp_name);
@@ -2533,9 +2534,10 @@ static int is_exported(const char *name, unsigned long value, {const struct kernel_symbol *ks;if (!mod) - ks =lookup_symbol (name, __start___ksymtab, __stop___ksymtab); + ks =lookup_exported_symbol (name, __start___ksymtab, __stop___ksymtab);else - ks =lookup_symbol (name, mod->syms, mod->syms + mod->num_syms); + ks =lookup_exported_symbol (name, mod->syms, mod->syms + mod->num_syms); +return ks !=NULL &&kernel_symbol_value (ks) == value; }
@@ -3592,7 +3594,7 @@ static int complete_formation(struct module *mod, struct load_info *info)mutex_lock (&module_mutex);/* Find duplicate symbols (must be called under lock). */ - err =verify_export_symbols (mod); + err =verify_exported_symbols (mod);if (err <0 ) goto out;
@@ -3911,15 +3913,19 @@ static inline int is_arm_mapping_symbol(const char *str) && (str[2 ] =='\0' || str[2 ] =='.' ); } -static const char *symname (struct mod_kallsyms *kallsyms,unsigned int symnum) +static const char *kallsyms_symbol_name (struct mod_kallsyms *kallsyms,unsigned int symnum) {return kallsyms->strtab + kallsyms->symtab[symnum].st_name; } -static const char *get_ksymbol (struct module *mod, -unsigned long addr, -unsigned long *size, -unsigned long *offset) +/* +* Given a module and address, find the corresponding symbol and return its name +* while providing its size and offset if needed. +*/ +static const char *find_kallsyms_symbol (struct module *mod, +unsigned long addr, +unsigned long *size, +unsigned long *offset) {unsigned int i, best =0 ;unsigned long nextval;
@@ -3939,8 +3945,8 @@ static const char *get_ksymbol(struct module *mod,/* We ignore unnamed symbols: they're uninformative * and inserted at a whim. */ -if (*symname (kallsyms, i) =='\0' - ||is_arm_mapping_symbol (symname (kallsyms, i))) +if (*kallsyms_symbol_name (kallsyms, i) =='\0' + ||is_arm_mapping_symbol (kallsyms_symbol_name (kallsyms, i)))continue ; if (kallsyms->symtab[i].st_value <= addr
@@ -3958,7 +3964,8 @@ static const char *get_ksymbol(struct module *mod, *size = nextval - kallsyms->symtab[best].st_value;if (offset) *offset = addr - kallsyms->symtab[best].st_value; -return symname (kallsyms, best); + +return kallsyms_symbol_name (kallsyms, best); } void * __weak dereference_module_function_descriptor(struct module *mod,
@@ -3983,7 +3990,8 @@ const char *module_address_lookup(unsigned long addr,if (mod) {if (modname) *modname = mod->name; - ret =get_ksymbol (mod, addr, size, offset); + + ret =find_kallsyms_symbol (mod, addr, size, offset); }/* Make a copy in here where it's safe */ if (ret) {
@@ -4006,9 +4014,10 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)if (within_module (addr, mod)) {const char *sym; - sym =get_ksymbol (mod, addr,NULL ,NULL ); + sym =find_kallsyms_symbol (mod, addr,NULL ,NULL );if (!sym) goto out; +strlcpy (symname, sym,KSYM_NAME_LEN );preempt_enable ();return 0 ;
@@ -4031,7 +4040,7 @@ int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,if (within_module (addr, mod)) {const char *sym; - sym =get_ksymbol (mod, addr, size, offset); + sym =find_kallsyms_symbol (mod, addr, size, offset);if (!sym) goto out;if (modname)
@@ -4062,7 +4071,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,if (symnum < kallsyms->num_symtab) { *value = kallsyms->symtab[symnum].st_value; *type = kallsyms->symtab[symnum].st_info; -strlcpy (name,symname (kallsyms, symnum),KSYM_NAME_LEN ); +strlcpy (name,kallsyms_symbol_name (kallsyms, symnum),KSYM_NAME_LEN );strlcpy (module_name, mod->name,MODULE_NAME_LEN ); *exported =is_exported (name, *value, mod);preempt_enable ();
@@ -4074,13 +4083,14 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,return -ERANGE ; } -static unsigned long mod_find_symname (struct module *mod,const char *name) +/* Given a module and name of symbol, find and return the symbol's value */ +static unsigned long find_kallsyms_symbol_value (struct module *mod,const char *name) {unsigned int i;struct mod_kallsyms *kallsyms =rcu_dereference_sched (mod->kallsyms);for (i =0 ; i < kallsyms->num_symtab; i++) -if (strcmp (name,symname (kallsyms, i)) ==0 && +if (strcmp (name,kallsyms_symbol_name (kallsyms, i)) ==0 && kallsyms->symtab[i].st_shndx !=SHN_UNDEF )return kallsyms->symtab[i].st_value;return 0 ;
@@ -4097,12 +4107,12 @@ unsigned long module_kallsyms_lookup_name(const char *name)preempt_disable ();if ((colon =strnchr (name,MODULE_NAME_LEN ,':' )) !=NULL ) {if ((mod =find_module_all (name, colon - name, false)) !=NULL ) - ret =mod_find_symname (mod, colon+1 ); + ret =find_kallsyms_symbol_value (mod, colon+1 ); }else {list_for_each_entry_rcu (mod, &modules, list) {if (mod->state ==MODULE_STATE_UNFORMED )continue ; -if ((ret =mod_find_symname (mod, name)) !=0 ) +if ((ret =find_kallsyms_symbol_value (mod, name)) !=0 )break ; } }
@@ -4131,7 +4141,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,if (kallsyms->symtab[i].st_shndx ==SHN_UNDEF )continue ; - ret =fn (data,symname (kallsyms, i), + ret =fn (data,kallsyms_symbol_name (kallsyms, i), mod, kallsyms->symtab[i].st_value);if (ret !=0 )return ret;
Signature · 1 line
2.16.4
Quoted 9 lines · Jessica Yu
The module loader internally works with both exported symbols represented as struct
kernel_symbol, as well as Elf symbols from a module's symbol table. It's hard to distinguish sometimes which type of symbol we're handling given that some helper function names are not consistent or helpful. Takeget_ksymbol()for instance - are we looking for an exported symbol or a kallsyms symbol here? Orsymname()andkernel_symbol_name()- which function handles an exported symbol and which one an Elf symbol?
Quoted 5 lines
Clean up and unify the function naming scheme a bit to make it clear which kind of symbol we're handling. This change only affects static functions internal to the module loader.
Signed-off-by Jessica Yu <[email protected]>
M
Jessica Yu
Applied+++ Miroslav Benes [29/11/18 09:18 +0100]:
Quoted 11 lines · Jessica Yu
Quoted 10 lines
The module loader internally works with both exported symbols represented as struct
kernel_symbol, as well as Elf symbols from a module's symbol table. It's hard to distinguish sometimes which type of symbol we're handling given that some helper function names are not consistent or helpful. Takeget_ksymbol()for instance - are we looking for an exported symbol or a kallsyms symbol here? Orsymname()andkernel_symbol_name()- which function handles an exported symbol and which one an Elf symbol?
Quoted 7 lines
Quoted 5 lines
Clean up and unify the function naming scheme a bit to make it clear which kind of symbol we're handling. This change only affects static functions internal to the module loader.
Signed-off-by Jessica Yu <[email protected]>Reviewed-by Miroslav Benes <[email protected]>
Applied, thanks!
Jessica