[PATCH v5 07/13] module: Move extra signature support out of core code

[PATCH v5 07/13] module: Move extra signature support out of core code

[Aaron Tomlin]

No functional change.

This patch migrates additional module signature check
code from core module code into kernel/module/signing.c.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 include/linux/module.h   |  1 +
 kernel/module/internal.h |  9 +++++
 kernel/module/main.c     | 87 ----------------------------------------
 kernel/module/signing.c  | 75 ++++++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+), 87 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index fd6161d78127..aea0ffd94a41 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -863,6 +863,7 @@ static inline bool module_sig_ok(struct module *module)
 {
 	return true;
 }
+#define sig_enforce false
 #endif	/* CONFIG_MODULE_SIG */
 
 int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 99204447ce86..6d5891cc8421 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -162,3 +162,12 @@ static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 	return 0;
 }
 #endif /* CONFIG_STRICT_MODULE_RWX */
+
+#ifdef CONFIG_MODULE_SIG
+int module_sig_check(struct load_info *info, int flags);
+#else /* !CONFIG_MODULE_SIG */
+static inline int module_sig_check(struct load_info *info, int flags)
+{
+	return 0;
+}
+#endif /* !CONFIG_MODULE_SIG */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index abdedc15f4f1..403f2aacb3f6 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -23,7 +23,6 @@
 #include <linux/vmalloc.h>
 #include <linux/elf.h>
 #include <linux/proc_fs.h>
-#include <linux/security.h>
 #include <linux/seq_file.h>
 #include <linux/syscalls.h>
 #include <linux/fcntl.h>
@@ -124,28 +123,6 @@ static void module_assert_mutex_or_preempt(void)
 #endif
 }
 
-#ifdef CONFIG_MODULE_SIG
-static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
-module_param(sig_enforce, bool_enable_only, 0644);
-
-void set_module_sig_enforced(void)
-{
-	sig_enforce = true;
-}
-#else
-#define sig_enforce false
-#endif
-
-/*
- * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
- * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
- */
-bool is_module_sig_enforced(void)
-{
-	return sig_enforce;
-}
-EXPORT_SYMBOL(is_module_sig_enforced);
-
 /* Block module loading/unloading? */
 int modules_disabled = 0;
 core_param(nomodule, modules_disabled, bint, 0);
@@ -2565,70 +2542,6 @@ static inline void kmemleak_load_module(const struct module *mod,
 }
 #endif
 
-#ifdef CONFIG_MODULE_SIG
-static int module_sig_check(struct load_info *info, int flags)
-{
-	int err = -ENODATA;
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
-	const char *reason;
-	const void *mod = info->hdr;
-	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
-				       MODULE_INIT_IGNORE_VERMAGIC);
-	/*
-	 * Do not allow mangled modules as a module with version information
-	 * removed is no longer the module that was signed.
-	 */
-	if (!mangled_module &&
-	    info->len > markerlen &&
-	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
-		/* We truncate the module to discard the signature */
-		info->len -= markerlen;
-		err = mod_verify_sig(mod, info);
-		if (!err) {
-			info->sig_ok = true;
-			return 0;
-		}
-	}
-
-	/*
-	 * We don't permit modules to be loaded into the trusted kernels
-	 * without a valid signature on them, but if we're not enforcing,
-	 * certain errors are non-fatal.
-	 */
-	switch (err) {
-	case -ENODATA:
-		reason = "unsigned module";
-		break;
-	case -ENOPKG:
-		reason = "module with unsupported crypto";
-		break;
-	case -ENOKEY:
-		reason = "module with unavailable key";
-		break;
-
-	default:
-		/*
-		 * All other errors are fatal, including lack of memory,
-		 * unparseable signatures, and signature check failures --
-		 * even if signatures aren't required.
-		 */
-		return err;
-	}
-
-	if (is_module_sig_enforced()) {
-		pr_notice("Loading of %s is rejected\n", reason);
-		return -EKEYREJECTED;
-	}
-
-	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
-}
-#else /* !CONFIG_MODULE_SIG */
-static int module_sig_check(struct load_info *info, int flags)
-{
-	return 0;
-}
-#endif /* !CONFIG_MODULE_SIG */
-
 static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr)
 {
 #if defined(CONFIG_64BIT)
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index 8aeb6d2ee94b..ff41541e982a 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -11,9 +11,28 @@
 #include <linux/module_signature.h>
 #include <linux/string.h>
 #include <linux/verification.h>
+#include <linux/security.h>
 #include <crypto/public_key.h>
 #include "internal.h"
 
+static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
+module_param(sig_enforce, bool_enable_only, 0644);
+
+/*
+ * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
+ * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
+ */
+bool is_module_sig_enforced(void)
+{
+	return sig_enforce;
+}
+EXPORT_SYMBOL(is_module_sig_enforced);
+
+void set_module_sig_enforced(void)
+{
+	sig_enforce = true;
+}
+
 /*
  * Verify the signature on a module.
  */
@@ -43,3 +62,59 @@ int mod_verify_sig(const void *mod, struct load_info *info)
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }
+
+int module_sig_check(struct load_info *info, int flags)
+{
+	int err = -ENODATA;
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+	const char *reason;
+	const void *mod = info->hdr;
+
+	/*
+	 * Require flags == 0, as a module with version information
+	 * removed is no longer the module that was signed
+	 */
+	if (flags == 0 &&
+	    info->len > markerlen &&
+	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
+		/* We truncate the module to discard the signature */
+		info->len -= markerlen;
+		err = mod_verify_sig(mod, info);
+		if (!err) {
+			info->sig_ok = true;
+			return 0;
+		}
+	}
+
+	/*
+	 * We don't permit modules to be loaded into the trusted kernels
+	 * without a valid signature on them, but if we're not enforcing,
+	 * certain errors are non-fatal.
+	 */
+	switch (err) {
+	case -ENODATA:
+		reason = "unsigned module";
+		break;
+	case -ENOPKG:
+		reason = "module with unsupported crypto";
+		break;
+	case -ENOKEY:
+		reason = "module with unavailable key";
+		break;
+
+	default:
+		/*
+		 * All other errors are fatal, including lack of memory,
+		 * unparseable signatures, and signature check failures --
+		 * even if signatures aren't required.
+		 */
+		return err;
+	}
+
+	if (is_module_sig_enforced()) {
+		pr_notice("Loading of %s is rejected\n", reason);
+		return -EKEYREJECTED;
+	}
+
+	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+}
-- 
2.34.1

[PATCH v5 08/13] module: Move kmemleak support to a separate file

[Aaron Tomlin]

No functional change.

This patch migrates kmemleak code out of core module
code into kernel/module/debug_kmemleak.c

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 kernel/module/Makefile         |  1 +
 kernel/module/debug_kmemleak.c | 30 ++++++++++++++++++++++++++++++
 kernel/module/internal.h       |  7 +++++++
 kernel/module/main.c           | 27 ---------------------------
 4 files changed, 38 insertions(+), 27 deletions(-)
 create mode 100644 kernel/module/debug_kmemleak.c

diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index 8f2857d9ba1e..62c9fc91d411 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -11,4 +11,5 @@ ifdef CONFIG_MODULES
 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
 endif
diff --git a/kernel/module/debug_kmemleak.c b/kernel/module/debug_kmemleak.c
new file mode 100644
index 000000000000..e896c2268011
--- /dev/null
+++ b/kernel/module/debug_kmemleak.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module kmemleak support
+ *
+ * Copyright (C) 2009 Catalin Marinas
+ */
+
+#include <linux/module.h>
+#include <linux/kmemleak.h>
+#include "internal.h"
+
+void kmemleak_load_module(const struct module *mod,
+				 const struct load_info *info)
+{
+	unsigned int i;
+
+	/* only scan the sections containing data */
+	kmemleak_scan_area(mod, sizeof(struct module), GFP_KERNEL);
+
+	for (i = 1; i < info->hdr->e_shnum; i++) {
+		/* Scan all writable sections that's not executable */
+		if (!(info->sechdrs[i].sh_flags & SHF_ALLOC) ||
+		    !(info->sechdrs[i].sh_flags & SHF_WRITE) ||
+		    (info->sechdrs[i].sh_flags & SHF_EXECINSTR))
+			continue;
+
+		kmemleak_scan_area((void *)info->sechdrs[i].sh_addr,
+				   info->sechdrs[i].sh_size, GFP_KERNEL);
+	}
+}
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 6d5891cc8421..33d7befd0602 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -171,3 +171,10 @@ static inline int module_sig_check(struct load_info *info, int flags)
 	return 0;
 }
 #endif /* !CONFIG_MODULE_SIG */
