linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/13] module: core code clean up
@ 2022-02-18 21:24 Aaron Tomlin
  2022-02-18 21:24 ` [PATCH v6 01/13] module: Move all into module/ Aaron Tomlin
                   ` (10 more replies)
  0 siblings, 11 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:24 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

Hi Luis,

As per your suggestion [1], this is an attempt to refactor and split
optional code out of core module support code into separate components.
This version is based on Linus' commit 7993e65fdd0f ("Merge tag
'mtd/fixes-for-5.17-rc5' of
git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux").
Please let me know your thoughts.

Changes since v5 [2]:

 - Updated MAINTAINERS to include the entire kernel/module/ directory
   (Christophe Leroy)
 - Reintroduce commit a97ac8cb24a3 ("module: fix signature check failures
   when using in-kernel decompression") (Michal Suchánek)
 - Refactored code to address some (i.e.
   --ignore=MULTIPLE_ASSIGNMENTS,ASSIGN_IN_IF was used) style violations
   e.g. "Alignment should match open parenthesis", reported by
   scripts/checkpatch.pl --strict (Christophe Leroy)
 - Used PAGE_ALIGN() and PAGE_ALIGNED() instead (Christophe Leroy)
 - Removed sig_enforce from include/linux/module.h as it is only
   used in kernel/module/signing.c (Christophe Leroy)
 - Added static keyword for anything not used outside a source file
   (Christophe Leroy)
 - Moved mod_sysfs_teardown() to kernel/module/sysfs.c (Christophe Leroy)
 - Removed kdb_modules from kernel/debug/kdb/kdb_private.h
   (Christophe Leroy)

Changes since v4 [3]:

 - Moved is_livepatch_module() and set_livepatch_module() to
   kernel/module/livepatch.c
 - Addressed minor compiler warning concerning
   kernel/module/internal.h (0-day)
 - Resolved style violations reported by scripts/checkpatch.pl
 - Dropped patch 5 [4] so external patch [5] can be applied at
   a later date post merge into module-next (Christophe Leroy)

Changes since v3 [6]:

 - Refactored both is_livepatch_module() and set_livepatch_module(),
   respectively, to use IS_ENABLED(CONFIG_LIVEPATCH) (Joe Perches)
 - Addressed various compiler warnings e.g., no previous prototype (0-day)

Changes since v2 [7]:

 - Moved module decompress support to a separate file
 - Made check_modinfo_livepatch() generic (Petr Mladek)
 - Removed filename from each newly created file (Luis Chamberlain)
 - Addressed some (i.e. --ignore=ASSIGN_IN_IF,AVOID_BUG was used)
   minor scripts/checkpatch.pl concerns e.g., use strscpy over
   strlcpy and missing a blank line after declarations (Allen)

Changes since v1 [8]:

  - Moved module version support code into a new file

[1]: https://lore.kernel.org/lkml/YbEZ4HgSYQEPuRmS@bombadil.infradead.org/
[2]: https://lore.kernel.org/lkml/20220209170358.3266629-1-atomlin@redhat.com/
[3]: https://lore.kernel.org/lkml/20220130213214.1042497-1-atomlin@redhat.com/
[4]: https://lore.kernel.org/lkml/20220130213214.1042497-6-atomlin@redhat.com/
[5]: https://lore.kernel.org/lkml/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.git.christophe.leroy@csgroup.eu/
[6]: https://lore.kernel.org/lkml/20220128203934.600247-1-atomlin@redhat.com/
[7]: https://lore.kernel.org/lkml/20220106234319.2067842-1-atomlin@redhat.com/
[8]: https://lore.kernel.org/lkml/20211228213041.1356334-1-atomlin@redhat.com/


Aaron Tomlin (13):
  module: Move all into module/
  module: Simple refactor in preparation for split
  module: Make internal.h and decompress.c more compliant
  module: Move livepatch support to a separate file
  module: Move latched RB-tree support to a separate file
  module: Move strict rwx support to a separate file
  module: Move extra signature support out of core code
  module: Move kmemleak support to a separate file
  module: Move kallsyms support into a separate file
  module: Move procfs support into a separate file
  module: Move sysfs support into a separate file
  module: Move kdb_modules list out of core code
  module: Move version support into a separate file

 MAINTAINERS                                   |    2 +-
 include/linux/module.h                        |    9 +-
 kernel/Makefile                               |    5 +-
 kernel/debug/kdb/kdb_main.c                   |    5 +
 kernel/debug/kdb/kdb_private.h                |    4 -
 kernel/module-internal.h                      |   50 -
 kernel/module/Makefile                        |   19 +
 kernel/module/debug_kmemleak.c                |   30 +
 .../decompress.c}                             |    5 +-
 kernel/module/internal.h                      |  284 +++
 kernel/module/kallsyms.c                      |  502 +++++
 kernel/module/livepatch.c                     |   74 +
 kernel/{module.c => module/main.c}            | 1856 +----------------
 kernel/module/procfs.c                        |  142 ++
 .../signature.c}                              |    0
 kernel/module/signing.c                       |  122 ++
 kernel/module/strict_rwx.c                    |   84 +
 kernel/module/sysfs.c                         |  436 ++++
 kernel/module/tree_lookup.c                   |  109 +
 kernel/module/version.c                       |  109 +
 kernel/module_signing.c                       |   45 -
 21 files changed, 2010 insertions(+), 1882 deletions(-)
 delete mode 100644 kernel/module-internal.h
 create mode 100644 kernel/module/Makefile
 create mode 100644 kernel/module/debug_kmemleak.c
 rename kernel/{module_decompress.c => module/decompress.c} (99%)
 create mode 100644 kernel/module/internal.h
 create mode 100644 kernel/module/kallsyms.c
 create mode 100644 kernel/module/livepatch.c
 rename kernel/{module.c => module/main.c} (64%)
 create mode 100644 kernel/module/procfs.c
 rename kernel/{module_signature.c => module/signature.c} (100%)
 create mode 100644 kernel/module/signing.c
 create mode 100644 kernel/module/strict_rwx.c
 create mode 100644 kernel/module/sysfs.c
 create mode 100644 kernel/module/tree_lookup.c
 create mode 100644 kernel/module/version.c
 delete mode 100644 kernel/module_signing.c


base-commit: 7993e65fdd0fe07beb9f36f998f9bbef2c0ee391
-- 
2.34.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v6 01/13] module: Move all into module/
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
@ 2022-02-18 21:24 ` Aaron Tomlin
  2022-02-21 12:21   ` Christophe Leroy
  2022-02-21 13:13   ` Christophe Leroy
  2022-02-18 21:25 ` [PATCH v6 02/13] module: Simple refactor in preparation for split Aaron Tomlin
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:24 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

No functional changes.

This patch moves all module related code into a separate directory,
modifies each file name and creates a new Makefile. Note: this effort
is in preparation to refactor core module code.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 MAINTAINERS                                         | 2 +-
 kernel/Makefile                                     | 5 +----
 kernel/module/Makefile                              | 9 +++++++++
 kernel/{module_decompress.c => module/decompress.c} | 2 +-
 kernel/{module-internal.h => module/internal.h}     | 0
 kernel/{module.c => module/main.c}                  | 2 +-
 kernel/{module_signature.c => module/signature.c}   | 0
 kernel/{module_signing.c => module/signing.c}       | 2 +-
 8 files changed, 14 insertions(+), 8 deletions(-)
 create mode 100644 kernel/module/Makefile
 rename kernel/{module_decompress.c => module/decompress.c} (99%)
 rename kernel/{module-internal.h => module/internal.h} (100%)
 rename kernel/{module.c => module/main.c} (99%)
 rename kernel/{module_signature.c => module/signature.c} (100%)
 rename kernel/{module_signing.c => module/signing.c} (97%)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd86ed9fbc79..463bdb829db4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13012,7 +13012,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next
 F:	include/linux/module.h
-F:	kernel/module.c
+F:	kernel/module/
 
 MONOLITHIC POWER SYSTEM PMIC DRIVER
 M:	Saravanan Sekar <sravanhome@gmail.com>
diff --git a/kernel/Makefile b/kernel/Makefile
index 56f4ee97f328..3a6380975c57 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -53,6 +53,7 @@ obj-y += rcu/
 obj-y += livepatch/
 obj-y += dma/
 obj-y += entry/
+obj-y += module/
 
 obj-$(CONFIG_KCMP) += kcmp.o
 obj-$(CONFIG_FREEZER) += freezer.o
@@ -66,10 +67,6 @@ ifneq ($(CONFIG_SMP),y)
 obj-y += up.o
 endif
 obj-$(CONFIG_UID16) += uid16.o
-obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_DECOMPRESS) += module_decompress.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o
-obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signature.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_CRASH_CORE) += crash_core.o
diff --git a/kernel/module/Makefile b/kernel/module/Makefile
new file mode 100644
index 000000000000..2902fc7d0ef1
--- /dev/null
+++ b/kernel/module/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for linux kernel module support
+#
+
+obj-$(CONFIG_MODULES) += main.o
+obj-$(CONFIG_MODULE_DECOMPRESS) += decompress.o
+obj-$(CONFIG_MODULE_SIG) += signing.o
+obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o
diff --git a/kernel/module_decompress.c b/kernel/module/decompress.c
similarity index 99%
rename from kernel/module_decompress.c
rename to kernel/module/decompress.c
index ffef98a20320..d14d6443225a 100644
--- a/kernel/module_decompress.c
+++ b/kernel/module/decompress.c
@@ -12,7 +12,7 @@
 #include <linux/sysfs.h>
 #include <linux/vmalloc.h>
 
