| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 10 Feb 2022 13:43:40 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: "mcgrof@...nel.org" <mcgrof@...nel.org>,
Aaron Tomlin <atomlin@...hat.com>
CC: "cl@...ux.com" <cl@...ux.com>,
"pmladek@...e.com" <pmladek@...e.com>,
"mbenes@...e.cz" <mbenes@...e.cz>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"jeyu@...nel.org" <jeyu@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
"live-patching@...r.kernel.org" <live-patching@...r.kernel.org>,
"atomlin@...mlin.com" <atomlin@...mlin.com>,
"ghalat@...hat.com" <ghalat@...hat.com>,
"allen.lkml@...il.com" <allen.lkml@...il.com>,
"void@...ifault.com" <void@...ifault.com>,
"joe@...ches.com" <joe@...ches.com>,
"msuchanek@...e.de" <msuchanek@...e.de>,
"oleksandr@...alenko.name" <oleksandr@...alenko.name>
Subject: Re: [PATCH v5 09/13] module: Move kallsyms support into a separate
file
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@...hat.com>
> ---
> 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@....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)
> }
> }
>
...
Powered by blists - more mailing lists