+
+#ifdef CONFIG_DEBUG_KMEMLEAK
+void kmemleak_load_module(const struct module *mod, const struct load_info *info);
+#else /* !CONFIG_DEBUG_KMEMLEAK */
+static inline void __maybe_unused kmemleak_load_module(const struct module *mod,
+						       const struct load_info *info) { }
+#endif /* CONFIG_DEBUG_KMEMLEAK */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 403f2aacb3f6..c9931479e2eb 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2515,33 +2515,6 @@ bool __weak module_exit_section(const char *name)
 	return strstarts(name, ".exit");
 }
 
-#ifdef CONFIG_DEBUG_KMEMLEAK
-static void kmemleak_load_module(const struct module *mod,
-				 const struct load_info *info)
-{
-	unsigned int i;
-
-	/* only scan the sections containing data */
-	kmemleak_scan_area(mod, sizeof(struct module), GFP_KERNEL);
-
-	for (i = 1; i < info->hdr->e_shnum; i++) {
-		/* Scan all writable sections that's not executable */
-		if (!(info->sechdrs[i].sh_flags & SHF_ALLOC) ||
-		    !(info->sechdrs[i].sh_flags & SHF_WRITE) ||
-		    (info->sechdrs[i].sh_flags & SHF_EXECINSTR))
-			continue;
-
-		kmemleak_scan_area((void *)info->sechdrs[i].sh_addr,
-				   info->sechdrs[i].sh_size, GFP_KERNEL);
-	}
-}
-#else
-static inline void kmemleak_load_module(const struct module *mod,
-					const struct load_info *info)
-{
-}
-#endif
-
 static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr)
 {
 #if defined(CONFIG_64BIT)
-- 
2.34.1

Re: [PATCH v5 08/13] module: Move kmemleak support to a separate file

[Christophe Leroy]

Le 09/02/2022 à 18:08, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates kmemleak code out of core module
> code into kernel/module/debug_kmemleak.c
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   kernel/module/Makefile         |  1 +
>   kernel/module/debug_kmemleak.c | 30 ++++++++++++++++++++++++++++++
>   kernel/module/internal.h       |  7 +++++++
>   kernel/module/main.c           | 27 ---------------------------
>   4 files changed, 38 insertions(+), 27 deletions(-)
>   create mode 100644 kernel/module/debug_kmemleak.c
> 
> diff --git a/kernel/module/Makefile b/kernel/module/Makefile
> index 8f2857d9ba1e..62c9fc91d411 100644
> --- a/kernel/module/Makefile
> +++ b/kernel/module/Makefile
> @@ -11,4 +11,5 @@ ifdef CONFIG_MODULES
>   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
>   endif
> diff --git a/kernel/module/debug_kmemleak.c b/kernel/module/debug_kmemleak.c
> new file mode 100644
> index 000000000000..e896c2268011
> --- /dev/null
> +++ b/kernel/module/debug_kmemleak.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Module kmemleak support
> + *
> + * Copyright (C) 2009 Catalin Marinas
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kmemleak.h>
> +#include "internal.h"
> +
> +void kmemleak_load_module(const struct module *mod,
> +				 const struct load_info *info)

CHECK: Alignment should match open parenthesis
#48: FILE: kernel/module/debug_kmemleak.c:13:
+void kmemleak_load_module(const struct module *mod,
+				 const struct load_info *info)



> +{
> +	unsigned int i;
> +
> +	/* only scan the sections containing data */
> +	kmemleak_scan_area(mod, sizeof(struct module), GFP_KERNEL);
> +
> +	for (i = 1; i < info->hdr->e_shnum; i++) {
> +		/* Scan all writable sections that's not executable */
> +		if (!(info->sechdrs[i].sh_flags & SHF_ALLOC) ||
> +		    !(info->sechdrs[i].sh_flags & SHF_WRITE) ||
> +		    (info->sechdrs[i].sh_flags & SHF_EXECINSTR))
> +			continue;
> +
> +		kmemleak_scan_area((void *)info->sechdrs[i].sh_addr,
> +				   info->sechdrs[i].sh_size, GFP_KERNEL);
> +	}
> +}
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 6d5891cc8421..33d7befd0602 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -171,3 +171,10 @@ static inline int module_sig_check(struct load_info *info, int flags)
>   	return 0;
>   }
>   #endif /* !CONFIG_MODULE_SIG */
> +
> +#ifdef CONFIG_DEBUG_KMEMLEAK
> +void kmemleak_load_module(const struct module *mod, const struct load_info *info);
> +#else /* !CONFIG_DEBUG_KMEMLEAK */
> +static inline void __maybe_unused kmemleak_load_module(const struct module *mod,
> +						       const struct load_info *info) { }

Remove __maybe_unused, not needed for a 'static inline' in the .h


> +#endif /* CONFIG_DEBUG_KMEMLEAK */
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 403f2aacb3f6..c9931479e2eb 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2515,33 +2515,6 @@ bool __weak module_exit_section(const char *name)
>   	return strstarts(name, ".exit");
>   }
>   
> -#ifdef CONFIG_DEBUG_KMEMLEAK
> -static void kmemleak_load_module(const struct module *mod,
> -				 const struct load_info *info)
> -{
> -	unsigned int i;
> -
> -	/* only scan the sections containing data */
> -	kmemleak_scan_area(mod, sizeof(struct module), GFP_KERNEL);
> -
> -	for (i = 1; i < info->hdr->e_shnum; i++) {
> -		/* Scan all writable sections that's not executable */
> -		if (!(info->sechdrs[i].sh_flags & SHF_ALLOC) ||
> -		    !(info->sechdrs[i].sh_flags & SHF_WRITE) ||
> -		    (info->sechdrs[i].sh_flags & SHF_EXECINSTR))
> -			continue;
> -
> -		kmemleak_scan_area((void *)info->sechdrs[i].sh_addr,
> -				   info->sechdrs[i].sh_size, GFP_KERNEL);
> -	}
> -}
> -#else
> -static inline void kmemleak_load_module(const struct module *mod,
> -					const struct load_info *info)
> -{
> -}
> -#endif
> -
>   static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr)
>   {
>   #if defined(CONFIG_64BIT)

Re: [PATCH v5 08/13] module: Move kmemleak support to a separate file

[Aaron Tomlin]

On Thu 2022-02-10 13:07 +0000, Christophe Leroy wrote:
> CHECK: Alignment should match open parenthesis
> #48: FILE: kernel/module/debug_kmemleak.c:13:
> +void kmemleak_load_module(const struct module *mod,
> +				 const struct load_info *info)

Ok.

> > +static inline void __maybe_unused kmemleak_load_module(const struct module *mod,
> > +						       const struct load_info *info) { }
> 
> Remove __maybe_unused, not needed for a 'static inline' in the .h

Agreed.

Thanks for your feedback Christophe.


Kind regards,

-- 
Aaron Tomlin

[PATCH v5 09/13] module: Move kallsyms support into a separate file

[Aaron Tomlin]

No functional change.

This patch migrates kallsyms code out of core module
code kernel/module/kallsyms.c

Signed-off-by: Aaron Tomlin <atomlin@redhat.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

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);
+int cmp_name(const void *name, const void *sym);
+long get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr,
+		       unsigned int section);
 
 #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);