-#include "module-internal.h"
+#include "internal.h"
 
 static int module_extend_max_pages(struct load_info *info, unsigned int extent)
 {
diff --git a/kernel/module-internal.h b/kernel/module/internal.h
similarity index 100%
rename from kernel/module-internal.h
rename to kernel/module/internal.h
diff --git a/kernel/module.c b/kernel/module/main.c
similarity index 99%
rename from kernel/module.c
rename to kernel/module/main.c
index 46a5c2ed1928..34a2b0cf3c3e 100644
--- a/kernel/module.c
+++ b/kernel/module/main.c
@@ -58,7 +58,7 @@
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
 #include <uapi/linux/module.h>
-#include "module-internal.h"
+#include "internal.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
diff --git a/kernel/module_signature.c b/kernel/module/signature.c
similarity index 100%
rename from kernel/module_signature.c
rename to kernel/module/signature.c
diff --git a/kernel/module_signing.c b/kernel/module/signing.c
similarity index 97%
rename from kernel/module_signing.c
rename to kernel/module/signing.c
index 8723ae70ea1f..8aeb6d2ee94b 100644
--- a/kernel/module_signing.c
+++ b/kernel/module/signing.c
@@ -12,7 +12,7 @@
 #include <linux/string.h>
 #include <linux/verification.h>
 #include <crypto/public_key.h>
-#include "module-internal.h"
+#include "internal.h"
 
 /*
  * Verify the signature on a module.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 02/13] module: Simple refactor in preparation for split
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
  2022-02-18 21:24 ` [PATCH v6 01/13] module: Move all into module/ Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-18 21:25 ` [PATCH v6 03/13] module: Make internal.h and decompress.c more compliant Aaron Tomlin
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

No functional change.

This patch makes it possible to move non-essential code
out of core module code.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 kernel/module/internal.h | 21 +++++++++++++++++++++
 kernel/module/main.c     | 22 ++--------------------
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 8c381c99062f..ea8c4c02614c 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -7,6 +7,27 @@
 
 #include <linux/elf.h>
 #include <asm/module.h>
+#include <linux/mutex.h>
+
+#ifndef ARCH_SHF_SMALL
+#define ARCH_SHF_SMALL 0
+#endif
+
+/* If this is set, the section belongs in the init part of the module */
+#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG - 1))
+/* Maximum number of characters written by module_flags() */
+#define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
+
+extern struct mutex module_mutex;
+extern struct list_head modules;
+
+/* Provided by the linker */
+extern const struct kernel_symbol __start___ksymtab[];
+extern const struct kernel_symbol __stop___ksymtab[];
+extern const struct kernel_symbol __start___ksymtab_gpl[];
+extern const struct kernel_symbol __stop___ksymtab_gpl[];
+extern const s32 __start___kcrctab[];
+extern const s32 __start___kcrctab_gpl[];
 
 struct load_info {
 	const char *name;
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 34a2b0cf3c3e..5f5e21f972dd 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -63,10 +63,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
 
-#ifndef ARCH_SHF_SMALL
-#define ARCH_SHF_SMALL 0
-#endif
-
 /*
  * Modules' sections will be aligned on page boundaries
  * to ensure complete separation of code and data, but
@@ -78,9 +74,6 @@
 # define debug_align(X) (X)
 #endif
 
-/* If this is set, the section belongs in the init part of the module */
-#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
-
 /*
  * Mutex protects:
  * 1) List of modules (also safely readable with preempt_disable),
@@ -88,8 +81,8 @@
  * 3) module_addr_min/module_addr_max.
  * (delete and add uses RCU list operations).
  */
-static DEFINE_MUTEX(module_mutex);
-static LIST_HEAD(modules);
+DEFINE_MUTEX(module_mutex);
+LIST_HEAD(modules);
 
 /* Work queue for freeing init sections in success case */
 static void do_free_init(struct work_struct *w);
@@ -408,14 +401,6 @@ static __maybe_unused void *any_section_objs(const struct load_info *info,
 	return (void *)info->sechdrs[sec].sh_addr;
 }
 
-/* Provided by the linker */
-extern const struct kernel_symbol __start___ksymtab[];
-extern const struct kernel_symbol __stop___ksymtab[];
-extern const struct kernel_symbol __start___ksymtab_gpl[];
-extern const struct kernel_symbol __stop___ksymtab_gpl[];
-extern const s32 __start___kcrctab[];
-extern const s32 __start___kcrctab_gpl[];
-
 #ifndef CONFIG_MODVERSIONS
 #define symversion(base, idx) NULL
 #else
@@ -4542,9 +4527,6 @@ static void cfi_cleanup(struct module *mod)
 #endif
 }
 
-/* Maximum number of characters written by module_flags() */
-#define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
-
 /* Keep in sync with MODULE_FLAGS_BUF_SIZE !!! */
 static char *module_flags(struct module *mod, char *buf)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 03/13] module: Make internal.h and decompress.c more compliant
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
  2022-02-18 21:24 ` [PATCH v6 01/13] module: Move all into module/ Aaron Tomlin
  2022-02-18 21:25 ` [PATCH v6 02/13] module: Simple refactor in preparation for split Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-18 21:25 ` [PATCH v6 04/13] module: Move livepatch support to a separate file Aaron Tomlin
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

This patch will address the following warning and style violations
generated by ./scripts/checkpatch.pl in strict mode:

  WARNING: Use #include <linux/module.h> instead of <asm/module.h>
  #10: FILE: kernel/module/internal.h:10:
  +#include <asm/module.h>

  CHECK: spaces preferred around that '-' (ctx:VxV)
  #18: FILE: kernel/module/internal.h:18:
  +#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))

  CHECK: Please use a blank line after function/struct/union/enum declarations
  #69: FILE: kernel/module/internal.h:69:
  +}
  +static inline void module_decompress_cleanup(struct load_info *info)
						   ^
  CHECK: extern prototypes should be avoided in .h files
  #84: FILE: kernel/module/internal.h:84:
  +extern int mod_verify_sig(const void *mod, struct load_info *info);

  WARNING: Missing a blank line after declarations
  #116: FILE: kernel/module/decompress.c:116:
  +               struct page *page = module_get_next_page(info);
  +               if (!page) {

  WARNING: Missing a blank line after declarations
  #174: FILE: kernel/module/decompress.c:174:
  +               struct page *page = module_get_next_page(info);
  +               if (!page) {

  CHECK: Please use a blank line after function/struct/union/enum declarations
  #258: FILE: kernel/module/decompress.c:258:
  +}
  +static struct kobj_attribute module_compression_attr = __ATTR_RO(compression);

Note: Fortunately, the multiple-include optimisation found in
include/linux/module.h will prevent duplication/or inclusion more than
once.

Fixes: f314dfea16a ("modsign: log module name in the event of an error")
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 kernel/module/decompress.c | 3 +++
 kernel/module/internal.h   | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
index d14d6443225a..2fc7081dd7c1 100644
--- a/kernel/module/decompress.c
+++ b/kernel/module/decompress.c
@@ -113,6 +113,7 @@ static ssize_t module_gzip_decompress(struct load_info *info,
 
 	do {
 		struct page *page = module_get_next_page(info);
+
 		if (!page) {
 			retval = -ENOMEM;
 			goto out_inflate_end;
@@ -171,6 +172,7 @@ static ssize_t module_xz_decompress(struct load_info *info,
 
 	do {
 		struct page *page = module_get_next_page(info);
+
 		if (!page) {
 			retval = -ENOMEM;
 			goto out;
@@ -256,6 +258,7 @@ static ssize_t compression_show(struct kobject *kobj,
 {
 	return sysfs_emit(buf, "%s\n", __stringify(MODULE_COMPRESSION));
 }
+
 static struct kobj_attribute module_compression_attr = __ATTR_RO(compression);
 
 static int __init module_decompress_sysfs_init(void)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index ea8c4c02614c..e0775e66bcf7 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -6,7 +6,8 @@
  */
 
 #include <linux/elf.h>
-#include <asm/module.h>
+#include <linux/compiler.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 
 #ifndef ARCH_SHF_SMALL
@@ -54,7 +55,7 @@ struct load_info {
 	} index;
 };
 
-extern int mod_verify_sig(const void *mod, struct load_info *info);
+int mod_verify_sig(const void *mod, struct load_info *info);
 
 #ifdef CONFIG_MODULE_DECOMPRESS
 int module_decompress(struct load_info *info, const void *buf, size_t size);
@@ -65,6 +66,7 @@ static inline int module_decompress(struct load_info *info,
 {
 	return -EOPNOTSUPP;
 }
+
 static inline void module_decompress_cleanup(struct load_info *info)
 {
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 04/13] module: Move livepatch support to a separate file
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (2 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 03/13] module: Make internal.h and decompress.c more compliant Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-18 21:25 ` [PATCH v6 05/13] module: Move latched RB-tree " Aaron Tomlin
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

No functional change.

This patch migrates livepatch support (i.e. used during module
add/or load and remove/or deletion) from core module code into
kernel/module/livepatch.c. At the moment it contains code to
persist Elf information about a given livepatch module, only.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 include/linux/module.h    |   9 ++--
 kernel/module/Makefile    |   1 +
 kernel/module/internal.h  |  22 ++++++++
 kernel/module/livepatch.c |  74 +++++++++++++++++++++++++++
 kernel/module/main.c      | 102 ++++----------------------------------
 5 files changed, 110 insertions(+), 98 deletions(-)
 create mode 100644 kernel/module/livepatch.c

diff --git a/include/linux/module.h b/include/linux/module.h
index 1e135fd5c076..7ec9715de7dc 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -663,17 +663,14 @@ static inline bool module_requested_async_probing(struct module *module)
 	return module && module->async_probe_requested;
 }
 
-#ifdef CONFIG_LIVEPATCH
 static inline bool is_livepatch_module(struct module *mod)
 {
+#ifdef CONFIG_LIVEPATCH
 	return mod->klp;
-}
-#else /* !CONFIG_LIVEPATCH */
-static inline bool is_livepatch_module(struct module *mod)
-{
+#else
 	return false;
+#endif
 }
-#endif /* CONFIG_LIVEPATCH */
 
 bool is_module_sig_enforced(void);
 void set_module_sig_enforced(void);
diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index 2902fc7d0ef1..ba3ebdb7055b 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_MODULES) += main.o
 obj-$(CONFIG_MODULE_DECOMPRESS) += decompress.o
 obj-$(CONFIG_MODULE_SIG) += signing.o
 obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index e0775e66bcf7..ad7a444253ed 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -57,6 +57,28 @@ struct load_info {
 
 int mod_verify_sig(const void *mod, struct load_info *info);
 
+#ifdef CONFIG_LIVEPATCH
+int copy_module_elf(struct module *mod, struct load_info *info);
+void free_module_elf(struct module *mod);
+#else /* !CONFIG_LIVEPATCH */
+static inline int copy_module_elf(struct module *mod, struct load_info *info)
+{
+	return 0;
+}
+
+static inline void free_module_elf(struct module *mod) { }
+#endif /* CONFIG_LIVEPATCH */
+
+static inline bool set_livepatch_module(struct module *mod)
+{
+#ifdef CONFIG_LIVEPATCH
+	mod->klp = true;
+	return true;
+#else
+	return false;
+#endif
+}
+
 #ifdef CONFIG_MODULE_DECOMPRESS
 int module_decompress(struct load_info *info, const void *buf, size_t size);
 void module_decompress_cleanup(struct load_info *info);
diff --git a/kernel/module/livepatch.c b/kernel/module/livepatch.c
new file mode 100644
index 000000000000..486d4ff92719
--- /dev/null
+++ b/kernel/module/livepatch.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module livepatch support
+ *
+ * Copyright (C) 2016 Jessica Yu <jeyu@redhat.com>
+ */
+
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include "internal.h"
+
+/*
+ * Persist Elf information about a module. Copy the Elf header,
+ * section header table, section string table, and symtab section
+ * index from info to mod->klp_info.
+ */
+int copy_module_elf(struct module *mod, struct load_info *info)
+{
+	unsigned int size, symndx;
+	int ret;
+
+	size = sizeof(*mod->klp_info);
+	mod->klp_info = kmalloc(size, GFP_KERNEL);
+	if (!mod->klp_info)
+		return -ENOMEM;
+
+	/* Elf header */
+	size = sizeof(mod->klp_info->hdr);
+	memcpy(&mod->klp_info->hdr, info->hdr, size);
+
+	/* Elf section header table */
+	size = sizeof(*info->sechdrs) * info->hdr->e_shnum;
+	mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL);
+	if (!mod->klp_info->sechdrs) {
+		ret = -ENOMEM;
+		goto free_info;
+	}
+
+	/* Elf section name string table */
+	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
+	mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL);
+	if (!mod->klp_info->secstrings) {
+		ret = -ENOMEM;
+		goto free_sechdrs;
+	}
+
+	/* Elf symbol section index */
+	symndx = info->index.sym;
+	mod->klp_info->symndx = symndx;
+
+	/*
+	 * For livepatch modules, core_kallsyms.symtab is a complete
+	 * copy of the original symbol table. Adjust sh_addr to point
+	 * to core_kallsyms.symtab since the copy of the symtab in module
+	 * init memory is freed at the end of do_init_module().
+	 */
+	mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long)mod->core_kallsyms.symtab;
+
+	return 0;
+
+free_sechdrs:
+	kfree(mod->klp_info->sechdrs);
+free_info:
+	kfree(mod->klp_info);
+	return ret;
+}
+
+void free_module_elf(struct module *mod)
+{
+	kfree(mod->klp_info->sechdrs);
+	kfree(mod->klp_info->secstrings);
+	kfree(mod->klp_info);
+}
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5f5e21f972dd..3596ebf3a6c3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2043,81 +2043,6 @@ static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 }
 #endif /*  CONFIG_STRICT_MODULE_RWX */
 
-#ifdef CONFIG_LIVEPATCH
-/*
- * Persist Elf information about a module. Copy the Elf header,
- * section header table, section string table, and symtab section
- * index from info to mod->klp_info.
- */
-static int copy_module_elf(struct module *mod, struct load_info *info)
-{
-	unsigned int size, symndx;
-	int ret;
-
-	size = sizeof(*mod->klp_info);
-	mod->klp_info = kmalloc(size, GFP_KERNEL);
-	if (mod->klp_info == NULL)
-		return -ENOMEM;
-
-	/* Elf header */
-	size = sizeof(mod->klp_info->hdr);
-	memcpy(&mod->klp_info->hdr, info->hdr, size);
-
-	/* Elf section header table */
-	size = sizeof(*info->sechdrs) * info->hdr->e_shnum;
-	mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL);
-	if (mod->klp_info->sechdrs == NULL) {
-		ret = -ENOMEM;
-		goto free_info;
-	}
-
-	/* Elf section name string table */
-	size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
-	mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL);
-	if (mod->klp_info->secstrings == NULL) {
-		ret = -ENOMEM;
-		goto free_sechdrs;
-	}
-
-	/* Elf symbol section index */
-	symndx = info->index.sym;
-	mod->klp_info->symndx = symndx;
-
-	/*
-	 * For livepatch modules, core_kallsyms.symtab is a complete
-	 * copy of the original symbol table. Adjust sh_addr to point
-	 * to core_kallsyms.symtab since the copy of the symtab in module
-	 * init memory is freed at the end of do_init_module().
-	 */
-	mod->klp_info->sechdrs[symndx].sh_addr = \
-		(unsigned long) mod->core_kallsyms.symtab;
-
-	return 0;
-
-free_sechdrs:
-	kfree(mod->klp_info->sechdrs);
-free_info:
-	kfree(mod->klp_info);
-	return ret;
-}
-
-static void free_module_elf(struct module *mod)
-{
-	kfree(mod->klp_info->sechdrs);
-	kfree(mod->klp_info->secstrings);
-	kfree(mod->klp_info);
-}
-#else /* !CONFIG_LIVEPATCH */
-static int copy_module_elf(struct module *mod, struct load_info *info)
-{
-	return 0;
-}
-
-static void free_module_elf(struct module *mod)
-{
-}
-#endif /* CONFIG_LIVEPATCH */
-
 void __weak module_memfree(void *module_region)
 {
 	/*
@@ -3092,30 +3017,23 @@ static int copy_chunked_from_user(void *dst, const void __user *usrc, unsigned l
 	return 0;
 }
 
-#ifdef CONFIG_LIVEPATCH
 static int check_modinfo_livepatch(struct module *mod, struct load_info *info)
 {
-	if (get_modinfo(info, "livepatch")) {
-		mod->klp = true;
+	if (!get_modinfo(info, "livepatch"))
+		/* Nothing more to do */
+		return 0;
+
+	if (set_livepatch_module(mod)) {
 		add_taint_module(mod, TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
 		pr_notice_once("%s: tainting kernel with TAINT_LIVEPATCH\n",
-			       mod->name);
-	}
-
-	return 0;
-}
-#else /* !CONFIG_LIVEPATCH */
-static int check_modinfo_livepatch(struct module *mod, struct load_info *info)
-{
-	if (get_modinfo(info, "livepatch")) {
-		pr_err("%s: module is marked as livepatch module, but livepatch support is disabled",
-		       mod->name);
-		return -ENOEXEC;
+				mod->name);
+		return 0;
 	}
 
-	return 0;
+	pr_err("%s: module is marked as livepatch module, but livepatch support is disabled",
+	       mod->name);
+	return -ENOEXEC;
 }
