diff mbox series

[v6,06/13] module: Move strict rwx support to a separate file

Message ID 20220218212511.887059-7-atomlin@redhat.com (mailing list archive)
State Superseded
Headers show
Series module: core code clean up | expand

Commit Message

Aaron Tomlin Feb. 18, 2022, 9:25 p.m. UTC 
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

Comments

Christophe Leroy Feb. 21, 2022, 6:31 a.m. UTC | #1 
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
Aaron Tomlin Feb. 21, 2022, 9:35 a.m. UTC | #2 
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,
Christophe Leroy Feb. 21, 2022, 11:27 a.m. UTC | #3 
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
Christophe Leroy Feb. 21, 2022, 4:15 p.m. UTC | #4 
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
diff mbox series

Patch

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