+const char *find_kallsyms_symbol(struct module *mod, unsigned long addr,
+					unsigned long *size, unsigned long *offset);
+#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)
+{
+	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 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module kallsyms support
+ *
+ * Copyright (C) 2010 Rusty Russell
+ */
+
+#include <linux/module.h>
+#include <linux/kallsyms.h>
+#include <linux/buildid.h>
+#include <linux/bsearch.h>
+#include "internal.h"
+
+/* 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);
+}
+
+static int is_exported(const char *name, unsigned long value,
+		       const struct module *mod)
+{
+	const struct kernel_symbol *ks;
+
+	if (!mod)
+		ks = lookup_exported_symbol(name, __start___ksymtab, __stop___ksymtab);
+	else
+		ks = lookup_exported_symbol(name, mod->syms, mod->syms + mod->num_syms);
+
+	return ks != NULL && kernel_symbol_value(ks) == value;
+}
+
+/* As per nm */
+static char elf_type(const Elf_Sym *sym, const struct load_info *info)
+{
+	const Elf_Shdr *sechdrs = info->sechdrs;
+
+	if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
+		if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT)
+			return 'v';
+		else
+			return 'w';
+	}
+	if (sym->st_shndx == SHN_UNDEF)
+		return 'U';
+	if (sym->st_shndx == SHN_ABS || sym->st_shndx == info->index.pcpu)
+		return 'a';
+	if (sym->st_shndx >= SHN_LORESERVE)
+		return '?';
+	if (sechdrs[sym->st_shndx].sh_flags & SHF_EXECINSTR)
+		return 't';
+	if (sechdrs[sym->st_shndx].sh_flags & SHF_ALLOC
+	    && sechdrs[sym->st_shndx].sh_type != SHT_NOBITS) {
+		if (!(sechdrs[sym->st_shndx].sh_flags & SHF_WRITE))
+			return 'r';
+		else if (sechdrs[sym->st_shndx].sh_flags & ARCH_SHF_SMALL)
+			return 'g';
+		else
+			return 'd';
+	}
+	if (sechdrs[sym->st_shndx].sh_type == SHT_NOBITS) {
+		if (sechdrs[sym->st_shndx].sh_flags & ARCH_SHF_SMALL)
+			return 's';
+		else
+			return 'b';
+	}
+	if (strstarts(info->secstrings + sechdrs[sym->st_shndx].sh_name,
+		      ".debug")) {
+		return 'n';
+	}
+	return '?';
+}
+
+static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
+			unsigned int shnum, unsigned int pcpundx)
+{
+	const Elf_Shdr *sec;
+
+	if (src->st_shndx == SHN_UNDEF
+	    || src->st_shndx >= shnum
+	    || !src->st_name)
+		return false;
+
+#ifdef CONFIG_KALLSYMS_ALL
+	if (src->st_shndx == pcpundx)
+		return true;
+#endif
+
+	sec = sechdrs + src->st_shndx;
+	if (!(sec->sh_flags & SHF_ALLOC)
+#ifndef CONFIG_KALLSYMS_ALL
+	    || !(sec->sh_flags & SHF_EXECINSTR)
+#endif
+	    || (sec->sh_entsize & INIT_OFFSET_MASK))
+		return false;
+
+	return true;
+}
+
+/*
+ * We only allocate and copy the strings needed by the parts of symtab
+ * we keep.  This is simple, but has the effect of making multiple
+ * copies of duplicates.  We could be more sophisticated, see
+ * linux-kernel thread starting with
+ * <73defb5e4bca04a6431392cc341112b1@localhost>.
+ */
+void layout_symtab(struct module *mod, struct load_info *info)
+{
+	Elf_Shdr *symsect = info->sechdrs + info->index.sym;
+	Elf_Shdr *strsect = info->sechdrs + info->index.str;
+	const Elf_Sym *src;
+	unsigned int i, nsrc, ndst, strtab_size = 0;
+
+	/* Put symbol section at end of init part of module. */
+	symsect->sh_flags |= SHF_ALLOC;
+	symsect->sh_entsize = get_offset(mod, &mod->init_layout.size, symsect,
+					 info->index.sym) | INIT_OFFSET_MASK;
+	pr_debug("\t%s\n", info->secstrings + symsect->sh_name);
+
+	src = (void *)info->hdr + symsect->sh_offset;
+	nsrc = symsect->sh_size / sizeof(*src);
+
+	/* Compute total space required for the core symbols' strtab. */
+	for (ndst = i = 0; i < nsrc; i++) {
+		if (i == 0 || is_livepatch_module(mod) ||
+		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
+				   info->index.pcpu)) {
+			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
+			ndst++;
+		}
+	}
+
+	/* Append room for core symbols at end of core part. */
+	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_typeoffs = 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. */
+	strsect->sh_flags |= SHF_ALLOC;
+	strsect->sh_entsize = get_offset(mod, &mod->init_layout.size, strsect,
+					 info->index.str) | INIT_OFFSET_MASK;
+	pr_debug("\t%s\n", info->secstrings + strsect->sh_name);
+
+	/* We'll tack temporary mod_kallsyms on the end. */
+	mod->init_layout.size = ALIGN(mod->init_layout.size,
+				      __alignof__(struct mod_kallsyms));
+	info->mod_kallsyms_init_off = mod->init_layout.size;
+	mod->init_layout.size += sizeof(struct mod_kallsyms);
+	info->init_typeoffs = mod->init_layout.size;
+	mod->init_layout.size += nsrc * sizeof(char);
+	mod->init_layout.size = debug_align(mod->init_layout.size);
+}
+
+/*
+ * We use the full symtab and strtab which layout_symtab arranged to
+ * be appended to the init section.  Later we switch to the cut-down
+ * core-only ones.
+ */
+void add_kallsyms(struct module *mod, const struct load_info *info)
+{
+	unsigned int i, ndst;
+	const Elf_Sym *src;
+	Elf_Sym *dst;
+	char *s;
+	Elf_Shdr *symsec = &info->sechdrs[info->index.sym];
+
+	/* Set up to point into init section. */
+	mod->kallsyms = mod->init_layout.base + info->mod_kallsyms_init_off;
+
+	mod->kallsyms->symtab = (void *)symsec->sh_addr;
+	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_typeoffs;
+
+	/*
+	 * 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_typeoffs;
+	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 += strscpy(s, &mod->kallsyms->strtab[src[i].st_name],
+				     KSYM_NAME_LEN) + 1;
+		}
+	}
+	mod->core_kallsyms.num_symtab = ndst;
+}
+
+inline bool sect_empty(const Elf_Shdr *sect)
+{
+	return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
+}
+
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+void init_build_id(struct module *mod, const struct load_info *info)
+{
+	const Elf_Shdr *sechdr;
+	unsigned int i;
+
+	for (i = 0; i < info->hdr->e_shnum; i++) {
+		sechdr = &info->sechdrs[i];
+		if (!sect_empty(sechdr) && sechdr->sh_type == SHT_NOTE &&
+		    !build_id_parse_buf((void *)sechdr->sh_addr, mod->build_id,
+					sechdr->sh_size))
+			break;
+	}
+}
+#endif
+
+/*
+ * This ignores the intensely annoying "mapping symbols" found
+ * in ARM ELF files: $a, $t and $d.
+ */
+static inline int is_arm_mapping_symbol(const char *str)
+{
+	if (str[0] == '.' && str[1] == 'L')
+		return true;
+	return str[0] == '$' && strchr("axtd", str[1])
+	       && (str[2] == '\0' || str[2] == '.');
+}
+
+static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned int symnum)
+{
+	return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
+}
+
+/*
+ * Given a module and address, find the corresponding symbol and return its name
+ * while providing its size and offset if needed.
+ */
+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, bestval;
+	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
+
+	/* At worse, next value is at end of module */
+	if (within_module_init(addr, mod))
+		nextval = (unsigned long)mod->init_layout.base+mod->init_layout.text_size;
+	else
+		nextval = (unsigned long)mod->core_layout.base+mod->core_layout.text_size;
+
+	bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);
+
+	/*
+	 * Scan for closest preceding symbol, and next symbol. (ELF
+	 * starts real symbols at 1).
+	 */
+	for (i = 1; i < kallsyms->num_symtab; i++) {
+		const Elf_Sym *sym = &kallsyms->symtab[i];
+		unsigned long thisval = kallsyms_symbol_value(sym);
+
+		if (sym->st_shndx == SHN_UNDEF)
+			continue;
+
+		/*
+		 * We ignore unnamed symbols: they're uninformative
+		 * and inserted at a whim.
+		 */
+		if (*kallsyms_symbol_name(kallsyms, i) == '\0'
+		    || is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
+			continue;
+
+		if (thisval <= addr && thisval > bestval) {
+			best = i;
+			bestval = thisval;
+		}
+		if (thisval > addr && thisval < nextval)
+			nextval = thisval;
+	}
+
+	if (!best)
+		return NULL;
+
+	if (size)
+		*size = nextval - bestval;
+	if (offset)
+		*offset = addr - bestval;
+
+	return kallsyms_symbol_name(kallsyms, best);
+}
+
+void * __weak dereference_module_function_descriptor(struct module *mod,
+						     void *ptr)
+{
+	return ptr;
+}
+
+/*
+ * For kallsyms to ask for address resolution.  NULL means not found.  Careful
+ * not to lock to avoid deadlock on oopses, simply disable preemption.
+ */
+const char *module_address_lookup(unsigned long addr,
+			    unsigned long *size,
+			    unsigned long *offset,
+			    char **modname,
+			    const unsigned char **modbuildid,
+			    char *namebuf)
+{
+	const char *ret = NULL;
+	struct module *mod;
+
+	preempt_disable();
+	mod = __module_address(addr);
+	if (mod) {
+		if (modname)
+			*modname = mod->name;
+		if (modbuildid) {
+#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
+			*modbuildid = mod->build_id;
+#else
+			*modbuildid = NULL;
+#endif
+		}
+
+		ret = find_kallsyms_symbol(mod, addr, size, offset);
+	}
+	/* Make a copy in here where it's safe */
+	if (ret) {
+		strncpy(namebuf, ret, KSYM_NAME_LEN - 1);
+		ret = namebuf;
+	}
+	preempt_enable();
+
+	return ret;
+}
+
+int lookup_module_symbol_name(unsigned long addr, char *symname)
+{
+	struct module *mod;
+
+	preempt_disable();
+	list_for_each_entry_rcu(mod, &modules, list) {
+		if (mod->state == MODULE_STATE_UNFORMED)
+			continue;
+		if (within_module(addr, mod)) {
+			const char *sym;
+
+			sym = find_kallsyms_symbol(mod, addr, NULL, NULL);
+			if (!sym)
+				goto out;
+
+			strscpy(symname, sym, KSYM_NAME_LEN);
+			preempt_enable();
+			return 0;
+		}
+	}
+out:
+	preempt_enable();
+	return -ERANGE;
+}
+
+int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
+			unsigned long *offset, char *modname, char *name)
+{
+	struct module *mod;
+
+	preempt_disable();
+	list_for_each_entry_rcu(mod, &modules, list) {
+		if (mod->state == MODULE_STATE_UNFORMED)
+			continue;
+		if (within_module(addr, mod)) {
+			const char *sym;
+
+			sym = find_kallsyms_symbol(mod, addr, size, offset);
+			if (!sym)
+				goto out;
+			if (modname)
+				strscpy(modname, mod->name, MODULE_NAME_LEN);
+			if (name)
+				strscpy(name, sym, KSYM_NAME_LEN);
+			preempt_enable();
+			return 0;
+		}
+	}
+out:
+	preempt_enable();
+	return -ERANGE;
+}
+
+int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
+			char *name, char *module_name, int *exported)
+{
+	struct module *mod;
+
+	preempt_disable();
+	list_for_each_entry_rcu(mod, &modules, list) {
+		struct mod_kallsyms *kallsyms;
+
+		if (mod->state == MODULE_STATE_UNFORMED)
+			continue;
+		kallsyms = rcu_dereference_sched(mod->kallsyms);
+		if (symnum < kallsyms->num_symtab) {
+			const Elf_Sym *sym = &kallsyms->symtab[symnum];
+
+			*value = kallsyms_symbol_value(sym);
+			*type = kallsyms->typetab[symnum];
+			strscpy(name, kallsyms_symbol_name(kallsyms, symnum), KSYM_NAME_LEN);
+			strscpy(module_name, mod->name, MODULE_NAME_LEN);
+			*exported = is_exported(name, *value, mod);
+			preempt_enable();
+			return 0;
+		}
+		symnum -= kallsyms->num_symtab;
+	}
+	preempt_enable();
+	return -ERANGE;
+}
+
+/* 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++) {
+		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;
+}
+
+/* Look for this name: can be of form module:name. */
+unsigned long module_kallsyms_lookup_name(const char *name)
+{
+	struct module *mod;
+	char *colon;
+	unsigned long ret = 0;
+
+	/* Don't lock: we're in enough trouble already. */
+	preempt_disable();
+	if ((colon = strnchr(name, MODULE_NAME_LEN, ':')) != NULL) {
+		if ((mod = find_module_all(name, colon - name, false)) != NULL)
+			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 = find_kallsyms_symbol_value(mod, name)) != 0)
+				break;
+		}
+	}
+	preempt_enable();
+	return ret;
+}
+
+#ifdef CONFIG_LIVEPATCH
+int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+					     struct module *, unsigned long),
+				   void *data)
+{
+	struct module *mod;
+	unsigned int i;
+	int ret = 0;
+
+	mutex_lock(&module_mutex);
+	list_for_each_entry(mod, &modules, list) {
+		/* We hold module_mutex: no need for rcu_dereference_sched */
+		struct mod_kallsyms *kallsyms = mod->kallsyms;
+
+		if (mod->state == MODULE_STATE_UNFORMED)
+			continue;
+		for (i = 0; i < kallsyms->num_symtab; i++) {
+			const Elf_Sym *sym = &kallsyms->symtab[i];
+
+			if (sym->st_shndx == SHN_UNDEF)
+				continue;
+
+			ret = fn(data, kallsyms_symbol_name(kallsyms, i),
+				 mod, kallsyms_symbol_value(sym));
+			if (ret != 0)
+				goto out;
+		}
+	}
+out:
+	mutex_unlock(&module_mutex);
+	return ret;
+}
+#endif /* CONFIG_LIVEPATCH */
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)
 {
 #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)
 {
 	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)
 	}
 }
 