-#endif /* CONFIG_LIVEPATCH */
 
 static void check_modinfo_retpoline(struct module *mod, struct load_info *info)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 05/13] module: Move latched RB-tree support to a separate file
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (3 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 04/13] module: Move livepatch support to a separate file Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-21 10:57   ` Christophe Leroy
  2022-02-18 21:25 ` [PATCH v6 06/13] module: Move strict rwx " Aaron Tomlin
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

No functional change.

This patch migrates module latched RB-tree support
(e.g. see __module_address()) from core module code
into kernel/module/tree_lookup.c.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 kernel/module/Makefile      |   3 +
 kernel/module/internal.h    |  33 +++++++++
 kernel/module/main.c        | 130 ++----------------------------------
 kernel/module/tree_lookup.c | 109 ++++++++++++++++++++++++++++++
 4 files changed, 149 insertions(+), 126 deletions(-)
 create mode 100644 kernel/module/tree_lookup.c

diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index ba3ebdb7055b..6fb21ebe1aa3 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -8,3 +8,6 @@ obj-$(CONFIG_MODULE_DECOMPRESS) += decompress.o
 obj-$(CONFIG_MODULE_SIG) += signing.o
 obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
+ifdef CONFIG_MODULES
+obj-$(CONFIG_MODULES_TREE_LOOKUP) += tree_lookup.o
+endif
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index ad7a444253ed..57a715454c9e 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -9,6 +9,7 @@
 #include <linux/compiler.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/rculist.h>
 
 #ifndef ARCH_SHF_SMALL
 #define ARCH_SHF_SMALL 0
@@ -93,3 +94,35 @@ static inline void module_decompress_cleanup(struct load_info *info)
 {
 }
 #endif
+
+#ifdef CONFIG_MODULES_TREE_LOOKUP
+struct mod_tree_root {
+	struct latch_tree_root root;
+	unsigned long addr_min;
+	unsigned long addr_max;
+};
+
+extern struct mod_tree_root mod_tree;
+
+void mod_tree_insert(struct module *mod);
+void mod_tree_remove_init(struct module *mod);
+void mod_tree_remove(struct module *mod);
+struct module *mod_find(unsigned long addr);
+#else /* !CONFIG_MODULES_TREE_LOOKUP */
+
+static void mod_tree_insert(struct module *mod) { }
+static void mod_tree_remove_init(struct module *mod) { }
+static void mod_tree_remove(struct module *mod) { }
+static inline struct module *mod_find(unsigned long addr)
+{
+	struct module *mod;
+
+	list_for_each_entry_rcu(mod, &modules, list,
+				lockdep_is_held(&module_mutex)) {
+		if (within_module(addr, mod))
+			return mod;
+	}
+
+	return NULL;
+}
+#endif /* CONFIG_MODULES_TREE_LOOKUP */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 3596ebf3a6c3..76b53880ad91 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -90,138 +90,16 @@ static DECLARE_WORK(init_free_wq, do_free_init);
 static LLIST_HEAD(init_free_list);
 
 #ifdef CONFIG_MODULES_TREE_LOOKUP
