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

[Christophe Leroy <christophe.leroy@csgroup.eu>]

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