-#ifdef CONFIG_KALLSYMS
-
-/* 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);
-}
-
-static int is_exported(const char *name, unsigned long value,
-		       const struct module *mod)
-{
-	const struct kernel_symbol *ks;
-	if (!mod)
-		ks = lookup_exported_symbol(name, __start___ksymtab, __stop___ksymtab);
-	else
-		ks = lookup_exported_symbol(name, mod->syms, mod->syms + mod->num_syms);
-
-	return ks != NULL && kernel_symbol_value(ks) == value;
-}
-
-/* As per nm */
-static char elf_type(const Elf_Sym *sym, const struct load_info *info)
-{
-	const Elf_Shdr *sechdrs = info->sechdrs;
-
-	if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
-		if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT)
-			return 'v';
-		else
-			return 'w';
-	}
-	if (sym->st_shndx == SHN_UNDEF)
-		return 'U';
-	if (sym->st_shndx == SHN_ABS || sym->st_shndx == info->index.pcpu)
-		return 'a';
-	if (sym->st_shndx >= SHN_LORESERVE)
-		return '?';
-	if (sechdrs[sym->st_shndx].sh_flags & SHF_EXECINSTR)
-		return 't';
-	if (sechdrs[sym->st_shndx].sh_flags & SHF_ALLOC
-	    && sechdrs[sym->st_shndx].sh_type != SHT_NOBITS) {
-		if (!(sechdrs[sym->st_shndx].sh_flags & SHF_WRITE))
-			return 'r';
-		else if (sechdrs[sym->st_shndx].sh_flags & ARCH_SHF_SMALL)
-			return 'g';
-		else
-			return 'd';
-	}
-	if (sechdrs[sym->st_shndx].sh_type == SHT_NOBITS) {
-		if (sechdrs[sym->st_shndx].sh_flags & ARCH_SHF_SMALL)
-			return 's';
-		else
-			return 'b';
-	}
-	if (strstarts(info->secstrings + sechdrs[sym->st_shndx].sh_name,
-		      ".debug")) {
-		return 'n';
-	}
-	return '?';
-}
-
-static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
-			unsigned int shnum, unsigned int pcpundx)
-{
-	const Elf_Shdr *sec;
-
-	if (src->st_shndx == SHN_UNDEF
-	    || src->st_shndx >= shnum
-	    || !src->st_name)
-		return false;
-
-#ifdef CONFIG_KALLSYMS_ALL
-	if (src->st_shndx == pcpundx)
-		return true;
-#endif
-
-	sec = sechdrs + src->st_shndx;
-	if (!(sec->sh_flags & SHF_ALLOC)
-#ifndef CONFIG_KALLSYMS_ALL
-	    || !(sec->sh_flags & SHF_EXECINSTR)
-#endif
-	    || (sec->sh_entsize & INIT_OFFSET_MASK))
-		return false;
-
-	return true;
-}
-
-/*
- * We only allocate and copy the strings needed by the parts of symtab
- * we keep.  This is simple, but has the effect of making multiple
- * copies of duplicates.  We could be more sophisticated, see
- * linux-kernel thread starting with
- * <73defb5e4bca04a6431392cc341112b1@localhost>.
- */
-static void layout_symtab(struct module *mod, struct load_info *info)
-{
-	Elf_Shdr *symsect = info->sechdrs + info->index.sym;
-	Elf_Shdr *strsect = info->sechdrs + info->index.str;
-	const Elf_Sym *src;
-	unsigned int i, nsrc, ndst, strtab_size = 0;
-
-	/* Put symbol section at end of init part of module. */
-	symsect->sh_flags |= SHF_ALLOC;
-	symsect->sh_entsize = get_offset(mod, &mod->init_layout.size, symsect,
-					 info->index.sym) | INIT_OFFSET_MASK;
-	pr_debug("\t%s\n", info->secstrings + symsect->sh_name);
-
-	src = (void *)info->hdr + symsect->sh_offset;
-	nsrc = symsect->sh_size / sizeof(*src);
-
-	/* Compute total space required for the core symbols' strtab. */
-	for (ndst = i = 0; i < nsrc; i++) {
-		if (i == 0 || is_livepatch_module(mod) ||
-		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
-				   info->index.pcpu)) {
-			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
-			ndst++;
-		}
-	}
-
-	/* Append room for core symbols at end of core part. */
-	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_typeoffs = 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. */
-	strsect->sh_flags |= SHF_ALLOC;
-	strsect->sh_entsize = get_offset(mod, &mod->init_layout.size, strsect,
-					 info->index.str) | INIT_OFFSET_MASK;
-	pr_debug("\t%s\n", info->secstrings + strsect->sh_name);
-
-	/* We'll tack temporary mod_kallsyms on the end. */
-	mod->init_layout.size = ALIGN(mod->init_layout.size,
-				      __alignof__(struct mod_kallsyms));
-	info->mod_kallsyms_init_off = mod->init_layout.size;
-	mod->init_layout.size += sizeof(struct mod_kallsyms);
-	info->init_typeoffs = mod->init_layout.size;
-	mod->init_layout.size += nsrc * sizeof(char);
-	mod->init_layout.size = debug_align(mod->init_layout.size);
-}
-
-/*
- * We use the full symtab and strtab which layout_symtab arranged to
- * be appended to the init section.  Later we switch to the cut-down
- * core-only ones.
- */
-static void add_kallsyms(struct module *mod, const struct load_info *info)
-{
-	unsigned int i, ndst;
-	const Elf_Sym *src;
-	Elf_Sym *dst;
-	char *s;
-	Elf_Shdr *symsec = &info->sechdrs[info->index.sym];
-
-	/* Set up to point into init section. */
-	mod->kallsyms = mod->init_layout.base + info->mod_kallsyms_init_off;
-
-	mod->kallsyms->symtab = (void *)symsec->sh_addr;
-	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_typeoffs;
-
-	/*
-	 * 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_typeoffs;
-	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],
-				     KSYM_NAME_LEN) + 1;
-		}
-	}
-	mod->core_kallsyms.num_symtab = ndst;
-}
-#else
-static inline void layout_symtab(struct module *mod, struct load_info *info)
-{
-}
-
-static void add_kallsyms(struct module *mod, const struct load_info *info)
-{
-}
-#endif /* CONFIG_KALLSYMS */
-
-#if IS_ENABLED(CONFIG_KALLSYMS) && IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
-static void init_build_id(struct module *mod, const struct load_info *info)
-{
-	const Elf_Shdr *sechdr;
-	unsigned int i;
-
-	for (i = 0; i < info->hdr->e_shnum; i++) {
-		sechdr = &info->sechdrs[i];
-		if (!sect_empty(sechdr) && sechdr->sh_type == SHT_NOTE &&
-		    !build_id_parse_buf((void *)sechdr->sh_addr, mod->build_id,
-					sechdr->sh_size))
-			break;
-	}
-}
-#else
-static void init_build_id(struct module *mod, const struct load_info *info)
-{
-}
-#endif
-
 static void dynamic_debug_setup(struct module *mod, struct _ddebug *debug, unsigned int num)
 {
 	if (!debug)
@@ -3795,287 +3566,6 @@ static inline int within(unsigned long addr, void *start, unsigned long size)
 	return ((void *)addr >= start && (void *)addr < start + size);
 }
 
-#ifdef CONFIG_KALLSYMS
-/*
- * This ignores the intensely annoying "mapping symbols" found
- * in ARM ELF files: $a, $t and $d.
- */
-static inline int is_arm_mapping_symbol(const char *str)
-{
-	if (str[0] == '.' && str[1] == 'L')
-		return true;
-	return str[0] == '$' && strchr("axtd", str[1])
-	       && (str[2] == '\0' || str[2] == '.');
-}
-
-static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned int symnum)
-{
-	return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
-}
-
-/*
- * 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, bestval;
-	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
-
-	/* At worse, next value is at end of module */
-	if (within_module_init(addr, mod))
-		nextval = (unsigned long)mod->init_layout.base+mod->init_layout.text_size;
-	else
-		nextval = (unsigned long)mod->core_layout.base+mod->core_layout.text_size;
-
-	bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);
-
-	/*
-	 * Scan for closest preceding symbol, and next symbol. (ELF
-	 * starts real symbols at 1).
-	 */
-	for (i = 1; i < kallsyms->num_symtab; i++) {
-		const Elf_Sym *sym = &kallsyms->symtab[i];
-		unsigned long thisval = kallsyms_symbol_value(sym);
-
-		if (sym->st_shndx == SHN_UNDEF)
-			continue;
-
-		/*
-		 * We ignore unnamed symbols: they're uninformative
-		 * and inserted at a whim.
-		 */
-		if (*kallsyms_symbol_name(kallsyms, i) == '\0'
-		    || is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
-			continue;
-
-		if (thisval <= addr && thisval > bestval) {
-			best = i;
-			bestval = thisval;
-		}
-		if (thisval > addr && thisval < nextval)
-			nextval = thisval;
-	}
-
-	if (!best)
-		return NULL;
-
-	if (size)
-		*size = nextval - bestval;
-	if (offset)
-		*offset = addr - bestval;
-
-	return kallsyms_symbol_name(kallsyms, best);
-}
-
-void * __weak dereference_module_function_descriptor(struct module *mod,
-						     void *ptr)
-{
-	return ptr;
-}
-
-/*
- * For kallsyms to ask for address resolution.  NULL means not found.  Careful
- * not to lock to avoid deadlock on oopses, simply disable preemption.
- */
-const char *module_address_lookup(unsigned long addr,
-			    unsigned long *size,
-			    unsigned long *offset,
-			    char **modname,
-			    const unsigned char **modbuildid,
-			    char *namebuf)
-{
-	const char *ret = NULL;
-	struct module *mod;
-
-	preempt_disable();
-	mod = __module_address(addr);
-	if (mod) {
-		if (modname)
-			*modname = mod->name;
-		if (modbuildid) {
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
-			*modbuildid = mod->build_id;
-#else
-			*modbuildid = NULL;
-#endif
-		}
-
-		ret = find_kallsyms_symbol(mod, addr, size, offset);
-	}
-	/* Make a copy in here where it's safe */
-	if (ret) {
-		strncpy(namebuf, ret, KSYM_NAME_LEN - 1);
-		ret = namebuf;
-	}
-	preempt_enable();
-
-	return ret;
-}
-
-int lookup_module_symbol_name(unsigned long addr, char *symname)
-{
-	struct module *mod;
-
-	preempt_disable();
-	list_for_each_entry_rcu(mod, &modules, list) {
-		if (mod->state == MODULE_STATE_UNFORMED)
-			continue;
-		if (within_module(addr, mod)) {
-			const char *sym;
-
-			sym = find_kallsyms_symbol(mod, addr, NULL, NULL);
-			if (!sym)
-				goto out;
-
-			strlcpy(symname, sym, KSYM_NAME_LEN);
-			preempt_enable();
-			return 0;
-		}
-	}
-out:
-	preempt_enable();
-	return -ERANGE;
-}
-
-int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
-			unsigned long *offset, char *modname, char *name)
-{
-	struct module *mod;
-
-	preempt_disable();
-	list_for_each_entry_rcu(mod, &modules, list) {
-		if (mod->state == MODULE_STATE_UNFORMED)
-			continue;
-		if (within_module(addr, mod)) {
-			const char *sym;
-
-			sym = find_kallsyms_symbol(mod, addr, size, offset);
-			if (!sym)
-				goto out;
-			if (modname)
-				strlcpy(modname, mod->name, MODULE_NAME_LEN);
-			if (name)
-				strlcpy(name, sym, KSYM_NAME_LEN);
-			preempt_enable();
-			return 0;
-		}
-	}
-out:
-	preempt_enable();
-	return -ERANGE;
-}
-
-int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
-			char *name, char *module_name, int *exported)
-{
-	struct module *mod;
-
-	preempt_disable();
-	list_for_each_entry_rcu(mod, &modules, list) {
-		struct mod_kallsyms *kallsyms;
-
-		if (mod->state == MODULE_STATE_UNFORMED)
-			continue;
-		kallsyms = rcu_dereference_sched(mod->kallsyms);
-		if (symnum < kallsyms->num_symtab) {
-			const Elf_Sym *sym = &kallsyms->symtab[symnum];
-
-			*value = kallsyms_symbol_value(sym);
-			*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);
-			preempt_enable();
-			return 0;
-		}
-		symnum -= kallsyms->num_symtab;
-	}
-	preempt_enable();
-	return -ERANGE;
-}
-
-/* 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++) {
-		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;
-}
-
-/* Look for this name: can be of form module:name. */
-unsigned long module_kallsyms_lookup_name(const char *name)
-{
-	struct module *mod;
-	char *colon;
-	unsigned long ret = 0;
-
-	/* Don't lock: we're in enough trouble already. */
-	preempt_disable();
-	if ((colon = strnchr(name, MODULE_NAME_LEN, ':')) != NULL) {
-		if ((mod = find_module_all(name, colon - name, false)) != NULL)
-			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 = find_kallsyms_symbol_value(mod, name)) != 0)
-				break;
-		}
-	}
-	preempt_enable();
-	return ret;
-}
-
-#ifdef CONFIG_LIVEPATCH
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
-					     struct module *, unsigned long),
-				   void *data)
-{
-	struct module *mod;
-	unsigned int i;
-	int ret = 0;
-
-	mutex_lock(&module_mutex);
-	list_for_each_entry(mod, &modules, list) {
-		/* We hold module_mutex: no need for rcu_dereference_sched */
-		struct mod_kallsyms *kallsyms = mod->kallsyms;
-
-		if (mod->state == MODULE_STATE_UNFORMED)
-			continue;
-		for (i = 0; i < kallsyms->num_symtab; i++) {
-			const Elf_Sym *sym = &kallsyms->symtab[i];
-
-			if (sym->st_shndx == SHN_UNDEF)
-				continue;
-
-			ret = fn(data, kallsyms_symbol_name(kallsyms, i),
-				 mod, kallsyms_symbol_value(sym));
-			if (ret != 0)
-				goto out;
-
-			cond_resched();
-		}
-	}
-out:
-	mutex_unlock(&module_mutex);
-	return ret;
-}
-#endif /* CONFIG_LIVEPATCH */
-#endif /* CONFIG_KALLSYMS */
-
 static void cfi_init(struct module *mod)
 {
 #ifdef CONFIG_CFI_CLANG
-- 
2.34.1

Re: [PATCH v5 09/13] module: Move kallsyms support into a separate file

[Christophe Leroy]

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@redhat.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@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)
>   	}
>   }
>   
...