-
-/*
- * Use a latched RB-tree for __module_address(); this allows us to use
- * RCU-sched lookups of the address from any context.
- *
- * This is conditional on PERF_EVENTS || TRACING because those can really hit
- * __module_address() hard by doing a lot of stack unwinding; potentially from
- * NMI context.
- */
-
-static __always_inline unsigned long __mod_tree_val(struct latch_tree_node *n)
-{
-	struct module_layout *layout = container_of(n, struct module_layout, mtn.node);
-
-	return (unsigned long)layout->base;
-}
-
-static __always_inline unsigned long __mod_tree_size(struct latch_tree_node *n)
-{
-	struct module_layout *layout = container_of(n, struct module_layout, mtn.node);
-
-	return (unsigned long)layout->size;
-}
-
-static __always_inline bool
-mod_tree_less(struct latch_tree_node *a, struct latch_tree_node *b)
-{
-	return __mod_tree_val(a) < __mod_tree_val(b);
-}
-
-static __always_inline int
-mod_tree_comp(void *key, struct latch_tree_node *n)
-{
-	unsigned long val = (unsigned long)key;
-	unsigned long start, end;
-
-	start = __mod_tree_val(n);
-	if (val < start)
-		return -1;
-
-	end = start + __mod_tree_size(n);
-	if (val >= end)
-		return 1;
-
-	return 0;
-}
-
-static const struct latch_tree_ops mod_tree_ops = {
-	.less = mod_tree_less,
-	.comp = mod_tree_comp,
-};
-
-static struct mod_tree_root {
-	struct latch_tree_root root;
-	unsigned long addr_min;
-	unsigned long addr_max;
-} mod_tree __cacheline_aligned = {
+struct mod_tree_root mod_tree __cacheline_aligned = {
 	.addr_min = -1UL,
 };
 
 #define module_addr_min mod_tree.addr_min
 #define module_addr_max mod_tree.addr_max
 
-static noinline void __mod_tree_insert(struct mod_tree_node *node)
-{
-	latch_tree_insert(&node->node, &mod_tree.root, &mod_tree_ops);
-}
-
-static void __mod_tree_remove(struct mod_tree_node *node)
-{
-	latch_tree_erase(&node->node, &mod_tree.root, &mod_tree_ops);
-}
-
-/*
- * These modifications: insert, remove_init and remove; are serialized by the
- * module_mutex.
- */
-static void mod_tree_insert(struct module *mod)
-{
-	mod->core_layout.mtn.mod = mod;
-	mod->init_layout.mtn.mod = mod;
-
-	__mod_tree_insert(&mod->core_layout.mtn);
-	if (mod->init_layout.size)
-		__mod_tree_insert(&mod->init_layout.mtn);
-}
-
-static void mod_tree_remove_init(struct module *mod)
-{
-	if (mod->init_layout.size)
-		__mod_tree_remove(&mod->init_layout.mtn);
-}
-
-static void mod_tree_remove(struct module *mod)
-{
-	__mod_tree_remove(&mod->core_layout.mtn);
-	mod_tree_remove_init(mod);
-}
-
-static struct module *mod_find(unsigned long addr)
-{
-	struct latch_tree_node *ltn;
-
-	ltn = latch_tree_find((void *)addr, &mod_tree.root, &mod_tree_ops);
-	if (!ltn)
-		return NULL;
-
-	return container_of(ltn, struct mod_tree_node, node)->mod;
-}
-
-#else /* MODULES_TREE_LOOKUP */
-
-static unsigned long module_addr_min = -1UL, module_addr_max = 0;
-
-static void mod_tree_insert(struct module *mod) { }
-static void mod_tree_remove_init(struct module *mod) { }
-static void mod_tree_remove(struct module *mod) { }
-
-static struct module *mod_find(unsigned long addr)
-{
-	struct module *mod;
-
-	list_for_each_entry_rcu(mod, &modules, list,
-				lockdep_is_held(&module_mutex)) {
-		if (within_module(addr, mod))
-			return mod;
-	}
-
-	return NULL;
-}
-
-#endif /* MODULES_TREE_LOOKUP */
+#else /* !CONFIG_MODULES_TREE_LOOKUP */
+static unsigned long module_addr_min = -1UL, module_addr_max;
+#endif /* CONFIG_MODULES_TREE_LOOKUP */
 
 /*
  * Bounds of module text, for speeding up __module_address.
diff --git a/kernel/module/tree_lookup.c b/kernel/module/tree_lookup.c
new file mode 100644
index 000000000000..0bc4ec3b22ce
--- /dev/null
+++ b/kernel/module/tree_lookup.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Modules tree lookup
+ *
+ * Copyright (C) 2015 Peter Zijlstra
+ * Copyright (C) 2015 Rusty Russell
+ */
+
+#include <linux/module.h>
+#include <linux/rbtree_latch.h>
+#include "internal.h"
+
+/*
+ * Use a latched RB-tree for __module_address(); this allows us to use
+ * RCU-sched lookups of the address from any context.
+ *
+ * This is conditional on PERF_EVENTS || TRACING because those can really hit
+ * __module_address() hard by doing a lot of stack unwinding; potentially from
+ * NMI context.
+ */
+
+static __always_inline unsigned long __mod_tree_val(struct latch_tree_node *n)
+{
+	struct module_layout *layout = container_of(n, struct module_layout, mtn.node);
+
+	return (unsigned long)layout->base;
+}
+
+static __always_inline unsigned long __mod_tree_size(struct latch_tree_node *n)
+{
+	struct module_layout *layout = container_of(n, struct module_layout, mtn.node);
+
+	return (unsigned long)layout->size;
+}
+
+static __always_inline bool
+mod_tree_less(struct latch_tree_node *a, struct latch_tree_node *b)
+{
+	return __mod_tree_val(a) < __mod_tree_val(b);
+}
+
+static __always_inline int
+mod_tree_comp(void *key, struct latch_tree_node *n)
+{
+	unsigned long val = (unsigned long)key;
+	unsigned long start, end;
+
+	start = __mod_tree_val(n);
+	if (val < start)
+		return -1;
+
+	end = start + __mod_tree_size(n);
+	if (val >= end)
+		return 1;
+
+	return 0;
+}
+
+static const struct latch_tree_ops mod_tree_ops = {
+	.less = mod_tree_less,
+	.comp = mod_tree_comp,
+};
+
+static noinline void __mod_tree_insert(struct mod_tree_node *node)
+{
+	latch_tree_insert(&node->node, &mod_tree.root, &mod_tree_ops);
+}
+
+static void __mod_tree_remove(struct mod_tree_node *node)
+{
+	latch_tree_erase(&node->node, &mod_tree.root, &mod_tree_ops);
+}
+
+/*
+ * These modifications: insert, remove_init and remove; are serialized by the
+ * module_mutex.
+ */
+void mod_tree_insert(struct module *mod)
+{
+	mod->core_layout.mtn.mod = mod;
+	mod->init_layout.mtn.mod = mod;
+
+	__mod_tree_insert(&mod->core_layout.mtn);
+	if (mod->init_layout.size)
+		__mod_tree_insert(&mod->init_layout.mtn);
+}
+
+void mod_tree_remove_init(struct module *mod)
+{
+	if (mod->init_layout.size)
+		__mod_tree_remove(&mod->init_layout.mtn);
+}
+
+void mod_tree_remove(struct module *mod)
+{
+	__mod_tree_remove(&mod->core_layout.mtn);
+	mod_tree_remove_init(mod);
+}
+
+struct module *mod_find(unsigned long addr)
+{
+	struct latch_tree_node *ltn;
+
+	ltn = latch_tree_find((void *)addr, &mod_tree.root, &mod_tree_ops);
+	if (!ltn)
+		return NULL;
+
+	return container_of(ltn, struct mod_tree_node, node)->mod;
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 06/13] module: Move strict rwx support to a separate file
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (4 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 05/13] module: Move latched RB-tree " Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-21  6:31   ` Christophe Leroy
                     ` (2 more replies)
  2022-02-18 21:25 ` [PATCH v6 07/13] module: Move extra signature support out of core code Aaron Tomlin
                   ` (4 subsequent siblings)
  10 siblings, 3 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

No functional change.

This patch migrates code that makes module text
and rodata memory read-only and non-text memory
non-executable from core module code into
kernel/module/strict_rwx.c.

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

diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index 6fb21ebe1aa3..3f48343636ff 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -10,4 +10,5 @@ obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
 ifdef CONFIG_MODULES
 obj-$(CONFIG_MODULES_TREE_LOOKUP) += tree_lookup.o
+obj-$(CONFIG_STRICT_MODULE_RWX) += strict_rwx.o
 endif
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 57a715454c9e..f4b7e123d625 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -20,6 +20,17 @@
 /* Maximum number of characters written by module_flags() */
 #define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
 
+/*
+ * Modules' sections will be aligned on page boundaries
+ * to ensure complete separation of code and data, but
+ * only when CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y
+ */
+#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
+# define debug_align(X) PAGE_ALIGN(X)
+#else
+# define debug_align(X) (X)
+#endif
+
 extern struct mutex module_mutex;
 extern struct list_head modules;
 
@@ -126,3 +137,30 @@ static inline struct module *mod_find(unsigned long addr)
 	return NULL;
 }
 #endif /* CONFIG_MODULES_TREE_LOOKUP */
+
+#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
+void frob_text(const struct module_layout *layout, int (*set_memory)(unsigned long start,
+								     int num_pages));
+#endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
+
+#ifdef CONFIG_STRICT_MODULE_RWX
+void frob_rodata(const struct module_layout *layout,
+		 int (*set_memory)(unsigned long start, int num_pages));
+void frob_ro_after_init(const struct module_layout *layout,
+			int (*set_memory)(unsigned long start, int num_pages));
+void frob_writable_data(const struct module_layout *layout,
+			int (*set_memory)(unsigned long start, int num_pages));
+void module_enable_ro(const struct module *mod, bool after_init);
+void module_enable_nx(const struct module *mod);
+int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
+				char *secstrings, struct module *mod);
+
+#else /* !CONFIG_STRICT_MODULE_RWX */
+static void module_enable_nx(const struct module *mod) { }
+static void module_enable_ro(const struct module *mod, bool after_init) {}
+static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
+				       char *secstrings, struct module *mod)
+{
+	return 0;
+}
+#endif /* CONFIG_STRICT_MODULE_RWX */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 76b53880ad91..5cd63f14b1ef 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -63,17 +63,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
 
-/*
- * Modules' sections will be aligned on page boundaries
- * to ensure complete separation of code and data, but
- * only when CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y
- */
-#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
-# define debug_align(X) ALIGN(X, PAGE_SIZE)
-#else
-# define debug_align(X) (X)
-#endif
-
 /*
  * Mutex protects:
  * 1) List of modules (also safely readable with preempt_disable),
@@ -1819,8 +1808,8 @@ static void mod_sysfs_teardown(struct module *mod)
  * whether we are strict.
  */
 #ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
-static void frob_text(const struct module_layout *layout,
-		      int (*set_memory)(unsigned long start, int num_pages))
+void frob_text(const struct module_layout *layout,
+	       int (*set_memory)(unsigned long start, int num_pages))
 {
 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
@@ -1837,90 +1826,6 @@ static void module_enable_x(const struct module *mod)
 static void module_enable_x(const struct module *mod) { }
 #endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
 
-#ifdef CONFIG_STRICT_MODULE_RWX
-static void frob_rodata(const struct module_layout *layout,
-			int (*set_memory)(unsigned long start, int num_pages))
-{
-	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
-	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
-	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base + layout->text_size,
-		   (layout->ro_size - layout->text_size) >> PAGE_SHIFT);
-}
-
-static void frob_ro_after_init(const struct module_layout *layout,
-				int (*set_memory)(unsigned long start, int num_pages))
-{
-	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
-	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
-	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base + layout->ro_size,
-		   (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT);
-}
-
-static void frob_writable_data(const struct module_layout *layout,
-			       int (*set_memory)(unsigned long start, int num_pages))
-{
-	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
-	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
-	BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base + layout->ro_after_init_size,
-		   (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT);
-}
-
-static void module_enable_ro(const struct module *mod, bool after_init)
-{
-	if (!rodata_enabled)
-		return;
-
-	set_vm_flush_reset_perms(mod->core_layout.base);
-	set_vm_flush_reset_perms(mod->init_layout.base);
-	frob_text(&mod->core_layout, set_memory_ro);
-
-	frob_rodata(&mod->core_layout, set_memory_ro);
-	frob_text(&mod->init_layout, set_memory_ro);
-	frob_rodata(&mod->init_layout, set_memory_ro);
-
-	if (after_init)
-		frob_ro_after_init(&mod->core_layout, set_memory_ro);
-}
-
-static void module_enable_nx(const struct module *mod)
-{
-	frob_rodata(&mod->core_layout, set_memory_nx);
-	frob_ro_after_init(&mod->core_layout, set_memory_nx);
-	frob_writable_data(&mod->core_layout, set_memory_nx);
-	frob_rodata(&mod->init_layout, set_memory_nx);
-	frob_writable_data(&mod->init_layout, set_memory_nx);
-}
-
-static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
-				       char *secstrings, struct module *mod)
-{
-	const unsigned long shf_wx = SHF_WRITE|SHF_EXECINSTR;
-	int i;
-
-	for (i = 0; i < hdr->e_shnum; i++) {
-		if ((sechdrs[i].sh_flags & shf_wx) == shf_wx) {
-			pr_err("%s: section %s (index %d) has invalid WRITE|EXEC flags\n",
-				mod->name, secstrings + sechdrs[i].sh_name, i);
-			return -ENOEXEC;
-		}
-	}
-
-	return 0;
-}
-
-#else /* !CONFIG_STRICT_MODULE_RWX */
-static void module_enable_nx(const struct module *mod) { }
-static void module_enable_ro(const struct module *mod, bool after_init) {}
-static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
-				       char *secstrings, struct module *mod)
-{
-	return 0;
-}
-#endif /*  CONFIG_STRICT_MODULE_RWX */
-
 void __weak module_memfree(void *module_region)
 {
 	/*
diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c
new file mode 100644
index 000000000000..c642889f8e77
--- /dev/null
+++ b/kernel/module/strict_rwx.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module strict rwx
+ *
+ * Copyright (C) 2015 Rusty Russell
+ */
+
+#include <linux/module.h>
+#include <linux/vmalloc.h>
+#include <linux/set_memory.h>
+#include "internal.h"
+
+void frob_rodata(const struct module_layout *layout,
+		 int (*set_memory)(unsigned long start, int num_pages))
+{
+	BUG_ON(!PAGE_ALIGNED(layout->base));
+	BUG_ON(!PAGE_ALIGNED(layout->text_size));
+	BUG_ON(!PAGE_ALIGNED(layout->ro_size));
+	set_memory((unsigned long)layout->base + layout->text_size,
+		   (layout->ro_size - layout->text_size) >> PAGE_SHIFT);
+}
+
+void frob_ro_after_init(const struct module_layout *layout,
+			int (*set_memory)(unsigned long start, int num_pages))
+{
+	BUG_ON(!PAGE_ALIGNED(layout->base));
+	BUG_ON(!PAGE_ALIGNED(layout->ro_size));
+	BUG_ON(!PAGE_ALIGNED(layout->ro_after_init_size));
+	set_memory((unsigned long)layout->base + layout->ro_size,
+		   (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT);
+}
+
+void frob_writable_data(const struct module_layout *layout,
+			int (*set_memory)(unsigned long start, int num_pages))
+{
+	BUG_ON(!PAGE_ALIGNED(layout->base));
+	BUG_ON(!PAGE_ALIGNED(layout->ro_after_init_size));
+	BUG_ON(!PAGE_ALIGNED(layout->size));
+	set_memory((unsigned long)layout->base + layout->ro_after_init_size,
+		   (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT);
+}
+
+void module_enable_ro(const struct module *mod, bool after_init)
+{
+	if (!rodata_enabled)
+		return;
+
+	set_vm_flush_reset_perms(mod->core_layout.base);
+	set_vm_flush_reset_perms(mod->init_layout.base);
+	frob_text(&mod->core_layout, set_memory_ro);
+
+	frob_rodata(&mod->core_layout, set_memory_ro);
+	frob_text(&mod->init_layout, set_memory_ro);
+	frob_rodata(&mod->init_layout, set_memory_ro);
+
+	if (after_init)
+		frob_ro_after_init(&mod->core_layout, set_memory_ro);
+}
+
+void module_enable_nx(const struct module *mod)
+{
+	frob_rodata(&mod->core_layout, set_memory_nx);
+	frob_ro_after_init(&mod->core_layout, set_memory_nx);
+	frob_writable_data(&mod->core_layout, set_memory_nx);
+	frob_rodata(&mod->init_layout, set_memory_nx);
+	frob_writable_data(&mod->init_layout, set_memory_nx);
+}
+
+int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
+				char *secstrings, struct module *mod)
+{
+	const unsigned long shf_wx = SHF_WRITE | SHF_EXECINSTR;
+	int i;
+
+	for (i = 0; i < hdr->e_shnum; i++) {
+		if ((sechdrs[i].sh_flags & shf_wx) == shf_wx) {
+			pr_err("%s: section %s (index %d) has invalid WRITE|EXEC flags\n",
+			       mod->name, secstrings + sechdrs[i].sh_name, i);
+			return -ENOEXEC;
+		}
+	}
+
+	return 0;
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 07/13] module: Move extra signature support out of core code
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (5 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 06/13] module: Move strict rwx " Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-18 21:25 ` [PATCH v6 08/13] module: Move kmemleak support to a separate file Aaron Tomlin
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

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>
---
 kernel/module/internal.h |  9 +++++
 kernel/module/main.c     | 87 ----------------------------------------
 kernel/module/signing.c  | 77 +++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 87 deletions(-)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index f4b7e123d625..0fdd9c0cd77d 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -164,3 +164,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 5cd63f14b1ef..c63e10c61694 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>
@@ -127,28 +126,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);
@@ -2569,70 +2546,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..85c8999dfecf 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -11,9 +11,29 @@
 #include <linux/module_signature.h>
 #include <linux/string.h>
 #include <linux/verification.h>
+#include <linux/security.h>
 #include <crypto/public_key.h>
