lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 22 Feb 2022 18:06:39 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Aaron Tomlin <atomlin@...hat.com>,
        "mcgrof@...nel.org" <mcgrof@...nel.org>
CC:     "cl@...ux.com" <cl@...ux.com>,
        "pmladek@...e.com" <pmladek@...e.com>,
        "mbenes@...e.cz" <mbenes@...e.cz>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "jeyu@...nel.org" <jeyu@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
        "void@...ifault.com" <void@...ifault.com>,
        "atomlin@...mlin.com" <atomlin@...mlin.com>,
        "allen.lkml@...il.com" <allen.lkml@...il.com>,
        "joe@...ches.com" <joe@...ches.com>,
        "msuchanek@...e.de" <msuchanek@...e.de>,
        "oleksandr@...alenko.name" <oleksandr@...alenko.name>
Subject: Re: [PATCH v8 13/13] module: Move version support into a separate
 file



Le 22/02/2022 à 15:13, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates module version support out of core code into
> kernel/module/version.c. In addition simple code refactoring to
> make this possible.
> 
> Signed-off-by: Aaron Tomlin <atomlin@...hat.com>

Reviewed-by: Christophe Leroy <christophe.leroy@...roup.eu>

> ---
>   kernel/module/Makefile   |   1 +
>   kernel/module/internal.h |  48 ++++++++++++
>   kernel/module/main.c     | 156 ++-------------------------------------
>   kernel/module/version.c  | 109 +++++++++++++++++++++++++++
>   4 files changed, 166 insertions(+), 148 deletions(-)
>   create mode 100644 kernel/module/version.c
> 
> diff --git a/kernel/module/Makefile b/kernel/module/Makefile
> index cf8dcdc6b55f..a46e6361017f 100644
> --- a/kernel/module/Makefile
> +++ b/kernel/module/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_DEBUG_KMEMLEAK) += debug_kmemleak.o
>   obj-$(CONFIG_KALLSYMS) += kallsyms.o
>   obj-$(CONFIG_PROC_FS) += procfs.o
>   obj-$(CONFIG_SYSFS) += sysfs.o
> +obj-$(CONFIG_MODVERSIONS) += version.o
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 62d749ef695e..3fc139d5074b 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -70,7 +70,27 @@ struct load_info {
>   	} index;
>   };
>   
> +enum mod_license {
> +	NOT_GPL_ONLY,
> +	GPL_ONLY,
> +};
> +
> +struct find_symbol_arg {
> +	/* Input */
> +	const char *name;
> +	bool gplok;
> +	bool warn;
> +
> +	/* Output */
> +	struct module *owner;
> +	const s32 *crc;
> +	const struct kernel_symbol *sym;
> +	enum mod_license license;
> +};
> +
>   int mod_verify_sig(const void *mod, struct load_info *info);
> +int try_to_force_load(struct module *mod, const char *reason);
> +bool find_symbol(struct find_symbol_arg *fsa);
>   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,
> @@ -225,3 +245,31 @@ static inline int mod_sysfs_setup(struct module *mod,
>   static inline void mod_sysfs_teardown(struct module *mod) { }
>   static inline void init_param_lock(struct module *mod) { }
>   #endif /* CONFIG_SYSFS */
> +
> +#ifdef CONFIG_MODVERSIONS
> +int check_version(const struct load_info *info,
> +		  const char *symname, struct module *mod, const s32 *crc);
> +void module_layout(struct module *mod, struct modversion_info *ver, struct kernel_param *kp,
> +		   struct kernel_symbol *ks, struct tracepoint * const *tp);
> +int check_modstruct_version(const struct load_info *info, struct module *mod);
> +int same_magic(const char *amagic, const char *bmagic, bool has_crcs);
> +#else /* !CONFIG_MODVERSIONS */
> +static inline int check_version(const struct load_info *info,
> +				const char *symname,
> +				struct module *mod,
> +				const s32 *crc)
> +{
> +	return 1;
> +}
> +
> +static inline int check_modstruct_version(const struct load_info *info,
> +					  struct module *mod)
> +{
> +	return 1;
> +}
> +
> +static inline int same_magic(const char *amagic, const char *bmagic, bool has_crcs)
> +{
> +	return strcmp(amagic, bmagic) == 0;
> +}
> +#endif /* CONFIG_MODVERSIONS */
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index bcc4f7a82649..0749afdc34b5 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -86,6 +86,12 @@ struct mod_tree_root mod_tree __cacheline_aligned = {
>   static unsigned long module_addr_min = -1UL, module_addr_max;
>   #endif /* CONFIG_MODULES_TREE_LOOKUP */
>   
> +struct symsearch {
> +	const struct kernel_symbol *start, *stop;
> +	const s32 *crcs;
> +	enum mod_license license;
> +};
> +
>   /*
>    * Bounds of module text, for speeding up __module_address.
>    * Protected by module_mutex.
> @@ -244,28 +250,6 @@ static __maybe_unused void *any_section_objs(const struct load_info *info,
>   #define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
>   #endif
>   
> -struct symsearch {
> -	const struct kernel_symbol *start, *stop;
> -	const s32 *crcs;
> -	enum mod_license {
> -		NOT_GPL_ONLY,
> -		GPL_ONLY,
> -	} license;
> -};
> -
> -struct find_symbol_arg {
> -	/* Input */
> -	const char *name;
> -	bool gplok;
> -	bool warn;
> -
> -	/* Output */
> -	struct module *owner;
> -	const s32 *crc;
> -	const struct kernel_symbol *sym;
> -	enum mod_license license;
> -};
> -
>   static bool check_exported_symbol(const struct symsearch *syms,
>   				  struct module *owner,
>   				  unsigned int symnum, void *data)
> @@ -327,7 +311,7 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
>    * Find an exported symbol and return it, along with, (optional) crc and
>    * (optional) module which owns it.  Needs preempt disabled or module_mutex.
>    */
> -static bool find_symbol(struct find_symbol_arg *fsa)
> +bool find_symbol(struct find_symbol_arg *fsa)
>   {
>   	static const struct symsearch arr[] = {
>   		{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
> @@ -1001,7 +985,7 @@ size_t modinfo_attrs_count = ARRAY_SIZE(modinfo_attrs);
>   
>   static const char vermagic[] = VERMAGIC_STRING;
>   
> -static int try_to_force_load(struct module *mod, const char *reason)
> +int try_to_force_load(struct module *mod, const char *reason)
>   {
>   #ifdef CONFIG_MODULE_FORCE_LOAD
>   	if (!test_taint(TAINT_FORCED_MODULE))
> @@ -1013,115 +997,6 @@ static int try_to_force_load(struct module *mod, const char *reason)
>   #endif
>   }
>   
> -#ifdef CONFIG_MODVERSIONS
> -
> -static u32 resolve_rel_crc(const s32 *crc)
> -{
> -	return *(u32 *)((void *)crc + *crc);
> -}
> -
> -static int check_version(const struct load_info *info,
> -			 const char *symname,
> -			 struct module *mod,
> -			 const s32 *crc)
> -{
> -	Elf_Shdr *sechdrs = info->sechdrs;
> -	unsigned int versindex = info->index.vers;
> -	unsigned int i, num_versions;
> -	struct modversion_info *versions;
> -
> -	/* Exporting module didn't supply crcs?  OK, we're already tainted. */
> -	if (!crc)
> -		return 1;
> -
> -	/* No versions at all?  modprobe --force does this. */
> -	if (versindex == 0)
> -		return try_to_force_load(mod, symname) == 0;
> -
> -	versions = (void *) sechdrs[versindex].sh_addr;
> -	num_versions = sechdrs[versindex].sh_size
> -		/ sizeof(struct modversion_info);
> -
> -	for (i = 0; i < num_versions; i++) {
> -		u32 crcval;
> -
> -		if (strcmp(versions[i].name, symname) != 0)
> -			continue;
> -
> -		if (IS_ENABLED(CONFIG_MODULE_REL_CRCS))
> -			crcval = resolve_rel_crc(crc);
> -		else
> -			crcval = *crc;
> -		if (versions[i].crc == crcval)
> -			return 1;
> -		pr_debug("Found checksum %X vs module %lX\n",
> -			 crcval, versions[i].crc);
> -		goto bad_version;
> -	}
> -
> -	/* Broken toolchain. Warn once, then let it go.. */
> -	pr_warn_once("%s: no symbol version for %s\n", info->name, symname);
> -	return 1;
> -
> -bad_version:
> -	pr_warn("%s: disagrees about version of symbol %s\n",
> -	       info->name, symname);
> -	return 0;
> -}
> -
> -static inline int check_modstruct_version(const struct load_info *info,
> -					  struct module *mod)
> -{
> -	struct find_symbol_arg fsa = {
> -		.name	= "module_layout",
> -		.gplok	= true,
> -	};
> -
> -	/*
> -	 * Since this should be found in kernel (which can't be removed), no
> -	 * locking is necessary -- use preempt_disable() to placate lockdep.
> -	 */
> -	preempt_disable();
> -	if (!find_symbol(&fsa)) {
> -		preempt_enable();
> -		BUG();
> -	}
> -	preempt_enable();
> -	return check_version(info, "module_layout", mod, fsa.crc);
> -}
> -
> -/* First part is kernel version, which we ignore if module has crcs. */
> -static inline int same_magic(const char *amagic, const char *bmagic,
> -			     bool has_crcs)
> -{
> -	if (has_crcs) {
> -		amagic += strcspn(amagic, " ");
> -		bmagic += strcspn(bmagic, " ");
> -	}
> -	return strcmp(amagic, bmagic) == 0;
> -}
> -#else
> -static inline int check_version(const struct load_info *info,
> -				const char *symname,
> -				struct module *mod,
> -				const s32 *crc)
> -{
> -	return 1;
> -}
> -
> -static inline int check_modstruct_version(const struct load_info *info,
> -					  struct module *mod)
> -{
> -	return 1;
> -}
> -
> -static inline int same_magic(const char *amagic, const char *bmagic,
> -			     bool has_crcs)
> -{
> -	return strcmp(amagic, bmagic) == 0;
> -}
> -#endif /* CONFIG_MODVERSIONS */
> -
>   static char *get_modinfo(const struct load_info *info, const char *tag);
>   static char *get_next_modinfo(const struct load_info *info, const char *tag,
>   			      char *prev);
> @@ -3247,18 +3122,3 @@ void print_modules(void)
>   		pr_cont(" [last unloaded: %s]", last_unloaded_module);
>   	pr_cont("\n");
>   }
> -
> -#ifdef CONFIG_MODVERSIONS
> -/*
> - * Generate the signature for all relevant module structures here.
> - * If these change, we don't want to try to parse the module.
> - */
> -void module_layout(struct module *mod,
> -		   struct modversion_info *ver,
> -		   struct kernel_param *kp,
> -		   struct kernel_symbol *ks,
> -		   struct tracepoint * const *tp)
> -{
> -}
> -EXPORT_SYMBOL(module_layout);
> -#endif
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> new file mode 100644
> index 000000000000..adaedce1dc97
> --- /dev/null
> +++ b/kernel/module/version.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Module version support
> + *
> + * Copyright (C) 2008 Rusty Russell
> + */
> +
> +#include <linux/module.h>
> +#include <linux/string.h>
> +#include <linux/printk.h>
> +#include "internal.h"
> +
> +static u32 resolve_rel_crc(const s32 *crc)
> +{
> +	return *(u32 *)((void *)crc + *crc);
> +}
> +
> +int check_version(const struct load_info *info,
> +		  const char *symname,
> +			 struct module *mod,
> +			 const s32 *crc)
> +{
> +	Elf_Shdr *sechdrs = info->sechdrs;
> +	unsigned int versindex = info->index.vers;
> +	unsigned int i, num_versions;
> +	struct modversion_info *versions;
> +
> +	/* Exporting module didn't supply crcs?  OK, we're already tainted. */
> +	if (!crc)
> +		return 1;
> +
> +	/* No versions at all?  modprobe --force does this. */
> +	if (versindex == 0)
> +		return try_to_force_load(mod, symname) == 0;
> +
> +	versions = (void *)sechdrs[versindex].sh_addr;
> +	num_versions = sechdrs[versindex].sh_size
> +		/ sizeof(struct modversion_info);
> +
> +	for (i = 0; i < num_versions; i++) {
> +		u32 crcval;
> +
> +		if (strcmp(versions[i].name, symname) != 0)
> +			continue;
> +
> +		if (IS_ENABLED(CONFIG_MODULE_REL_CRCS))
> +			crcval = resolve_rel_crc(crc);
> +		else
> +			crcval = *crc;
> +		if (versions[i].crc == crcval)
> +			return 1;
> +		pr_debug("Found checksum %X vs module %lX\n",
> +			 crcval, versions[i].crc);
> +		goto bad_version;
> +	}
> +
> +	/* Broken toolchain. Warn once, then let it go.. */
> +	pr_warn_once("%s: no symbol version for %s\n", info->name, symname);
> +	return 1;
> +
> +bad_version:
> +	pr_warn("%s: disagrees about version of symbol %s\n", info->name, symname);
> +	return 0;
> +}
> +
> +int check_modstruct_version(const struct load_info *info,
> +			    struct module *mod)
> +{
> +	struct find_symbol_arg fsa = {
> +		.name	= "module_layout",
> +		.gplok	= true,
> +	};
> +
> +	/*
> +	 * Since this should be found in kernel (which can't be removed), no
> +	 * locking is necessary -- use preempt_disable() to placate lockdep.
> +	 */
> +	preempt_disable();
> +	if (!find_symbol(&fsa)) {
> +		preempt_enable();
> +		BUG();
> +	}
> +	preempt_enable();
> +	return check_version(info, "module_layout", mod, fsa.crc);
> +}
> +
> +/* First part is kernel version, which we ignore if module has crcs. */
> +int same_magic(const char *amagic, const char *bmagic,
> +	       bool has_crcs)
> +{
> +	if (has_crcs) {
> +		amagic += strcspn(amagic, " ");
> +		bmagic += strcspn(bmagic, " ");
> +	}
> +	return strcmp(amagic, bmagic) == 0;
> +}
> +
> +/*
> + * Generate the signature for all relevant module structures here.
> + * If these change, we don't want to try to parse the module.
> + */
> +void module_layout(struct module *mod,
> +		   struct modversion_info *ver,
> +		   struct kernel_param *kp,
> +		   struct kernel_symbol *ks,
> +		   struct tracepoint * const *tp)
> +{
> +}
> +EXPORT_SYMBOL(module_layout);

Powered by blists - more mailing lists