Re: [PATCH v5 09/13] module: Move kallsyms support into a separate file

[Aaron Tomlin]

On Thu 2022-02-10 13:43 +0000, Christophe Leroy wrote:
> Checkpatch reports:
>
> total: 3 errors, 1 warnings, 26 checks, 1103 lines checked

Christophe,

> 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 *


Thanks once again for your review and feedback!

Indeed I can see the same via 'make C=2 kernel/module/'. Looking at struct
'module' declaration we see that field namely "kallsyms" has the __rcu
marker. So, If I understand correctly, perhaps this can be resolved as
follows, to be more explicit:

@@ -171,7 +171,7 @@ void add_kallsyms(struct module *mod, const struct
load_info *info)
        Elf_Shdr *symsec = &info->sechdrs[info->index.sym];

        /* Set up to point into init section. */
-       mod->kallsyms = mod->init_layout.base + info->mod_kallsyms_init_off;
+       mod->kallsyms = (struct mod_kallsyms __rcu
*)mod->init_layout.base + info->mod_kallsyms_init_off;


> 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

I will use rcu_dereference*() for the above since the pointer should not be
accessed directly.

> >
> > 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

Fair enough.

> > +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()

The rename is a good idea.

> > +bool sect_empty(const Elf_Shdr *sect);
>
> sect_empty() is small enough to remain a static inline.