+#include <uapi/linux/module.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 +63,60 @@ 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;
+	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);
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 08/13] module: Move kmemleak support to a separate file
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (6 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 07/13] module: Move extra signature support out of core code Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-18 21:25 ` [PATCH v6 09/13] module: Move kallsyms support into " Aaron Tomlin
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

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 3f48343636ff..e2a5ae0264f9 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -11,4 +11,5 @@ obj-$(CONFIG_LIVEPATCH) += livepatch.o
 ifdef CONFIG_MODULES
 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..12a569d361e8
--- /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 0fdd9c0cd77d..1c9604443e2f 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -173,3 +173,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 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 c63e10c61694..7dd283959c5c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2519,33 +2519,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


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (7 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 08/13] module: Move kmemleak support to a separate file Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-21  8:15   ` Christophe Leroy
  2022-02-21 10:49   ` Christophe Leroy
  2022-02-18 21:25 ` [PATCH v6 10/13] module: Move procfs " Aaron Tomlin
  2022-02-19  2:12 ` [PATCH v6 00/13] module: core code clean up Luis Chamberlain
  10 siblings, 2 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

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 |  32 +++
 kernel/module/kallsyms.c | 502 ++++++++++++++++++++++++++++++++++++
 kernel/module/main.c     | 531 +--------------------------------------
 4 files changed, 541 insertions(+), 525 deletions(-)
 create mode 100644 kernel/module/kallsyms.c

diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index e2a5ae0264f9..a5a0963ba481 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -12,4 +12,5 @@ ifdef CONFIG_MODULES
 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 1c9604443e2f..99b88e2eb479 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -68,6 +68,19 @@ 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);
+int cmp_name(const void *name, const void *sym);
+long module_get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr,
+		       unsigned int section);
+
+static inline 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);
+#else
+	return sym->value;
+#endif
+}
 
 #ifdef CONFIG_LIVEPATCH
 int copy_module_elf(struct module *mod, struct load_info *info);
@@ -180,3 +193,22 @@ void kmemleak_load_module(const struct module *mod, const struct load_info *info
 static inline void 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);
+unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name);
+
+static inline bool sect_empty(const Elf_Shdr *sect)
+{
+	return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
+}
+#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) { }
+#endif /* CONFIG_KALLSYMS */
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
new file mode 100644
index 000000000000..6c8f1f390cf5
--- /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 && 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 = module_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 = module_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 = (struct mod_kallsyms __rcu *)mod->init_layout.base +
+		info->mod_kallsyms_init_off;
+
+	/* The following is safe since this pointer cannot change */
+	rcu_dereference_sched(mod->kallsyms)->symtab = (void *)symsec->sh_addr;
+	rcu_dereference_sched(mod->kallsyms)->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
+	/* Make sure we get permanent strtab: don't use info->strtab. */
+	rcu_dereference_sched(mod->kallsyms)->strtab =
+		(void *)info->sechdrs[info->index.str].sh_addr;
+	rcu_dereference_sched(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 = rcu_dereference_sched(mod->kallsyms)->symtab;
+	for (ndst = i = 0; i < rcu_dereference_sched(mod->kallsyms)->num_symtab; i++) {
+		rcu_dereference_sched(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] =
+			    rcu_dereference_sched(mod->kallsyms)->typetab[i];
+			dst[ndst] = src[i];
+			dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
+			s += strscpy(s,
+				     &rcu_dereference_sched(mod->kallsyms)->strtab[src[i].st_name],
+				     KSYM_NAME_LEN) + 1;
+		}
+	}
+	mod->core_kallsyms.num_symtab = ndst;
+}
+
+#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.
+ */
+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;
+
+			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 */
+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) {
+		/* Still use rcu_dereference_sched to remain compliant with sparse */
+		struct mod_kallsyms *kallsyms = rcu_dereference_sched(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 7dd283959c5c..952079987ea4 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -288,15 +288,6 @@ static bool check_exported_symbol(const struct symsearch *syms,
 	return true;
 }
 
-static 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);
-#else
-	return sym->value;
-#endif
-}
-
 static const char *kernel_symbol_name(const struct kernel_symbol *sym)
 {
 #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
@@ -317,7 +308,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));
 }
@@ -387,8 +378,8 @@ 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,
-				      bool even_unformed)
+struct module *find_module_all(const char *name, size_t len,
+			       bool even_unformed)
 {
 	struct module *mod;
 
@@ -1294,13 +1285,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>
@@ -2065,7 +2049,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 module_get_offset(struct module *mod, unsigned int *size,
 		       Elf_Shdr *sechdr, unsigned int section)
 {
 	long ret;
@@ -2121,7 +2105,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 			    || s->sh_entsize != ~0UL
 			    || module_init_layout_section(sname))
 				continue;
-			s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
+			s->sh_entsize = module_get_offset(mod, &mod->core_layout.size, s, i);
 			pr_debug("\t%s\n", sname);
 		}
 		switch (m) {
@@ -2154,7 +2138,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
 			    || s->sh_entsize != ~0UL
 			    || !module_init_layout_section(sname))
 				continue;
-			s->sh_entsize = (get_offset(mod, &mod->init_layout.size, s, i)
+			s->sh_entsize = (module_get_offset(mod, &mod->init_layout.size, s, i)
 					 | INIT_OFFSET_MASK);
 			pr_debug("\t%s\n", sname);
 		}
@@ -2267,228 +2251,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)
@@ -3799,287 +3561,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


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v6 10/13] module: Move procfs support into a separate file
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (8 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 09/13] module: Move kallsyms support into " Aaron Tomlin
@ 2022-02-18 21:25 ` Aaron Tomlin
  2022-02-19  2:12 ` [PATCH v6 00/13] module: core code clean up Luis Chamberlain
  10 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-18 21:25 UTC (permalink / raw)
  To: mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

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 a5a0963ba481..f66fda0b41cc 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 99b88e2eb479..ddb37024a0d6 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -72,6 +72,7 @@ struct module *find_module_all(const char *name, size_t len, bool even_unformed)
 int cmp_name(const void *name, const void *sym);
 long module_get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr,
 		       unsigned int section);
+char *module_flags(struct module *mod, char *buf);
 
 static inline unsigned long kernel_symbol_value(const struct kernel_symbol *sym)
 {
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 952079987ea4..44b6fd1acc44 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>
@@ -805,31 +804,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 = {
@@ -919,12 +893,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)
 {
 }
@@ -3596,7 +3564,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;
 
@@ -3619,103 +3587,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..2717e130788e
--- /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 && !mod->exit) {
+		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


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 00/13] module: core code clean up
  2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
                   ` (9 preceding siblings ...)
  2022-02-18 21:25 ` [PATCH v6 10/13] module: Move procfs " Aaron Tomlin
@ 2022-02-19  2:12 ` Luis Chamberlain
  2022-02-21 12:47   ` Miroslav Benes
                     ` (2 more replies)
  10 siblings, 3 replies; 32+ messages in thread
From: Luis Chamberlain @ 2022-02-19  2:12 UTC (permalink / raw)
  To: Aaron Tomlin
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

On Fri, Feb 18, 2022 at 09:24:58PM +0000, Aaron Tomlin wrote:
> Hi Luis,
> 
> As per your suggestion [1], this is an attempt to refactor and split
> optional code out of core module support code into separate components.
> This version is based on Linus' commit 7993e65fdd0f ("Merge tag
> 'mtd/fixes-for-5.17-rc5' of
> git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux").
> Please let me know your thoughts.

Fantastic, thanks for doing all this work, I've pushed this out to
modules-next so that the testing can start as this will be in linux-next
soon. I'll obviously wait for more reviews, we have a long time before this
gets merged to Linus, so just want to start getting testing done now rather
than later. And other folks are depending on your changes to start
getting their own code up too.

Thanks!

  Luis

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 06/13] module: Move strict rwx support to a separate file
  2022-02-18 21:25 ` [PATCH v6 06/13] module: Move strict rwx " Aaron Tomlin
@ 2022-02-21  6:31   ` Christophe Leroy
  2022-02-21  9:35     ` Aaron Tomlin
  2022-02-21 11:27   ` Christophe Leroy
  2022-02-21 16:15   ` Christophe Leroy
  2 siblings, 1 reply; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21  6:31 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:25, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates code that makes module text
> and rodata memory read-only and non-text memory
> non-executable from core module code into
> kernel/module/strict_rwx.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   kernel/module/Makefile     |  1 +
>   kernel/module/internal.h   | 38 +++++++++++++++
>   kernel/module/main.c       | 99 +-------------------------------------
>   kernel/module/strict_rwx.c | 84 ++++++++++++++++++++++++++++++++
>   4 files changed, 125 insertions(+), 97 deletions(-)
>   create mode 100644 kernel/module/strict_rwx.c

   CC      kernel/module/strict_rwx.o
In file included from ./include/linux/build_bug.h:5,
                  from ./include/linux/container_of.h:5,
                  from ./include/linux/list.h:5,
                  from ./include/linux/module.h:12,
                  from kernel/module/strict_rwx.c:8:
kernel/module/strict_rwx.c: In function 'frob_rodata':
kernel/module/strict_rwx.c:16:17: error: implicit declaration of 
function 'PAGE_ALIGNED'; did you mean 'IS_ALIGNED'? 
[-Werror=implicit-function-declaration]
    16 |         BUG_ON(!PAGE_ALIGNED(layout->base));
       |                 ^~~~~~~~~~~~
./include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
    78 | # define unlikely(x)    __builtin_expect(!!(x), 0)
       |                                             ^
kernel/module/strict_rwx.c:16:9: note: in expansion of macro 'BUG_ON'
    16 |         BUG_ON(!PAGE_ALIGNED(layout->base));
       |         ^~~~~~
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:288 : kernel/module/strict_rwx.o] 
Erreur 1



You have to include <linux/mm.h>

Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-18 21:25 ` [PATCH v6 09/13] module: Move kallsyms support into " Aaron Tomlin
@ 2022-02-21  8:15   ` Christophe Leroy
  2022-02-21  8:35     ` Christophe Leroy
  2022-02-21  9:21     ` Aaron Tomlin
  2022-02-21 10:49   ` Christophe Leroy
  1 sibling, 2 replies; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21  8:15 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:25, 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>

With this patch I get an Oops:

