Re: [PATCH v5 09/13] module: Move kallsyms support into a separate file
From: Christophe Leroy <hidden>
Date: 2022-02-10 13:43:47
Also in:
live-patching, lkml
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 <redacted> --- 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
quoted hunk ↗ jump to hunk
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 endifdiff --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()
quoted hunk ↗ jump to hunk
#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
quoted hunk ↗ jump to hunk
+{ + 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;
+}
+...
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
{ #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
quoted hunk ↗ jump to hunk
{ 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@lwn.net>@@ -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) } }
...