Yes and moved to kernel/module/internal.h.

>
> > +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

Agreed.

>
> > +#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

Agreed.

>
> > +{
> > +    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

Agreed. This was an unfortunate oversight.

> > -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

Agreed.

> > +int cmp_name(const void *name, const void *sym)
>
> This function is small enought to become a 'static inline' in internal.h

Agreed.


Kind regards,

-- 
Aaron Tomlin

[PATCH v5 10/13] module: Move procfs support into a separate file

[Aaron Tomlin]

No functional change.

This patch migrates code that allows one to generate a
list of loaded/or linked modules via /proc when procfs
support is enabled into kernel/module/procfs.c.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 kernel/module/Makefile   |   1 +
 kernel/module/internal.h |   1 +
 kernel/module/main.c     | 131 +-----------------------------------
 kernel/module/procfs.c   | 142 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+), 130 deletions(-)
 create mode 100644 kernel/module/procfs.c

diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index 868b13c06920..c6be08060252 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -13,4 +13,5 @@ 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
+obj-$(CONFIG_PROC_FS) += procfs.o
 endif
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 7973666452c3..b67ce836746a 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -74,6 +74,7 @@ unsigned long kernel_symbol_value(const struct kernel_symbol *sym);
 int cmp_name(const void *name, const void *sym);
 long get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr,
 		       unsigned int section);