[   36.421711] BUG: Unable to handle kernel data access on write at 
0xbe79bb40
[   36.428435] Faulting instruction address: 0xc008b74c
[   36.433342] Oops: Kernel access of bad area, sig: 11 [#1]
[   36.438672] BE PAGE_SIZE=16K PREEMPT CMPC885
[   36.442947] SAF3000 DIE NOTIFICATION
[   36.446421] Modules linked in:
[   36.449436] CPU: 0 PID: 374 Comm: insmod Not tainted 
5.17.0-rc4-s3k-dev-02274-g7d4ec8831803 #1016
[   36.458211] NIP:  c008b74c LR: c00897ac CTR: c001145c
[   36.463200] REGS: caf8bcf0 TRAP: 0300   Not tainted 
(5.17.0-rc4-s3k-dev-02274-g7d4ec8831803)
[   36.471633] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 24002842  XER: 00000000
[   36.478261] DAR: be79bb40 DSISR: c2000000
[   36.478261] GPR00: c00899a0 caf8bdb0 c230a980 be74c000 caf8beb8 
00000008 00000000 c035629c
[   36.478261] GPR08: be75c000 caf9c9fc be79bb40 00000004 24002842 
100d166a 00000290 00000000
[   36.478261] GPR16: c0747320 caf9c9fc c11ff918 caf9b2a7 be75c290 
00000550 00000022 c0747210
[   36.478261] GPR24: c11ff8e8 be74c000 c11ff8fc 100b8820 00000000 
00000000 be74c000 caf8beb8
[   36.516729] NIP [c008b74c] add_kallsyms+0x48/0x30c
[   36.521465] LR [c00897ac] load_module+0x16f8/0x2504
[   36.526286] Call Trace:
[   36.528693] [caf8bdb0] [00000208] 0x208 (unreliable)
[   36.533598] [caf8bde0] [c00899a0] load_module+0x18ec/0x2504
[   36.539107] [caf8beb0] [c008a7a8] sys_finit_module+0xb4/0xf8
[   36.544700] [caf8bf30] [c00120a4] ret_from_syscall+0x0/0x28
[   36.550209] --- interrupt: c00 at 0xfd5e7c0
[   36.554339] NIP:  0fd5e7c0 LR: 100144e8 CTR: 10013238
[   36.559331] REGS: caf8bf40 TRAP: 0c00   Not tainted 
(5.17.0-rc4-s3k-dev-02274-g7d4ec8831803)
[   36.567763] MSR:  0000d032 <EE,PR,ME,IR,DR,RI>  CR: 24002222  XER: 
00000000
[   36.574735]
[   36.574735] GPR00: 00000161 7fc5a130 7792f4e0 00000003 100b8820 
00000000 0fd4ded4 0000d032
[   36.574735] GPR08: 00000000 10013238 00000000 00000002 142d2297 
100d166a 100a0920 00000000
[   36.574735] GPR16: 100cac0c 100b0000 1013c3cc 1013d685 100d0000 
100d0000 00000000 100a0900
[   36.574735] GPR24: ffffffa2 ffffffff 1013c3a4 00000003 1013c3cc 
100b8820 1013c3f0 1013c3cc
[   36.610535] NIP [0fd5e7c0] 0xfd5e7c0
[   36.614065] LR [100144e8] 0x100144e8
[   36.617593] --- interrupt: c00
[   36.620609] Instruction dump:
[   36.623534] 7c7e1b78 81040038 8124003c 81430100 55082036 7d0a4214 
1d490028 81240010
[   36.631365] 9103015c 7d295214 8109000c 8143015c <910a0000> 81290014 
8143015c 5529e13e
[   36.639376] ---[ end trace 0000000000000000 ]---


Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-21  8:15   ` Christophe Leroy
@ 2022-02-21  8:35     ` Christophe Leroy
  2022-02-21  9:22       ` Aaron Tomlin
  2022-02-21  9:21     ` Aaron Tomlin
  1 sibling, 1 reply; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21  8:35 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 21/02/2022 à 09:15, Christophe Leroy a écrit :
> 
> 
> Le 18/02/2022 à 22:25, 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>
> 
> With this patch I get an Oops:
> 
> [   36.421711] BUG: Unable to handle kernel data access on write at 
> 0xbe79bb40
> [   36.428435] Faulting instruction address: 0xc008b74c
> [   36.433342] Oops: Kernel access of bad area, sig: 11 [#1]
> [   36.438672] BE PAGE_SIZE=16K PREEMPT CMPC885
> [   36.442947] SAF3000 DIE NOTIFICATION
> [   36.446421] Modules linked in:
> [   36.449436] CPU: 0 PID: 374 Comm: insmod Not tainted 
> 5.17.0-rc4-s3k-dev-02274-g7d4ec8831803 #1016
> [   36.458211] NIP:  c008b74c LR: c00897ac CTR: c001145c
> [   36.463200] REGS: caf8bcf0 TRAP: 0300   Not tainted 
> (5.17.0-rc4-s3k-dev-02274-g7d4ec8831803)
> [   36.471633] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 24002842  XER: 00000000
> [   36.478261] DAR: be79bb40 DSISR: c2000000
> [   36.478261] GPR00: c00899a0 caf8bdb0 c230a980 be74c000 caf8beb8 
> 00000008 00000000 c035629c
> [   36.478261] GPR08: be75c000 caf9c9fc be79bb40 00000004 24002842 
> 100d166a 00000290 00000000
> [   36.478261] GPR16: c0747320 caf9c9fc c11ff918 caf9b2a7 be75c290 
> 00000550 00000022 c0747210
> [   36.478261] GPR24: c11ff8e8 be74c000 c11ff8fc 100b8820 00000000 
> 00000000 be74c000 caf8beb8
> [   36.516729] NIP [c008b74c] add_kallsyms+0x48/0x30c
> [   36.521465] LR [c00897ac] load_module+0x16f8/0x2504

Fixup:

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 6c8f1f390cf5..2ee8d2e67068 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -171,8 +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 = (struct mod_kallsyms __rcu *)mod->init_layout.base +
-		info->mod_kallsyms_init_off;
+	mod->kallsyms = (void __rcu *)mod->init_layout.base + 
info->mod_kallsyms_init_off;

  	/* The following is safe since this pointer cannot change */
  	rcu_dereference_sched(mod->kallsyms)->symtab = (void *)symsec->sh_addr;


Christophe

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-21  8:15   ` Christophe Leroy
  2022-02-21  8:35     ` Christophe Leroy
@ 2022-02-21  9:21     ` Aaron Tomlin
  1 sibling, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-21  9:21 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: mcgrof, cl, pmladek, mbenes, akpm, jeyu, linux-kernel,
	linux-modules, live-patching, atomlin, ghalat, allen.lkml, joe,
	msuchanek, oleksandr

On Mon 2022-02-21 08:15 +0000, Christophe Leroy wrote:
> [   36.421711] BUG: Unable to handle kernel data access on write at
> 0xbe79bb40
> [   36.428435] Faulting instruction address: 0xc008b74c
> [   36.433342] Oops: Kernel access of bad area, sig: 11 [#1]
> [   36.438672] BE PAGE_SIZE=16K PREEMPT CMPC885
> [   36.442947] SAF3000 DIE NOTIFICATION
> [   36.446421] Modules linked in:
> [   36.449436] CPU: 0 PID: 374 Comm: insmod Not tainted
> 5.17.0-rc4-s3k-dev-02274-g7d4ec8831803 #1016
> [   36.458211] NIP:  c008b74c LR: c00897ac CTR: c001145c
> [   36.463200] REGS: caf8bcf0 TRAP: 0300   Not tainted
> (5.17.0-rc4-s3k-dev-02274-g7d4ec8831803)
> [   36.471633] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 24002842  XER: 00000000
> [   36.478261] DAR: be79bb40 DSISR: c2000000
> [   36.478261] GPR00: c00899a0 caf8bdb0 c230a980 be74c000 caf8beb8
> 00000008 00000000 c035629c
> [   36.478261] GPR08: be75c000 caf9c9fc be79bb40 00000004 24002842
> 100d166a 00000290 00000000
> [   36.478261] GPR16: c0747320 caf9c9fc c11ff918 caf9b2a7 be75c290
> 00000550 00000022 c0747210
> [   36.478261] GPR24: c11ff8e8 be74c000 c11ff8fc 100b8820 00000000
> 00000000 be74c000 caf8beb8
> [   36.516729] NIP [c008b74c] add_kallsyms+0x48/0x30c
> [   36.521465] LR [c00897ac] load_module+0x16f8/0x2504
> [   36.526286] Call Trace:
> [   36.528693] [caf8bdb0] [00000208] 0x208 (unreliable)
> [   36.533598] [caf8bde0] [c00899a0] load_module+0x18ec/0x2504
> [   36.539107] [caf8beb0] [c008a7a8] sys_finit_module+0xb4/0xf8
> [   36.544700] [caf8bf30] [c00120a4] ret_from_syscall+0x0/0x28
> [   36.550209] --- interrupt: c00 at 0xfd5e7c0
> [   36.554339] NIP:  0fd5e7c0 LR: 100144e8 CTR: 10013238
> [   36.559331] REGS: caf8bf40 TRAP: 0c00   Not tainted
> (5.17.0-rc4-s3k-dev-02274-g7d4ec8831803)
> [   36.567763] MSR:  0000d032 <EE,PR,ME,IR,DR,RI>  CR: 24002222  XER:
> 00000000
> [   36.574735]
> [   36.574735] GPR00: 00000161 7fc5a130 7792f4e0 00000003 100b8820
> 00000000 0fd4ded4 0000d032
> [   36.574735] GPR08: 00000000 10013238 00000000 00000002 142d2297
> 100d166a 100a0920 00000000
> [   36.574735] GPR16: 100cac0c 100b0000 1013c3cc 1013d685 100d0000
> 100d0000 00000000 100a0900
> [   36.574735] GPR24: ffffffa2 ffffffff 1013c3a4 00000003 1013c3cc
> 100b8820 1013c3f0 1013c3cc
> [   36.610535] NIP [0fd5e7c0] 0xfd5e7c0
> [   36.614065] LR [100144e8] 0x100144e8
> [   36.617593] --- interrupt: c00
> [   36.620609] Instruction dump:
> [   36.623534] 7c7e1b78 81040038 8124003c 81430100 55082036 7d0a4214
> 1d490028 81240010
> [   36.631365] 9103015c 7d295214 8109000c 8143015c <910a0000> 81290014
> 8143015c 5529e13e
> [   36.639376] ---[ end trace 0000000000000000 ]---

Hi Christophe,

Unfortunately, I do not have the debuginfo data. Albeit, based on the stack
trace, I suspect this was the result of the dereference attempt (below) due
to the invalid pointer stored in mod->kallsyms i.e. mod->kallsyms = (struct
mod_kallsyms __rcu *)mod->init_layout.base + info->mod_kallsyms_init_off:

  177             /* The following is safe since this pointer cannot change */
  178             rcu_dereference_sched(mod->kallsyms)->symtab = (void
*)symsec->sh_addr;

Kind regards,

-- 
Aaron Tomlin


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-21  8:35     ` Christophe Leroy
@ 2022-02-21  9:22       ` Aaron Tomlin
  2022-02-22  9:58         ` Miroslav Benes
  0 siblings, 1 reply; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-21  9:22 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Luis R. Rodriguez, Lameter, Christoph, Petr Mladek,
	Miroslav Benes, Andrew Morton, jeyu, linux-kernel, linux-modules,
	live-patching, Aaron Tomlin, Grzegorz Halat, Allen, Joe Perches,
	Michal Suchanek, oleksandr

On Mon 2022-02-21 09:35 +0100, Christophe Leroy wrote:
> Fixup:
>
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index 6c8f1f390cf5..2ee8d2e67068 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -171,8 +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 = (struct mod_kallsyms __rcu *)mod->init_layout.base +
> -        info->mod_kallsyms_init_off;
> +    mod->kallsyms = (void __rcu *)mod->init_layout.base +
> info->mod_kallsyms_init_off;
>
>      /* The following is safe since this pointer cannot change */
>      rcu_dereference_sched(mod->kallsyms)->symtab = (void *)symsec->sh_addr;

Agreed.

-- 
Aaron Tomlin


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 06/13] module: Move strict rwx support to a separate file
  2022-02-21  6:31   ` Christophe Leroy
@ 2022-02-21  9:35     ` Aaron Tomlin
  0 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-21  9:35 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: mcgrof, cl, pmladek, mbenes, akpm, jeyu, linux-kernel,
	linux-modules, live-patching, atomlin, ghalat, allen.lkml, joe,
	msuchanek, oleksandr

On Mon 2022-02-21 06:31 +0000, Christophe Leroy wrote:
>    CC      kernel/module/strict_rwx.o
> In file included from ./include/linux/build_bug.h:5,
>                   from ./include/linux/container_of.h:5,
>                   from ./include/linux/list.h:5,
>                   from ./include/linux/module.h:12,
>                   from kernel/module/strict_rwx.c:8:
> kernel/module/strict_rwx.c: In function 'frob_rodata':
> kernel/module/strict_rwx.c:16:17: error: implicit declaration of
> function 'PAGE_ALIGNED'; did you mean 'IS_ALIGNED'?
> [-Werror=implicit-function-declaration]
>     16 |         BUG_ON(!PAGE_ALIGNED(layout->base));
>        |                 ^~~~~~~~~~~~
> ./include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
>     78 | # define unlikely(x)    __builtin_expect(!!(x), 0)
>        |                                             ^
> kernel/module/strict_rwx.c:16:9: note: in expansion of macro 'BUG_ON'
>     16 |         BUG_ON(!PAGE_ALIGNED(layout->base));
>        |         ^~~~~~
> cc1: some warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:288 : kernel/module/strict_rwx.o]
> Erreur 1
>
>
>
> You have to include <linux/mm.h>

Christophe,

Strange, I have not seen this before. Locally, looking at
kernel/module/strict_rwx.i it is eventually included. Anyhow, you are
right. Also, it will not hurt due to the multiple-include optimisation
found in include/linux/mm.h.


Kind regards,

-- 
Aaron Tomlin


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-18 21:25 ` [PATCH v6 09/13] module: Move kallsyms support into " Aaron Tomlin
  2022-02-21  8:15   ` Christophe Leroy
@ 2022-02-21 10:49   ` Christophe Leroy
  2022-02-21 12:02     ` Aaron Tomlin
  1 sibling, 1 reply; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21 10:49 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:25, 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 |  32 +++
>   kernel/module/kallsyms.c | 502 ++++++++++++++++++++++++++++++++++++
>   kernel/module/main.c     | 531 +--------------------------------------
>   4 files changed, 541 insertions(+), 525 deletions(-)
>   create mode 100644 kernel/module/kallsyms.c
> 

   CC      kernel/module/main.o
kernel/module/main.c: In function 'load_module':
kernel/module/main.c:3379:9: error: implicit declaration of function 
'init_build_id' [-Werror=implicit-function-declaration]
  3379 |         init_build_id(mod, info);
       |         ^~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:288 : kernel/module/main.o] Erreur 1

Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 05/13] module: Move latched RB-tree support to a separate file
  2022-02-18 21:25 ` [PATCH v6 05/13] module: Move latched RB-tree " Aaron Tomlin
@ 2022-02-21 10:57   ` Christophe Leroy
  0 siblings, 0 replies; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21 10:57 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:25, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates module latched RB-tree support
> (e.g. see __module_address()) from core module code
> into kernel/module/tree_lookup.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   kernel/module/Makefile      |   3 +
>   kernel/module/internal.h    |  33 +++++++++
>   kernel/module/main.c        | 130 ++----------------------------------
>   kernel/module/tree_lookup.c | 109 ++++++++++++++++++++++++++++++
>   4 files changed, 149 insertions(+), 126 deletions(-)
>   create mode 100644 kernel/module/tree_lookup.c


   CC      kernel/module/main.o
