Le 09/02/2022 à 18:08, Aaron Tomlin a écrit : > No functional change. > > This patch migrates kallsyms code out of core module > code kernel/module/kallsyms.c > > Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxx> > --- > kernel/module/Makefile | 1 + > kernel/module/internal.h | 27 ++ > kernel/module/kallsyms.c | 502 +++++++++++++++++++++++++++++++++++++ > kernel/module/main.c | 518 +-------------------------------------- > 4 files changed, 534 insertions(+), 514 deletions(-) > create mode 100644 kernel/module/kallsyms.c Checkpatch reports: total: 3 errors, 1 warnings, 26 checks, 1103 lines checked Sparse reports the following: CHECK kernel/module/kallsyms.c kernel/module/kallsyms.c:174:23: warning: incorrect type in assignment (different address spaces) kernel/module/kallsyms.c:174:23: expected struct mod_kallsyms [noderef] __rcu *kallsyms kernel/module/kallsyms.c:174:23: got void * kernel/module/kallsyms.c:176:12: warning: dereference of noderef expression kernel/module/kallsyms.c:177:12: warning: dereference of noderef expression kernel/module/kallsyms.c:179:12: warning: dereference of noderef expression kernel/module/kallsyms.c:180:12: warning: dereference of noderef expression kernel/module/kallsyms.c:189:18: warning: dereference of noderef expression kernel/module/kallsyms.c:190:35: warning: dereference of noderef expression kernel/module/kallsyms.c:191:20: warning: dereference of noderef expression kernel/module/kallsyms.c:196:32: warning: dereference of noderef expression kernel/module/kallsyms.c:199:45: warning: dereference of noderef expression > > diff --git a/kernel/module/Makefile b/kernel/module/Makefile > index 62c9fc91d411..868b13c06920 100644 > --- a/kernel/module/Makefile > +++ b/kernel/module/Makefile > @@ -12,4 +12,5 @@ obj-$(CONFIG_LIVEPATCH) += livepatch.o > obj-$(CONFIG_MODULES_TREE_LOOKUP) += tree_lookup.o > obj-$(CONFIG_STRICT_MODULE_RWX) += strict_rwx.o > obj-$(CONFIG_DEBUG_KMEMLEAK) += debug_kmemleak.o > +obj-$(CONFIG_KALLSYMS) += kallsyms.o > endif > diff --git a/kernel/module/internal.h b/kernel/module/internal.h > index 33d7befd0602..7973666452c3 100644 > --- a/kernel/module/internal.h > +++ b/kernel/module/internal.h > @@ -69,6 +69,11 @@ struct load_info { > }; > > int mod_verify_sig(const void *mod, struct load_info *info); > +struct module *find_module_all(const char *name, size_t len, bool even_unformed); > +unsigned long kernel_symbol_value(const struct kernel_symbol *sym); This function is small enought to be a 'static inline' in internal.h > +int cmp_name(const void *name, const void *sym); > +long get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr, > + unsigned int section); Having a non static function called get_offset() seems dangerous. There are already several get_offset() functions in the kernel allthough they are all static. It takes a struct module as an argument so it could be called module_get_offset() > > #ifdef CONFIG_LIVEPATCH > int copy_module_elf(struct module *mod, struct load_info *info); > @@ -178,3 +183,25 @@ void kmemleak_load_module(const struct module *mod, const struct load_info *info > static inline void __maybe_unused kmemleak_load_module(const struct module *mod, > const struct load_info *info) { } > #endif /* CONFIG_DEBUG_KMEMLEAK */ > + > +#ifdef CONFIG_KALLSYMS > +#ifdef CONFIG_STACKTRACE_BUILD_ID > +void init_build_id(struct module *mod, const struct load_info *info); > +#else /* !CONFIG_STACKTRACE_BUILD_ID */ > +static inline void init_build_id(struct module *mod, const struct load_info *info) { } > + > +#endif > +void layout_symtab(struct module *mod, struct load_info *info); > +void add_kallsyms(struct module *mod, const struct load_info *info); > +bool sect_empty(const Elf_Shdr *sect); sect_empty() is small enough to remain a static inline. > +const char *find_kallsyms_symbol(struct module *mod, unsigned long addr, > + unsigned long *size, unsigned long *offset); This is not used outside kallsyms.c, no need to have it in internal.h > +#else /* !CONFIG_KALLSYMS */ > +static inline void layout_symtab(struct module *mod, struct load_info *info) { } > +static inline void add_kallsyms(struct module *mod, const struct load_info *info) { } > +static inline char *find_kallsyms_symbol(struct module *mod, unsigned long addr, > + unsigned long *size, unsigned long *offset) This is not used outside kallsyms.c, no need to have it when !CONFIG_KALLSYMS > +{ > + return NULL; > +} > +#endif /* CONFIG_KALLSYMS */ > diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c > new file mode 100644 > index 000000000000..ed28f6310701 > --- /dev/null > +++ b/kernel/module/kallsyms.c > @@ -0,0 +1,502 @@ ... > + > +/* 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) This function is called from main.c, it can't be static and must be defined in internal.h > +{ > + unsigned int i; > + struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms); > + > + for (i = 0; i < kallsyms->num_symtab; i++) { > + const Elf_Sym *sym = &kallsyms->symtab[i]; > + > + if (strcmp(name, kallsyms_symbol_name(kallsyms, i)) == 0 && > + sym->st_shndx != SHN_UNDEF) > + return kallsyms_symbol_value(sym); > + } > + return 0; > +} > + ... > diff --git a/kernel/module/main.c b/kernel/module/main.c > index c9931479e2eb..378dd7fd1b6a 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -285,7 +285,7 @@ static bool check_exported_symbol(const struct symsearch *syms, > return true; > } > > -static unsigned long kernel_symbol_value(const struct kernel_symbol *sym) > +unsigned long kernel_symbol_value(const struct kernel_symbol *sym) This function is small enought to become a 'static inline' in internal.h > { > #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS > return (unsigned long)offset_to_ptr(&sym->value_offset); > @@ -314,7 +314,7 @@ static const char *kernel_symbol_namespace(const struct kernel_symbol *sym) > #endif > } > > -static int cmp_name(const void *name, const void *sym) > +int cmp_name(const void *name, const void *sym) This function is small enought to become a 'static inline' in internal.h > { > return strcmp(name, kernel_symbol_name(sym)); > } > @@ -384,7 +384,7 @@ static bool find_symbol(struct find_symbol_arg *fsa) > * Search for module by name: must hold module_mutex (or preempt disabled > * for read-only access). > */ > -static struct module *find_module_all(const char *name, size_t len, > +struct module *find_module_all(const char *name, size_t len, > bool even_unformed) > { > struct module *mod; > @@ -1291,13 +1291,6 @@ resolve_symbol_wait(struct module *mod, > return ksym; > } > > -#ifdef CONFIG_KALLSYMS > -static inline bool sect_empty(const Elf_Shdr *sect) > -{ > - return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0; > -} > -#endif > - > /* > * /sys/module/foo/sections stuff > * J. Corbet <corbet@xxxxxxx> > @@ -2061,7 +2054,7 @@ unsigned int __weak arch_mod_section_prepend(struct module *mod, > } > > /* Update size with this section: return offset. */ > -static long get_offset(struct module *mod, unsigned int *size, > +long get_offset(struct module *mod, unsigned int *size, > Elf_Shdr *sechdr, unsigned int section) > { > long ret; > @@ -2263,228 +2256,6 @@ static void free_modinfo(struct module *mod) > } > } > ...