+char *module_flags(struct module *mod, char *buf);
 
 #ifdef CONFIG_LIVEPATCH
 int copy_module_elf(struct module *mod, struct load_info *info);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 378dd7fd1b6a..ff39c556bdf8 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -22,7 +22,6 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/elf.h>
-#include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/syscalls.h>
 #include <linux/fcntl.h>
@@ -811,31 +810,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	return ret;
 }
 
-static inline void print_unload_info(struct seq_file *m, struct module *mod)
-{
-	struct module_use *use;
-	int printed_something = 0;
-
-	seq_printf(m, " %i ", module_refcount(mod));
-
-	/*
-	 * Always include a trailing , so userspace can differentiate
-	 * between this and the old multi-field proc format.
-	 */
-	list_for_each_entry(use, &mod->source_list, source_list) {
-		printed_something = 1;
-		seq_printf(m, "%s,", use->source->name);
-	}
-
-	if (mod->init != NULL && mod->exit == NULL) {
-		printed_something = 1;
-		seq_puts(m, "[permanent],");
-	}
-
-	if (!printed_something)
-		seq_puts(m, "-");
-}
-
 void __symbol_put(const char *symbol)
 {
 	struct find_symbol_arg fsa = {
@@ -925,12 +899,6 @@ void module_put(struct module *module)
 EXPORT_SYMBOL(module_put);
 
 #else /* !CONFIG_MODULE_UNLOAD */
-static inline void print_unload_info(struct seq_file *m, struct module *mod)
-{
-	/* We don't know the usage count, or what modules are using. */
-	seq_puts(m, " - -");
-}
-
 static inline void module_unload_free(struct module *mod)
 {
 }
@@ -3601,7 +3569,7 @@ static void cfi_cleanup(struct module *mod)
 }
 
 /* Keep in sync with MODULE_FLAGS_BUF_SIZE !!! */
-static char *module_flags(struct module *mod, char *buf)
+char *module_flags(struct module *mod, char *buf)
 {
 	int bx = 0;
 
@@ -3624,103 +3592,6 @@ static char *module_flags(struct module *mod, char *buf)
 	return buf;
 }
 
-#ifdef CONFIG_PROC_FS
-/* Called by the /proc file system to return a list of modules. */
-static void *m_start(struct seq_file *m, loff_t *pos)
-{
-	mutex_lock(&module_mutex);
-	return seq_list_start(&modules, *pos);
-}
-
-static void *m_next(struct seq_file *m, void *p, loff_t *pos)
-{
-	return seq_list_next(p, &modules, pos);
-}
-
-static void m_stop(struct seq_file *m, void *p)
-{
-	mutex_unlock(&module_mutex);
-}
-
-static int m_show(struct seq_file *m, void *p)
-{
-	struct module *mod = list_entry(p, struct module, list);
-	char buf[MODULE_FLAGS_BUF_SIZE];
-	void *value;
-
-	/* We always ignore unformed modules. */
-	if (mod->state == MODULE_STATE_UNFORMED)
-		return 0;
-
-	seq_printf(m, "%s %u",
-		   mod->name, mod->init_layout.size + mod->core_layout.size);
-	print_unload_info(m, mod);
-
-	/* Informative for users. */
-	seq_printf(m, " %s",
-		   mod->state == MODULE_STATE_GOING ? "Unloading" :
-		   mod->state == MODULE_STATE_COMING ? "Loading" :
-		   "Live");
-	/* Used by oprofile and other similar tools. */
-	value = m->private ? NULL : mod->core_layout.base;
-	seq_printf(m, " 0x%px", value);
-
-	/* Taints info */
-	if (mod->taints)
-		seq_printf(m, " %s", module_flags(mod, buf));
-
-	seq_puts(m, "\n");
-	return 0;
-}
-
-/*
- * Format: modulename size refcount deps address
- *
- * Where refcount is a number or -, and deps is a comma-separated list
- * of depends or -.
- */
-static const struct seq_operations modules_op = {
-	.start	= m_start,
-	.next	= m_next,
-	.stop	= m_stop,
-	.show	= m_show
-};
-
-/*
- * This also sets the "private" pointer to non-NULL if the
- * kernel pointers should be hidden (so you can just test
- * "m->private" to see if you should keep the values private).
- *
- * We use the same logic as for /proc/kallsyms.
- */
-static int modules_open(struct inode *inode, struct file *file)
-{
-	int err = seq_open(file, &modules_op);
-
-	if (!err) {
-		struct seq_file *m = file->private_data;
-		m->private = kallsyms_show_value(file->f_cred) ? NULL : (void *)8ul;
-	}
-
-	return err;
-}
-
-static const struct proc_ops modules_proc_ops = {
-	.proc_flags	= PROC_ENTRY_PERMANENT,
-	.proc_open	= modules_open,
-	.proc_read	= seq_read,
-	.proc_lseek	= seq_lseek,
-	.proc_release	= seq_release,
-};
-
-static int __init proc_modules_init(void)
-{
-	proc_create("modules", 0, NULL, &modules_proc_ops);
-	return 0;
-}
-module_init(proc_modules_init);
-#endif
-
 /* Given an address, look for it in the module exception tables. */
 const struct exception_table_entry *search_module_extables(unsigned long addr)
 {
diff --git a/kernel/module/procfs.c b/kernel/module/procfs.c
new file mode 100644
index 000000000000..d706a798b52e
--- /dev/null
+++ b/kernel/module/procfs.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module proc support
+ *
+ * Copyright (C) 2008 Alexey Dobriyan
+ */
+
+#include <linux/module.h>
+#include <linux/kallsyms.h>
+#include <linux/mutex.h>
+#include <linux/seq_file.h>
+#include <linux/proc_fs.h>
+#include "internal.h"
+
+#ifdef CONFIG_MODULE_UNLOAD
+static inline void print_unload_info(struct seq_file *m, struct module *mod)
+{
+	struct module_use *use;
+	int printed_something = 0;
+
+	seq_printf(m, " %i ", module_refcount(mod));
+
+	/*
+	 * Always include a trailing , so userspace can differentiate
+	 * between this and the old multi-field proc format.
+	 */
+	list_for_each_entry(use, &mod->source_list, source_list) {
+		printed_something = 1;
+		seq_printf(m, "%s,", use->source->name);
+	}
+
+	if (mod->init != NULL && mod->exit == NULL) {
+		printed_something = 1;
+		seq_puts(m, "[permanent],");
+	}
+
+	if (!printed_something)
+		seq_puts(m, "-");
+}
+#else /* !CONFIG_MODULE_UNLOAD */
+static inline void print_unload_info(struct seq_file *m, struct module *mod)
+{
+	/* We don't know the usage count, or what modules are using. */
+	seq_puts(m, " - -");
+}
+#endif /* CONFIG_MODULE_UNLOAD */
+
+/* Called by the /proc file system to return a list of modules. */
+static void *m_start(struct seq_file *m, loff_t *pos)
+{
+	mutex_lock(&module_mutex);
+	return seq_list_start(&modules, *pos);
+}
+
+static void *m_next(struct seq_file *m, void *p, loff_t *pos)
+{
+	return seq_list_next(p, &modules, pos);
+}
+
+static void m_stop(struct seq_file *m, void *p)
+{
+	mutex_unlock(&module_mutex);
+}
+
+static int m_show(struct seq_file *m, void *p)
+{
+	struct module *mod = list_entry(p, struct module, list);
+	char buf[MODULE_FLAGS_BUF_SIZE];
+	void *value;
+
+	/* We always ignore unformed modules. */
+	if (mod->state == MODULE_STATE_UNFORMED)
+		return 0;
+
+	seq_printf(m, "%s %u",
+		   mod->name, mod->init_layout.size + mod->core_layout.size);
+	print_unload_info(m, mod);
+
+	/* Informative for users. */
+	seq_printf(m, " %s",
+		   mod->state == MODULE_STATE_GOING ? "Unloading" :
+		   mod->state == MODULE_STATE_COMING ? "Loading" :
+		   "Live");
+	/* Used by oprofile and other similar tools. */
+	value = m->private ? NULL : mod->core_layout.base;
+	seq_printf(m, " 0x%px", value);
+
+	/* Taints info */
+	if (mod->taints)
+		seq_printf(m, " %s", module_flags(mod, buf));
+
+	seq_puts(m, "\n");
+	return 0;
+}
+
+/*
+ * Format: modulename size refcount deps address
+ *
+ * Where refcount is a number or -, and deps is a comma-separated list
+ * of depends or -.
+ */
+static const struct seq_operations modules_op = {
+	.start	= m_start,
+	.next	= m_next,
+	.stop	= m_stop,
+	.show	= m_show
+};
+
+/*
+ * This also sets the "private" pointer to non-NULL if the
+ * kernel pointers should be hidden (so you can just test
+ * "m->private" to see if you should keep the values private).
+ *
+ * We use the same logic as for /proc/kallsyms.
+ */
+static int modules_open(struct inode *inode, struct file *file)
+{
+	int err = seq_open(file, &modules_op);
+
+	if (!err) {
+		struct seq_file *m = file->private_data;
+
+		m->private = kallsyms_show_value(file->f_cred) ? NULL : (void *)8ul;
+	}
+
+	return err;
+}
+
+static const struct proc_ops modules_proc_ops = {
+	.proc_flags	= PROC_ENTRY_PERMANENT,
+	.proc_open	= modules_open,
+	.proc_read	= seq_read,
+	.proc_lseek	= seq_lseek,
+	.proc_release	= seq_release,
+};
+
+static int __init proc_modules_init(void)
+{
+	proc_create("modules", 0, NULL, &modules_proc_ops);
+	return 0;
+}
+module_init(proc_modules_init);
-- 
2.34.1

Re: [PATCH v5 10/13] module: Move procfs support into a separate file

[Christophe Leroy]

Le 09/02/2022 à 18:08, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates code that allows one to generate a
> list of loaded/or linked modules via /proc when procfs
> support is enabled into kernel/module/procfs.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   kernel/module/Makefile   |   1 +
>   kernel/module/internal.h |   1 +
>   kernel/module/main.c     | 131 +-----------------------------------
>   kernel/module/procfs.c   | 142 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 145 insertions(+), 130 deletions(-)
>   create mode 100644 kernel/module/procfs.c

Checkpatch:

	total: 0 errors, 2 warnings, 2 checks, 315 lines checked

Re: [PATCH v5 07/13] module: Move extra signature support out of core code

[Michal Suchánek]

Hello,

On Wed, Feb 09, 2022 at 05:08:08PM +0000, Aaron Tomlin wrote:
> No functional change.

There is functional change.


> @@ -2565,70 +2542,6 @@ static inline void kmemleak_load_module(const struct module *mod,
>  }
>  #endif
>  
> -#ifdef CONFIG_MODULE_SIG
> -static int module_sig_check(struct load_info *info, int flags)
> -{
> -	int err = -ENODATA;
> -	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> -	const char *reason;
> -	const void *mod = info->hdr;
> -	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> -				       MODULE_INIT_IGNORE_VERMAGIC);
> -	/*
> -	 * Do not allow mangled modules as a module with version information
> -	 * removed is no longer the module that was signed.
> -	 */
> -	if (!mangled_module &&
             ^^^^^^^^^^^^^
> -	    info->len > markerlen &&
> -	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> -		/* We truncate the module to discard the signature */
> -		info->len -= markerlen;
> -		err = mod_verify_sig(mod, info);
> -		if (!err) {
> -			info->sig_ok = true;
> -			return 0;
> -		}
> -	}

> diff --git a/kernel/module/signing.c b/kernel/module/signing.c
> index 8aeb6d2ee94b..ff41541e982a 100644
> --- a/kernel/module/signing.c
> +++ b/kernel/module/signing.c

> @@ -43,3 +62,59 @@ int mod_verify_sig(const void *mod, struct load_info *info)
>  				      VERIFYING_MODULE_SIGNATURE,
>  				      NULL, NULL);
>  }
> +
> +int module_sig_check(struct load_info *info, int flags)
> +{
> +	int err = -ENODATA;
> +	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> +	const char *reason;
> +	const void *mod = info->hdr;
> +
> +	/*
> +	 * Require flags == 0, as a module with version information
> +	 * removed is no longer the module that was signed
> +	 */
> +	if (flags == 0 &&
            ^^^^^^

This reverts a97ac8cb24a3c3ad74794adb83717ef1605d1b47

Please re-apply.

Thanks

Michal
> +	    info->len > markerlen &&
> +	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> +		/* We truncate the module to discard the signature */
> +		info->len -= markerlen;
> +		err = mod_verify_sig(mod, info);
> +		if (!err) {
> +			info->sig_ok = true;
> +			return 0;
> +		}
> +	}

Re: [PATCH v5 07/13] module: Move extra signature support out of core code

[Aaron Tomlin]

On Wed 2022-02-09 21:48 +0100, Michal Suchánek wrote:
> This reverts a97ac8cb24a3c3ad74794adb83717ef1605d1b47

Hi Michal,

Oops! I'll address this.


Thanks,

-- 
Aaron Tomlin

Re: [PATCH v5 07/13] module: Move extra signature support out of core code

[Christophe Leroy]

Why do patches 7 to 13 have a Reply-to: 
20220209170358.3266629-1-atomlin@redhat.com and not patches 1 to 6 ?

Le 09/02/2022 à 18:08, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates additional module signature check
> code from core module code into kernel/module/signing.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   include/linux/module.h   |  1 +
>   kernel/module/internal.h |  9 +++++
>   kernel/module/main.c     | 87 ----------------------------------------
>   kernel/module/signing.c  | 75 ++++++++++++++++++++++++++++++++++
>   4 files changed, 85 insertions(+), 87 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index fd6161d78127..aea0ffd94a41 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -863,6 +863,7 @@ static inline bool module_sig_ok(struct module *module)
>   {
>   	return true;
>   }
> +#define sig_enforce false

Having that is module.h  it may redefine some existing symbol, like in 
security/integrity/ima/ima_main.c

sig_enforce is used only in signing.c so it should be defined there 
exclusively. This #define shouldn't be needed at all.



And checkpatch is not happy:

CHECK: Please use a blank line after function/struct/union/enum declarations
#27: FILE: include/linux/module.h:866:
  }
+#define sig_enforce false


>   #endif	/* CONFIG_MODULE_SIG */
>   
>   int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

Re: [PATCH v5 07/13] module: Move extra signature support out of core code

[Aaron Tomlin]

On Thu 2022-02-10 13:01 +0000, Christophe Leroy wrote:
> Why do patches 7 to 13 have a Reply-to:
> 20220209170358.3266629-1-atomlin@redhat.com and not patches 1 to 6 ?

Christophe,

Please disregard this mishap. Unfortunately, at the time I hit the relay
quota.

> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index fd6161d78127..aea0ffd94a41 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -863,6 +863,7 @@ static inline bool module_sig_ok(struct module *module)
> >   {
> >       return true;
> >   }
> > +#define sig_enforce false
> sig_enforce is used only in signing.c so it should be defined there
> exclusively.

Agreed.

> And checkpatch is not happy:
>
> CHECK: Please use a blank line after function/struct/union/enum declarations
> #27: FILE: include/linux/module.h:866:
>   }
> +#define sig_enforce false

Ok.


Kind regards,

-- 
Aaron Tomlin