kernel/module/main.c:3723:6: warning: no previous prototype for 
'module_layout' [-Wmissing-prototypes]
  3723 | void module_layout(struct module *mod,
       |      ^~~~~~~~~~~~~
   CC      kernel/module/strict_rwx.o
In file included from kernel/module/strict_rwx.c:12:
kernel/module/internal.h:140:13: warning: 'mod_tree_remove' defined but 
not used [-Wunused-function]
   140 | static void mod_tree_remove(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~
kernel/module/internal.h:139:13: warning: 'mod_tree_remove_init' defined 
but not used [-Wunused-function]
   139 | static void mod_tree_remove_init(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~~~~~~
kernel/module/internal.h:138:13: warning: 'mod_tree_insert' defined but 
not used [-Wunused-function]
   138 | static void mod_tree_insert(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~
   CC      kernel/module/kallsyms.o
In file included from kernel/module/kallsyms.c:12:
kernel/module/internal.h:140:13: warning: 'mod_tree_remove' defined but 
not used [-Wunused-function]
   140 | static void mod_tree_remove(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~
kernel/module/internal.h:139:13: warning: 'mod_tree_remove_init' defined 
but not used [-Wunused-function]
   139 | static void mod_tree_remove_init(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~~~~~~
kernel/module/internal.h:138:13: warning: 'mod_tree_insert' defined but 
not used [-Wunused-function]
   138 | static void mod_tree_insert(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~
   CC      kernel/module/procfs.o
In file included from kernel/module/procfs.c:13:
kernel/module/internal.h:140:13: warning: 'mod_tree_remove' defined but 
not used [-Wunused-function]
   140 | static void mod_tree_remove(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~
kernel/module/internal.h:139:13: warning: 'mod_tree_remove_init' defined 
but not used [-Wunused-function]
   139 | static void mod_tree_remove_init(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~~~~~~
kernel/module/internal.h:138:13: warning: 'mod_tree_insert' defined but 
not used [-Wunused-function]
   138 | static void mod_tree_insert(struct module *mod) { }
       |             ^~~~~~~~~~~~~~~


Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 06/13] module: Move strict rwx support to a separate file
  2022-02-18 21:25 ` [PATCH v6 06/13] module: Move strict rwx " Aaron Tomlin
  2022-02-21  6:31   ` Christophe Leroy
@ 2022-02-21 11:27   ` Christophe Leroy
  2022-02-21 16:15   ` Christophe Leroy
  2 siblings, 0 replies; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21 11:27 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:25, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates code that makes module text
> and rodata memory read-only and non-text memory
> non-executable from core module code into
> kernel/module/strict_rwx.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   kernel/module/Makefile     |  1 +
>   kernel/module/internal.h   | 38 +++++++++++++++
>   kernel/module/main.c       | 99 +-------------------------------------
>   kernel/module/strict_rwx.c | 84 ++++++++++++++++++++++++++++++++
>   4 files changed, 125 insertions(+), 97 deletions(-)
>   create mode 100644 kernel/module/strict_rwx.c


kernel/module/internal.h:182:12: error: 'module_enforce_rwx_sections' 
defined but not used [-Werror=unused-function]
kernel/module/internal.h:181:13: error: 'module_enable_ro' defined but 
not used [-Werror=unused-function]
kernel/module/internal.h:180:13: error: 'module_enable_nx' defined but 
not used [-Werror=unused-function]
cc1: all warnings being treated as errors
make[3]: *** [scripts/Makefile.build:288: kernel/module/signing.o] Error 1

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-21 10:49   ` Christophe Leroy
@ 2022-02-21 12:02     ` Aaron Tomlin
  0 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-21 12:02 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Aaron Tomlin, mcgrof, cl, pmladek, mbenes, akpm, jeyu,
	linux-kernel, linux-modules, live-patching, ghalat, allen.lkml,
	joe, msuchanek, oleksandr

On Mon 2022-02-21 10:49 +0000, Christophe Leroy wrote:
> Le 18/02/2022 à 22:25, 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 |  32 +++
> >   kernel/module/kallsyms.c | 502 ++++++++++++++++++++++++++++++++++++
> >   kernel/module/main.c     | 531 +--------------------------------------
> >   4 files changed, 541 insertions(+), 525 deletions(-)
> >   create mode 100644 kernel/module/kallsyms.c
> > 
> 
>    CC      kernel/module/main.o
> kernel/module/main.c: In function 'load_module':
> kernel/module/main.c:3379:9: error: implicit declaration of function 
> 'init_build_id' [-Werror=implicit-function-declaration]
>   3379 |         init_build_id(mod, info);
>        |         ^~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:288 : kernel/module/main.o] Erreur 1

Christophe,

I assume both Kconfig option CONFIG_KALLSYMS and CONFIG_STACKTRACE_BUILD_ID
was not enabled. I'll address this.

Thanks,

-- 
Aaron Tomlin

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 01/13] module: Move all into module/
  2022-02-18 21:24 ` [PATCH v6 01/13] module: Move all into module/ Aaron Tomlin
@ 2022-02-21 12:21   ` Christophe Leroy
  2022-02-22 10:48     ` Aaron Tomlin
  2022-02-21 13:13   ` Christophe Leroy
  1 sibling, 1 reply; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21 12:21 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:24, Aaron Tomlin a écrit :
> No functional changes.
> 
> This patch moves all module related code into a separate directory,
> modifies each file name and creates a new Makefile. Note: this effort
> is in preparation to refactor core module code.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   MAINTAINERS                                         | 2 +-
>   kernel/Makefile                                     | 5 +----
>   kernel/module/Makefile                              | 9 +++++++++
>   kernel/{module_decompress.c => module/decompress.c} | 2 +-
>   kernel/{module-internal.h => module/internal.h}     | 0
>   kernel/{module.c => module/main.c}                  | 2 +-
>   kernel/{module_signature.c => module/signature.c}   | 0
>   kernel/{module_signing.c => module/signing.c}       | 2 +-
>   8 files changed, 14 insertions(+), 8 deletions(-)
>   create mode 100644 kernel/module/Makefile
>   rename kernel/{module_decompress.c => module/decompress.c} (99%)
>   rename kernel/{module-internal.h => module/internal.h} (100%)
>   rename kernel/{module.c => module/main.c} (99%)
>   rename kernel/{module_signature.c => module/signature.c} (100%)
>   rename kernel/{module_signing.c => module/signing.c} (97%)
> 

I'm wondering whether we should avoid moving module_signature.c and 
leave it in kernel/ as this file is used even when CONFIG_MODULES is not 
selected, and he is the only one like this.

Keeping it outside of kernel/module/ would allow to conditionaly build 
entire kernel/module/ based of CONFIG_MODULES and then avoid all checks 
against CONFIG_MODULES which look misleading at times.


Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 00/13] module: core code clean up
  2022-02-19  2:12 ` [PATCH v6 00/13] module: core code clean up Luis Chamberlain
@ 2022-02-21 12:47   ` Miroslav Benes
  2022-02-22 10:58   ` Christophe Leroy
  2022-02-22 11:18   ` Aaron Tomlin
  2 siblings, 0 replies; 32+ messages in thread
From: Miroslav Benes @ 2022-02-21 12:47 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Aaron Tomlin, cl, pmladek, akpm, jeyu, linux-kernel,
	linux-modules, live-patching, atomlin, ghalat, allen.lkml, joe,
	christophe.leroy, msuchanek, oleksandr

On Fri, 18 Feb 2022, Luis Chamberlain wrote:

> On Fri, Feb 18, 2022 at 09:24:58PM +0000, Aaron Tomlin wrote:
> > Hi Luis,
> > 
> > As per your suggestion [1], this is an attempt to refactor and split
> > optional code out of core module support code into separate components.
> > This version is based on Linus' commit 7993e65fdd0f ("Merge tag
> > 'mtd/fixes-for-5.17-rc5' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux").
> > Please let me know your thoughts.
> 
> Fantastic, thanks for doing all this work, I've pushed this out to
> modules-next so that the testing can start as this will be in linux-next
> soon. I'll obviously wait for more reviews, we have a long time before this
> gets merged to Linus, so just want to start getting testing done now rather
> than later. And other folks are depending on your changes to start
> getting their own code up too.

Aaron's series is unfortunately split. Could you also push out the 
remaining 3 patches (20220218212757.888751-1-atomlin@redhat.com), please?

Miroslav

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 01/13] module: Move all into module/
  2022-02-18 21:24 ` [PATCH v6 01/13] module: Move all into module/ Aaron Tomlin
  2022-02-21 12:21   ` Christophe Leroy
@ 2022-02-21 13:13   ` Christophe Leroy
  1 sibling, 0 replies; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21 13:13 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:24, Aaron Tomlin a écrit :
> No functional changes.
> 
> This patch moves all module related code into a separate directory,
> modifies each file name and creates a new Makefile. Note: this effort
> is in preparation to refactor core module code.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   MAINTAINERS                                         | 2 +-
>   kernel/Makefile                                     | 5 +----
>   kernel/module/Makefile                              | 9 +++++++++
>   kernel/{module_decompress.c => module/decompress.c} | 2 +-
>   kernel/{module-internal.h => module/internal.h}     | 0
>   kernel/{module.c => module/main.c}                  | 2 +-
>   kernel/{module_signature.c => module/signature.c}   | 0
>   kernel/{module_signing.c => module/signing.c}       | 2 +-
>   8 files changed, 14 insertions(+), 8 deletions(-)
>   create mode 100644 kernel/module/Makefile
>   rename kernel/{module_decompress.c => module/decompress.c} (99%)
>   rename kernel/{module-internal.h => module/internal.h} (100%)
>   rename kernel/{module.c => module/main.c} (99%)
>   rename kernel/{module_signature.c => module/signature.c} (100%)
>   rename kernel/{module_signing.c => module/signing.c} (97%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bd86ed9fbc79..463bdb829db4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13012,7 +13012,7 @@ L:	linux-kernel@vger.kernel.org
>   S:	Maintained
>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next
>   F:	include/linux/module.h
> -F:	kernel/module.c
> +F:	kernel/module/
>   
>   MONOLITHIC POWER SYSTEM PMIC DRIVER
>   M:	Saravanan Sekar <sravanhome@gmail.com>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 56f4ee97f328..3a6380975c57 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile

This file also contains:

KCOV_INSTRUMENT_module.o := n

It needs to follow through into kernel/module/Makefile, with a copy of 
the comment. And then it needs to be taken care of while dismantling main.c


> @@ -53,6 +53,7 @@ obj-y += rcu/
>   obj-y += livepatch/
>   obj-y += dma/
>   obj-y += entry/
> +obj-y += module/
>   
>   obj-$(CONFIG_KCMP) += kcmp.o
>   obj-$(CONFIG_FREEZER) += freezer.o
> @@ -66,10 +67,6 @@ ifneq ($(CONFIG_SMP),y)
>   obj-y += up.o
>   endif
>   obj-$(CONFIG_UID16) += uid16.o
> -obj-$(CONFIG_MODULES) += module.o
> -obj-$(CONFIG_MODULE_DECOMPRESS) += module_decompress.o
> -obj-$(CONFIG_MODULE_SIG) += module_signing.o
> -obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signature.o
>   obj-$(CONFIG_KALLSYMS) += kallsyms.o
>   obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
>   obj-$(CONFIG_CRASH_CORE) += crash_core.o
> diff --git a/kernel/module/Makefile b/kernel/module/Makefile
> new file mode 100644
> index 000000000000..2902fc7d0ef1
> --- /dev/null
> +++ b/kernel/module/Makefile
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for linux kernel module support
> +#
> +
> +obj-$(CONFIG_MODULES) += main.o
> +obj-$(CONFIG_MODULE_DECOMPRESS) += decompress.o
> +obj-$(CONFIG_MODULE_SIG) += signing.o
> +obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 06/13] module: Move strict rwx support to a separate file
  2022-02-18 21:25 ` [PATCH v6 06/13] module: Move strict rwx " Aaron Tomlin
  2022-02-21  6:31   ` Christophe Leroy
  2022-02-21 11:27   ` Christophe Leroy
@ 2022-02-21 16:15   ` Christophe Leroy
  2 siblings, 0 replies; 32+ messages in thread
From: Christophe Leroy @ 2022-02-21 16:15 UTC (permalink / raw)
  To: Aaron Tomlin, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 18/02/2022 à 22:25, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates code that makes module text
> and rodata memory read-only and non-text memory
> non-executable from core module code into
> kernel/module/strict_rwx.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
>   kernel/module/Makefile     |  1 +
>   kernel/module/internal.h   | 38 +++++++++++++++
>   kernel/module/main.c       | 99 +-------------------------------------
>   kernel/module/strict_rwx.c | 84 ++++++++++++++++++++++++++++++++
>   4 files changed, 125 insertions(+), 97 deletions(-)
>   create mode 100644 kernel/module/strict_rwx.c
> 
> diff --git a/kernel/module/Makefile b/kernel/module/Makefile
> index 6fb21ebe1aa3..3f48343636ff 100644
> --- a/kernel/module/Makefile
> +++ b/kernel/module/Makefile
> @@ -10,4 +10,5 @@ obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o
>   obj-$(CONFIG_LIVEPATCH) += livepatch.o
>   ifdef CONFIG_MODULES
>   obj-$(CONFIG_MODULES_TREE_LOOKUP) += tree_lookup.o
> +obj-$(CONFIG_STRICT_MODULE_RWX) += strict_rwx.o
>   endif
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 57a715454c9e..f4b7e123d625 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -20,6 +20,17 @@
>   /* Maximum number of characters written by module_flags() */
>   #define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
>   
> +/*
> + * Modules' sections will be aligned on page boundaries
> + * to ensure complete separation of code and data, but
> + * only when CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y
> + */
> +#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
> +# define debug_align(X) PAGE_ALIGN(X)
> +#else
> +# define debug_align(X) (X)
> +#endif
> +
>   extern struct mutex module_mutex;
>   extern struct list_head modules;
>   
> @@ -126,3 +137,30 @@ static inline struct module *mod_find(unsigned long addr)
>   	return NULL;
>   }
>   #endif /* CONFIG_MODULES_TREE_LOOKUP */
> +
> +#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
> +void frob_text(const struct module_layout *layout, int (*set_memory)(unsigned long start,
> +								     int num_pages));
> +#endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
> +
> +#ifdef CONFIG_STRICT_MODULE_RWX
> +void frob_rodata(const struct module_layout *layout,
> +		 int (*set_memory)(unsigned long start, int num_pages));
> +void frob_ro_after_init(const struct module_layout *layout,
> +			int (*set_memory)(unsigned long start, int num_pages));
> +void frob_writable_data(const struct module_layout *layout,
> +			int (*set_memory)(unsigned long start, int num_pages));

Those three frob_() functions are only used in strict_rwx.c, they should 
not appear in internal.h and should be static in strict_rwx.c

> +void module_enable_ro(const struct module *mod, bool after_init);
> +void module_enable_nx(const struct module *mod);
> +int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> +				char *secstrings, struct module *mod);
> +
> +#else /* !CONFIG_STRICT_MODULE_RWX */
> +static void module_enable_nx(const struct module *mod) { }
> +static void module_enable_ro(const struct module *mod, bool after_init) {}
> +static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> +				       char *secstrings, struct module *mod)

Those three must be static inline

> +{
> +	return 0;
> +}
> +#endif /* CONFIG_STRICT_MODULE_RWX */


Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 09/13] module: Move kallsyms support into a separate file
  2022-02-21  9:22       ` Aaron Tomlin
@ 2022-02-22  9:58         ` Miroslav Benes
  0 siblings, 0 replies; 32+ messages in thread
From: Miroslav Benes @ 2022-02-22  9:58 UTC (permalink / raw)
  To: Aaron Tomlin
  Cc: Christophe Leroy, Luis R. Rodriguez, Lameter, Christoph,
	Petr Mladek, Andrew Morton, jeyu, linux-kernel, linux-modules,
	live-patching, Aaron Tomlin, Grzegorz Halat, Allen, Joe Perches,
	Michal Suchanek, oleksandr

On Mon, 21 Feb 2022, Aaron Tomlin wrote:

> On Mon 2022-02-21 09:35 +0100, Christophe Leroy wrote:
> > Fixup:
> >
> > diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> > index 6c8f1f390cf5..2ee8d2e67068 100644
> > --- a/kernel/module/kallsyms.c
> > +++ b/kernel/module/kallsyms.c
> > @@ -171,8 +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 = (struct mod_kallsyms __rcu *)mod->init_layout.base +
> > -        info->mod_kallsyms_init_off;
> > +    mod->kallsyms = (void __rcu *)mod->init_layout.base +
> > info->mod_kallsyms_init_off;
> >
> >      /* The following is safe since this pointer cannot change */
> >      rcu_dereference_sched(mod->kallsyms)->symtab = (void *)symsec->sh_addr;
> 
> Agreed.

Could you split all of this to a follow-up patch, please? So that 9/13 is 
really only about moving the code.

Btw, the sparse warnings "dereference of noderef expression" appeared 
after 5/13 of the series which moved latched RB-tree support, so it does 
not belong here anyway.

Miroslav

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 01/13] module: Move all into module/
  2022-02-21 12:21   ` Christophe Leroy
@ 2022-02-22 10:48     ` Aaron Tomlin
  2022-02-23  1:16       ` Luis Chamberlain
  0 siblings, 1 reply; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-22 10:48 UTC (permalink / raw)
  To: Christophe Leroy, mcgrof
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr

On Mon 2022-02-21 12:21 +0000, Christophe Leroy wrote:
>
>
> Le 18/02/2022 à 22:24, Aaron Tomlin a écrit :
> > No functional changes.
> >
> > This patch moves all module related code into a separate directory,
> > modifies each file name and creates a new Makefile. Note: this effort
> > is in preparation to refactor core module code.
> >
> > Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> > ---
> >   MAINTAINERS                                         | 2 +-
> >   kernel/Makefile                                     | 5 +----
> >   kernel/module/Makefile                              | 9 +++++++++
> >   kernel/{module_decompress.c => module/decompress.c} | 2 +-
> >   kernel/{module-internal.h => module/internal.h}     | 0
> >   kernel/{module.c => module/main.c}                  | 2 +-
> >   kernel/{module_signature.c => module/signature.c}   | 0
> >   kernel/{module_signing.c => module/signing.c}       | 2 +-
> >   8 files changed, 14 insertions(+), 8 deletions(-)
> >   create mode 100644 kernel/module/Makefile
> >   rename kernel/{module_decompress.c => module/decompress.c} (99%)
> >   rename kernel/{module-internal.h => module/internal.h} (100%)
> >   rename kernel/{module.c => module/main.c} (99%)
> >   rename kernel/{module_signature.c => module/signature.c} (100%)
> >   rename kernel/{module_signing.c => module/signing.c} (97%)
> >
>
> I'm wondering whether we should avoid moving module_signature.c and
> leave it in kernel/ as this file is used even when CONFIG_MODULES is not
> selected, and he is the only one like this.
>
> Keeping it outside of kernel/module/ would allow to conditionaly build
> entire kernel/module/ based of CONFIG_MODULES and then avoid all checks
> against CONFIG_MODULES which look misleading at times.

Luis,

What is your opinion on this? Indeed, mod_check_sig() is used by code
outside of kernel/module/ too i.e. ima_read_modsig(); albeit, I believe it
does make sense to keep it under kernel/module/ since the function in
question is used to review a given module's signature anyway.

Kind regards,

-- 
Aaron Tomlin


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 00/13] module: core code clean up
  2022-02-19  2:12 ` [PATCH v6 00/13] module: core code clean up Luis Chamberlain
  2022-02-21 12:47   ` Miroslav Benes
@ 2022-02-22 10:58   ` Christophe Leroy
  2022-02-22 11:18   ` Aaron Tomlin
  2 siblings, 0 replies; 32+ messages in thread
From: Christophe Leroy @ 2022-02-22 10:58 UTC (permalink / raw)
  To: Luis Chamberlain, Aaron Tomlin
  Cc: cl, pmladek, mbenes, akpm, jeyu, linux-kernel, linux-modules,
	live-patching, atomlin, ghalat, allen.lkml, joe, msuchanek,
	oleksandr



Le 19/02/2022 à 03:12, Luis Chamberlain a écrit :
> On Fri, Feb 18, 2022 at 09:24:58PM +0000, Aaron Tomlin wrote:
>> Hi Luis,
>>
>> As per your suggestion [1], this is an attempt to refactor and split
>> optional code out of core module support code into separate components.
>> This version is based on Linus' commit 7993e65fdd0f ("Merge tag
>> 'mtd/fixes-for-5.17-rc5' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux").
>> Please let me know your thoughts.
> 
> Fantastic, thanks for doing all this work, I've pushed this out to
> modules-next so that the testing can start as this will be in linux-next
> soon. I'll obviously wait for more reviews, we have a long time before this
> gets merged to Linus, so just want to start getting testing done now rather
> than later. And other folks are depending on your changes to start
> getting their own code up too.
> 

Hi Luis,

modules-next misses the 3 last patches from Aaron.

Aaron's series as build problems, I sent 4 fixups which allows it to 
build, then I rebased my series on top of that.

And I added on top of it some misc cleanups.

All this has undergone kisskb build test at 
http://kisskb.ellerman.id.au/kisskb/head/d8ee36c58284baf315e8aa3532a8d97abae8026d/
This only build failure (indeed a final link failure) is unrelated to my 
series.

Everything is also available on https://github.com/chleroy/linux.git in 
branch module-v4

Looking forward to getting everything into modules-next hence into 
linux-next.

Christophe

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 00/13] module: core code clean up
  2022-02-19  2:12 ` [PATCH v6 00/13] module: core code clean up Luis Chamberlain
  2022-02-21 12:47   ` Miroslav Benes
  2022-02-22 10:58   ` Christophe Leroy
@ 2022-02-22 11:18   ` Aaron Tomlin
  2 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-22 11:18 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Lameter, Christoph, Petr Mladek, Miroslav Benes, Andrew Morton,
	jeyu, linux-kernel, linux-modules, live-patching, Aaron Tomlin,
	Grzegorz Halat, Allen, Joe Perches, Christophe Leroy,
	Michal Suchanek, oleksandr

On Fri 2022-02-18 18:12 -0800, Luis Chamberlain wrote:
> On Fri, Feb 18, 2022 at 09:24:58PM +0000, Aaron Tomlin wrote:
> > Hi Luis,
> >
> > As per your suggestion [1], this is an attempt to refactor and split
> > optional code out of core module support code into separate components.
> > This version is based on Linus' commit 7993e65fdd0f ("Merge tag
> > 'mtd/fixes-for-5.17-rc5' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux").
> > Please let me know your thoughts.
>
> Fantastic, thanks for doing all this work, I've pushed this out to
> modules-next so that the testing can start as this will be in linux-next
> soon. I'll obviously wait for more reviews, we have a long time before this
> gets merged to Linus, so just want to start getting testing done now rather
> than later. And other folks are depending on your changes to start
> getting their own code up too.

Hi Luis,

No problem. Unfortunately, this was not the complete series. I can send a
v7 later today with the suggested improvements made by Christophe.


Kind regards,

-- 
Aaron Tomlin


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 01/13] module: Move all into module/
  2022-02-22 10:48     ` Aaron Tomlin
@ 2022-02-23  1:16       ` Luis Chamberlain
  2022-02-23 16:57         ` Aaron Tomlin
  0 siblings, 1 reply; 32+ messages in thread
From: Luis Chamberlain @ 2022-02-23  1:16 UTC (permalink / raw)
  To: Aaron Tomlin, Thiago Jung Bauermann, Mimi Zohar, Heiko Carstens,
	Philipp Rudo
  Cc: Christophe Leroy, cl, pmladek, mbenes, akpm, jeyu, linux-kernel,
	linux-modules, live-patching, atomlin, ghalat, allen.lkml, joe,
	msuchanek, oleksandr

On Tue, Feb 22, 2022 at 10:48:00AM +0000, Aaron Tomlin wrote:
> On Mon 2022-02-21 12:21 +0000, Christophe Leroy wrote:
> > Le 18/02/2022 à 22:24, Aaron Tomlin a écrit :
> > >   kernel/{module_signature.c => module/signature.c}   | 0
> > Keeping it outside of kernel/module/ would allow to conditionaly build
> > entire kernel/module/ based of CONFIG_MODULES and then avoid all checks
> > against CONFIG_MODULES which look misleading at times.
> 
> Luis,
> 
> What is your opinion on this? Indeed, mod_check_sig() is used by code
> outside of kernel/module/ too i.e. ima_read_modsig(); albeit, I believe it
> does make sense to keep it under kernel/module/ since the function in
> question is used to review a given module's signature anyway.

How about:

obj-$(CONFIG_MODULE_SIG_FORMAT) += module/module_signature.o 

  Luis

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v6 01/13] module: Move all into module/
  2022-02-23  1:16       ` Luis Chamberlain
@ 2022-02-23 16:57         ` Aaron Tomlin
  0 siblings, 0 replies; 32+ messages in thread
From: Aaron Tomlin @ 2022-02-23 16:57 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Thiago Jung Bauermann, Mimi Zohar, Heiko Carstens, Philipp Rudo,
	Christophe Leroy, cl, pmladek, mbenes, akpm, jeyu, linux-kernel,
	linux-modules, live-patching, atomlin, ghalat, allen.lkml, joe,
	msuchanek, oleksandr

On Tue 2022-02-22 17:16 -0800, Luis Chamberlain wrote:
> How about:
>
> obj-$(CONFIG_MODULE_SIG_FORMAT) += module/module_signature.o
>
>   Luis

Hi Luis,

Please see v8 [1].

[1]: https://lore.kernel.org/all/20220222141303.1392190-1-atomlin@redhat.com/


Kind regards,

-- 
Aaron Tomlin


^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2022-02-23 16:57 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-18 21:24 [PATCH v6 00/13] module: core code clean up Aaron Tomlin
2022-02-18 21:24 ` [PATCH v6 01/13] module: Move all into module/ Aaron Tomlin
2022-02-21 12:21   ` Christophe Leroy
2022-02-22 10:48     ` Aaron Tomlin
2022-02-23  1:16       ` Luis Chamberlain
2022-02-23 16:57         ` Aaron Tomlin
2022-02-21 13:13   ` Christophe Leroy
2022-02-18 21:25 ` [PATCH v6 02/13] module: Simple refactor in preparation for split Aaron Tomlin
2022-02-18 21:25 ` [PATCH v6 03/13] module: Make internal.h and decompress.c more compliant Aaron Tomlin
2022-02-18 21:25 ` [PATCH v6 04/13] module: Move livepatch support to a separate file Aaron Tomlin
2022-02-18 21:25 ` [PATCH v6 05/13] module: Move latched RB-tree " Aaron Tomlin
2022-02-21 10:57   ` Christophe Leroy
2022-02-18 21:25 ` [PATCH v6 06/13] module: Move strict rwx " Aaron Tomlin
2022-02-21  6:31   ` Christophe Leroy
2022-02-21  9:35     ` Aaron Tomlin
2022-02-21 11:27   ` Christophe Leroy
2022-02-21 16:15   ` Christophe Leroy
2022-02-18 21:25 ` [PATCH v6 07/13] module: Move extra signature support out of core code Aaron Tomlin
2022-02-18 21:25 ` [PATCH v6 08/13] module: Move kmemleak support to a separate file Aaron Tomlin
2022-02-18 21:25 ` [PATCH v6 09/13] module: Move kallsyms support into " Aaron Tomlin
2022-02-21  8:15   ` Christophe Leroy
2022-02-21  8:35     ` Christophe Leroy
2022-02-21  9:22       ` Aaron Tomlin
2022-02-22  9:58         ` Miroslav Benes
2022-02-21  9:21     ` Aaron Tomlin
2022-02-21 10:49   ` Christophe Leroy
2022-02-21 12:02     ` Aaron Tomlin
2022-02-18 21:25 ` [PATCH v6 10/13] module: Move procfs " Aaron Tomlin
2022-02-19  2:12 ` [PATCH v6 00/13] module: core code clean up Luis Chamberlain
2022-02-21 12:47   ` Miroslav Benes
2022-02-22 10:58   ` Christophe Leroy
2022-02-22 11:18   ` Aaron Tomlin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).