Re: [PATCH v8 13/13] module: Move version support into a separate file

From: Christophe Leroy
Date: Tue Feb 22 2022 - 13:06:49 EST

Next message: Rahul T R: "[PATCH] arm64: defconfig: Enable configs for DisplayPort on J721e"
Previous message: Christophe Leroy: "Re: [PATCH v8 12/13] module: Move kdb_modules list out of core code"
In reply to: Aaron Tomlin: "[PATCH v8 13/13] module: Move version support into a separate file"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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

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

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

Next message: Rahul T R: "[PATCH] arm64: defconfig: Enable configs for DisplayPort on J721e"
Previous message: Christophe Leroy: "Re: [PATCH v8 12/13] module: Move kdb_modules list out of core code"
In reply to: Aaron Tomlin: "[PATCH v8 13/13] module: Move version support into a separate file"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]