From 3675a926feefdf3afabea12f806f31ea582065e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Sat, 28 Dec 2024 09:43:41 +0100 Subject: sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most users use this function through the BIN_ATTR_SIMPLE* macros, they can handle the switch transparently. Also adapt the two non-macro users in the same change. Signed-off-by: Thomas Weißschuh Acked-by: Madhavan Srinivasan Reviewed-by: Mahesh Salgaonkar Tested-by: Aditya Gupta Link: https://lore.kernel.org/r/20241228-sysfs-const-bin_attr-simple-v2-1-7c6f3f1767a3@weissschuh.net Signed-off-by: Greg Kroah-Hartman --- kernel/module/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 456358e1fdc4..254017b58b64 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -196,7 +196,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) nattr->attr.mode = 0444; nattr->size = info->sechdrs[i].sh_size; nattr->private = (void *)info->sechdrs[i].sh_addr; - nattr->read = sysfs_bin_attr_simple_read; + nattr->read_new = sysfs_bin_attr_simple_read; ++nattr; } ++loaded; -- cgit v1.2.3 From 1cd9502ee9275c6176a7312863f939cca9506114 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 29 Dec 2024 00:45:28 +0900 Subject: module: get symbol CRC back to unsigned Commit 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities") changed the CRC fields to s32 because the __kcrctab and __kcrctab_gpl sections contained relative references to the actual CRC values stored in the .rodata section when CONFIG_MODULE_REL_CRCS=y. Commit 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS") removed this complexity. Now, the __kcrctab and __kcrctab_gpl sections directly contain the CRC values in all cases. The genksyms tool outputs unsigned 32-bit CRC values, so u32 is preferred over s32. No functional changes are intended. Regardless of this change, the CRC value is assigned to the u32 variable 'crcval' before the comparison, as seen in kernel/module/version.c: crcval = *crc; It was previously mandatory (but now optional) in order to avoid sign extension because the following line previously compared 'unsigned long' and 's32': if (versions[i].crc == crcval) return 1; versions[i].crc is still 'unsigned long' for backward compatibility. Signed-off-by: Masahiro Yamada Reviewed-by: Petr Pavlu --- kernel/module/internal.h | 10 +++++----- kernel/module/main.c | 2 +- kernel/module/version.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/internal.h b/kernel/module/internal.h index daef2be83902..f10dc3ea7ff8 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -55,8 +55,8 @@ 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[]; +extern const u32 __start___kcrctab[]; +extern const u32 __start___kcrctab_gpl[]; struct load_info { const char *name; @@ -102,7 +102,7 @@ struct find_symbol_arg { /* Output */ struct module *owner; - const s32 *crc; + const u32 *crc; const struct kernel_symbol *sym; enum mod_license license; }; @@ -384,7 +384,7 @@ static inline void init_param_lock(struct module *mod) { } #ifdef CONFIG_MODVERSIONS int check_version(const struct load_info *info, - const char *symname, struct module *mod, const s32 *crc); + const char *symname, struct module *mod, const u32 *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); @@ -393,7 +393,7 @@ int same_magic(const char *amagic, const char *bmagic, bool has_crcs); static inline int check_version(const struct load_info *info, const char *symname, struct module *mod, - const s32 *crc) + const u32 *crc) { return 1; } diff --git a/kernel/module/main.c b/kernel/module/main.c index 5399c182b3cb..e58bff88b8d6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -86,7 +86,7 @@ struct mod_tree_root mod_tree __cacheline_aligned = { struct symsearch { const struct kernel_symbol *start, *stop; - const s32 *crcs; + const u32 *crcs; enum mod_license license; }; diff --git a/kernel/module/version.c b/kernel/module/version.c index 53f43ac5a73e..4e5731d403af 100644 --- a/kernel/module/version.c +++ b/kernel/module/version.c @@ -13,7 +13,7 @@ int check_version(const struct load_info *info, const char *symname, struct module *mod, - const s32 *crc) + const u32 *crc) { Elf_Shdr *sechdrs = info->sechdrs; unsigned int versindex = info->index.vers; -- cgit v1.2.3 From f28568841ae0a0dd48dfc5400aaebedf10a54d10 Mon Sep 17 00:00:00 2001 From: Sami Tolvanen Date: Fri, 3 Jan 2025 20:45:23 +0000 Subject: tools: Add gendwarfksyms Add a basic DWARF parser, which uses libdw to traverse the debugging information in an object file and looks for functions and variables. In follow-up patches, this will be expanded to produce symbol versions for CONFIG_MODVERSIONS from DWARF. Signed-off-by: Sami Tolvanen Reviewed-by: Petr Pavlu Signed-off-by: Masahiro Yamada --- kernel/module/Kconfig | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'kernel/module') diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index 7b329057997a..4637f063d0fc 100644 --- a/kernel/module/Kconfig +++ b/kernel/module/Kconfig @@ -169,6 +169,14 @@ config MODVERSIONS make them incompatible with the kernel you are running. If unsure, say N. +config GENDWARFKSYMS + bool "gendwarfksyms (from debugging information)" + depends on DEBUG_INFO + # Requires full debugging information, split DWARF not supported. + depends on !DEBUG_INFO_REDUCED && !DEBUG_INFO_SPLIT + # Requires ELF object files. + depends on !LTO + config ASM_MODVERSIONS bool default HAVE_ASM_MODVERSIONS && MODVERSIONS -- cgit v1.2.3 From 9c3681f9b9fd12cdbc4a542df599f1837512f3d5 Mon Sep 17 00:00:00 2001 From: Sami Tolvanen Date: Fri, 3 Jan 2025 20:45:39 +0000 Subject: kbuild: Add gendwarfksyms as an alternative to genksyms When MODVERSIONS is enabled, allow selecting gendwarfksyms as the implementation, but default to genksyms. Signed-off-by: Sami Tolvanen Signed-off-by: Masahiro Yamada --- kernel/module/Kconfig | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'kernel/module') diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index 4637f063d0fc..d443fc504ffc 100644 --- a/kernel/module/Kconfig +++ b/kernel/module/Kconfig @@ -169,6 +169,22 @@ config MODVERSIONS make them incompatible with the kernel you are running. If unsure, say N. +choice + prompt "Module versioning implementation" + depends on MODVERSIONS + help + Select the tool used to calculate symbol versions for modules. + + If unsure, select GENKSYMS. + +config GENKSYMS + bool "genksyms (from source code)" + help + Calculate symbol versions from pre-processed source code using + genksyms. + + If unsure, say Y. + config GENDWARFKSYMS bool "gendwarfksyms (from debugging information)" depends on DEBUG_INFO @@ -176,6 +192,12 @@ config GENDWARFKSYMS depends on !DEBUG_INFO_REDUCED && !DEBUG_INFO_SPLIT # Requires ELF object files. depends on !LTO + help + Calculate symbol versions from DWARF debugging information using + gendwarfksyms. Requires DEBUG_INFO to be enabled. + + If unsure, say N. +endchoice config ASM_MODVERSIONS bool -- cgit v1.2.3 From 54ac1ac8edeb74ff87fc880d1ee58785bdcbe323 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 3 Jan 2025 17:37:01 +0000 Subject: modules: Support extended MODVERSIONS info Adds a new format for MODVERSIONS which stores each field in a separate ELF section. This initially adds support for variable length names, but could later be used to add additional fields to MODVERSIONS in a backwards compatible way if needed. Any new fields will be ignored by old user tooling, unlike the current format where user tooling cannot tolerate adjustments to the format (for example making the name field longer). Since PPC munges its version records to strip leading dots, we reproduce the munging for the new format. Other architectures do not appear to have architecture-specific usage of this information. Reviewed-by: Sami Tolvanen Signed-off-by: Matthew Maurer Signed-off-by: Masahiro Yamada --- kernel/module/internal.h | 11 ++++++ kernel/module/main.c | 92 +++++++++++++++++++++++++++++++++++++++++++----- kernel/module/version.c | 45 +++++++++++++++++++++++ 3 files changed, 140 insertions(+), 8 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/internal.h b/kernel/module/internal.h index f10dc3ea7ff8..887838589020 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -86,6 +86,8 @@ struct load_info { unsigned int vers; unsigned int info; unsigned int pcpu; + unsigned int vers_ext_crc; + unsigned int vers_ext_name; } index; }; @@ -389,6 +391,15 @@ void module_layout(struct module *mod, struct modversion_info *ver, struct kerne 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); +struct modversion_info_ext { + size_t remaining; + const u32 *crc; + const char *name; +}; +void modversion_ext_start(const struct load_info *info, struct modversion_info_ext *ver); +void modversion_ext_advance(struct modversion_info_ext *ver); +#define for_each_modversion_info_ext(ver, info) \ + for (modversion_ext_start(info, &ver); ver.remaining > 0; modversion_ext_advance(&ver)) #else /* !CONFIG_MODVERSIONS */ static inline int check_version(const struct load_info *info, const char *symname, diff --git a/kernel/module/main.c b/kernel/module/main.c index e58bff88b8d6..0830892070fe 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2073,6 +2073,82 @@ static int elf_validity_cache_index_str(struct load_info *info) return 0; } +/** + * elf_validity_cache_index_versions() - Validate and cache version indices + * @info: Load info to cache version indices in. + * Must have &load_info->sechdrs and &load_info->secstrings populated. + * @flags: Load flags, relevant to suppress version loading, see + * uapi/linux/module.h + * + * If we're ignoring modversions based on @flags, zero all version indices + * and return validity. Othewrise check: + * + * * If "__version_ext_crcs" is present, "__version_ext_names" is present + * * There is a name present for every crc + * + * Then populate: + * + * * &load_info->index.vers + * * &load_info->index.vers_ext_crc + * * &load_info->index.vers_ext_names + * + * if present. + * + * Return: %0 if valid, %-ENOEXEC on failure. + */ +static int elf_validity_cache_index_versions(struct load_info *info, int flags) +{ + unsigned int vers_ext_crc; + unsigned int vers_ext_name; + size_t crc_count; + size_t remaining_len; + size_t name_size; + char *name; + + /* If modversions were suppressed, pretend we didn't find any */ + if (flags & MODULE_INIT_IGNORE_MODVERSIONS) { + info->index.vers = 0; + info->index.vers_ext_crc = 0; + info->index.vers_ext_name = 0; + return 0; + } + + vers_ext_crc = find_sec(info, "__version_ext_crcs"); + vers_ext_name = find_sec(info, "__version_ext_names"); + + /* If we have one field, we must have the other */ + if (!!vers_ext_crc != !!vers_ext_name) { + pr_err("extended version crc+name presence does not match"); + return -ENOEXEC; + } + + /* + * If we have extended version information, we should have the same + * number of entries in every section. + */ + if (vers_ext_crc) { + crc_count = info->sechdrs[vers_ext_crc].sh_size / sizeof(u32); + name = (void *)info->hdr + + info->sechdrs[vers_ext_name].sh_offset; + remaining_len = info->sechdrs[vers_ext_name].sh_size; + + while (crc_count--) { + name_size = strnlen(name, remaining_len) + 1; + if (name_size > remaining_len) { + pr_err("more extended version crcs than names"); + return -ENOEXEC; + } + remaining_len -= name_size; + name += name_size; + } + } + + info->index.vers = find_sec(info, "__versions"); + info->index.vers_ext_crc = vers_ext_crc; + info->index.vers_ext_name = vers_ext_name; + return 0; +} + /** * elf_validity_cache_index() - Resolve, validate, cache section indices * @info: Load info to read from and update. @@ -2087,9 +2163,7 @@ static int elf_validity_cache_index_str(struct load_info *info) * * elf_validity_cache_index_mod() * * elf_validity_cache_index_sym() * * elf_validity_cache_index_str() - * - * If versioning is not suppressed via flags, load the version index from - * a section called "__versions" with no validation. + * * elf_validity_cache_index_versions() * * If CONFIG_SMP is enabled, load the percpu section by name with no * validation. @@ -2112,11 +2186,9 @@ static int elf_validity_cache_index(struct load_info *info, int flags) err = elf_validity_cache_index_str(info); if (err < 0) return err; - - if (flags & MODULE_INIT_IGNORE_MODVERSIONS) - info->index.vers = 0; /* Pretend no __versions section! */ - else - info->index.vers = find_sec(info, "__versions"); + err = elf_validity_cache_index_versions(info, flags); + if (err < 0) + return err; info->index.pcpu = find_pcpusec(info); @@ -2327,6 +2399,10 @@ static int rewrite_section_headers(struct load_info *info, int flags) /* Track but don't keep modinfo and version sections. */ info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC; + info->sechdrs[info->index.vers_ext_crc].sh_flags &= + ~(unsigned long)SHF_ALLOC; + info->sechdrs[info->index.vers_ext_name].sh_flags &= + ~(unsigned long)SHF_ALLOC; info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC; return 0; diff --git a/kernel/module/version.c b/kernel/module/version.c index 4e5731d403af..3718a8868321 100644 --- a/kernel/module/version.c +++ b/kernel/module/version.c @@ -19,11 +19,28 @@ int check_version(const struct load_info *info, unsigned int versindex = info->index.vers; unsigned int i, num_versions; struct modversion_info *versions; + struct modversion_info_ext version_ext; /* Exporting module didn't supply crcs? OK, we're already tainted. */ if (!crc) return 1; + /* If we have extended version info, rely on it */ + if (info->index.vers_ext_crc) { + for_each_modversion_info_ext(version_ext, info) { + if (strcmp(version_ext.name, symname) != 0) + continue; + if (*version_ext.crc == *crc) + return 1; + pr_debug("Found checksum %X vs module %X\n", + *crc, *version_ext.crc); + goto bad_version; + } + pr_warn_once("%s: no extended symbol version for %s\n", + info->name, symname); + return 1; + } + /* No versions at all? modprobe --force does this. */ if (versindex == 0) return try_to_force_load(mod, symname) == 0; @@ -87,6 +104,34 @@ int same_magic(const char *amagic, const char *bmagic, return strcmp(amagic, bmagic) == 0; } +void modversion_ext_start(const struct load_info *info, + struct modversion_info_ext *start) +{ + unsigned int crc_idx = info->index.vers_ext_crc; + unsigned int name_idx = info->index.vers_ext_name; + Elf_Shdr *sechdrs = info->sechdrs; + + /* + * Both of these fields are needed for this to be useful + * Any future fields should be initialized to NULL if absent. + */ + if (crc_idx == 0 || name_idx == 0) { + start->remaining = 0; + return; + } + + start->crc = (const u32 *)sechdrs[crc_idx].sh_addr; + start->name = (const char *)sechdrs[name_idx].sh_addr; + start->remaining = sechdrs[crc_idx].sh_size / sizeof(*start->crc); +} + +void modversion_ext_advance(struct modversion_info_ext *vers) +{ + vers->remaining--; + vers->crc++; + vers->name += strlen(vers->name) + 1; +} + /* * Generate the signature for all relevant module structures here. * If these change, we don't want to try to parse the module. -- cgit v1.2.3 From fc7d5e3210ae083a29ce224ffce18eaf3d1c645a Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 3 Jan 2025 17:37:02 +0000 Subject: modpost: Produce extended MODVERSIONS information Generate both the existing modversions format and the new extended one when running modpost. Presence of this metadata in the final .ko is guarded by CONFIG_EXTENDED_MODVERSIONS. We no longer generate an error on long symbols in modpost if CONFIG_EXTENDED_MODVERSIONS is set, as they can now be appropriately encoded in the extended section. These symbols will be skipped in the previous encoding. An error will still be generated if CONFIG_EXTENDED_MODVERSIONS is not set. Reviewed-by: Sami Tolvanen Signed-off-by: Matthew Maurer Signed-off-by: Masahiro Yamada --- kernel/module/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'kernel/module') diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index d443fc504ffc..9568b629a03c 100644 --- a/kernel/module/Kconfig +++ b/kernel/module/Kconfig @@ -207,6 +207,16 @@ config ASM_MODVERSIONS assembly. This can be enabled only when the target architecture supports it. +config EXTENDED_MODVERSIONS + bool "Extended Module Versioning Support" + depends on MODVERSIONS + help + This enables extended MODVERSIONs support, allowing long symbol + names to be versioned. + + The most likely reason you would enable this is to enable Rust + support. If unsure, say N. + config MODULE_SRCVERSION_ALL bool "Source checksum for all modules" help -- cgit v1.2.3 From e8639b7ef0f871753b4262ec0eacd3da29eebcee Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 3 Jan 2025 17:37:03 +0000 Subject: modpost: Allow extended modversions without basic MODVERSIONS If you know that your kernel modules will only ever be loaded by a newer kernel, you can disable BASIC_MODVERSIONS to save space. This also allows easy creation of test modules to see how tooling will respond to modules that only have the new format. Signed-off-by: Matthew Maurer Signed-off-by: Masahiro Yamada --- kernel/module/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'kernel/module') diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index 9568b629a03c..4538f3af63e1 100644 --- a/kernel/module/Kconfig +++ b/kernel/module/Kconfig @@ -217,6 +217,21 @@ config EXTENDED_MODVERSIONS The most likely reason you would enable this is to enable Rust support. If unsure, say N. +config BASIC_MODVERSIONS + bool "Basic Module Versioning Support" + depends on MODVERSIONS + default y + help + This enables basic MODVERSIONS support, allowing older tools or + kernels to potentially load modules. + + Disabling this may cause older `modprobe` or `kmod` to be unable + to read MODVERSIONS information from built modules. With this + disabled, older kernels may treat this module as unversioned. + + This is enabled by default when MODVERSIONS are enabled. + If unsure, say Y. + config MODULE_SRCVERSION_ALL bool "Source checksum for all modules" help -- cgit v1.2.3 From c8e0bd579ed35355ea7e3e3190556c9738870ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 15 Nov 2024 19:50:30 +0100 Subject: module: Put known GPL offenders in an array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of repeating the add_taint_module() call for each offender, create an array and loop over that one. This simplifies adding new entries considerably. Signed-off-by: Uwe Kleine-König Reviewed-by: Greg Kroah-Hartman Reviewed-by: Christoph Hellwig Signed-off-by: Werner Sembach Link: https://lore.kernel.org/r/20241115185253.1299264-2-wse@tuxedocomputers.com [ppavlu: make the array const] Signed-off-by: Petr Pavlu --- kernel/module/main.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/main.c b/kernel/module/main.c index 5399c182b3cb..fd483d436e43 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2332,11 +2332,20 @@ static int rewrite_section_headers(struct load_info *info, int flags) return 0; } +static const char *const module_license_offenders[] = { + /* driverloader was caught wrongly pretending to be under GPL */ + "driverloader", + + /* lve claims to be GPL but upstream won't provide source */ + "lve", +}; + /* * These calls taint the kernel depending certain module circumstances */ static void module_augment_kernel_taints(struct module *mod, struct load_info *info) { int prev_taint = test_taint(TAINT_PROPRIETARY_MODULE); + size_t i; if (!get_modinfo(info, "intree")) { if (!test_taint(TAINT_OOT_MODULE)) @@ -2385,15 +2394,11 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i if (strcmp(mod->name, "ndiswrapper") == 0) add_taint(TAINT_PROPRIETARY_MODULE, LOCKDEP_NOW_UNRELIABLE); - /* driverloader was caught wrongly pretending to be under GPL */ - if (strcmp(mod->name, "driverloader") == 0) - add_taint_module(mod, TAINT_PROPRIETARY_MODULE, - LOCKDEP_NOW_UNRELIABLE); - - /* lve claims to be GPL but upstream won't provide source */ - if (strcmp(mod->name, "lve") == 0) - add_taint_module(mod, TAINT_PROPRIETARY_MODULE, - LOCKDEP_NOW_UNRELIABLE); + for (i = 0; i < ARRAY_SIZE(module_license_offenders); ++i) { + if (strcmp(mod->name, module_license_offenders[i]) == 0) + add_taint_module(mod, TAINT_PROPRIETARY_MODULE, + LOCKDEP_NOW_UNRELIABLE); + } if (!prev_taint && test_taint(TAINT_PROPRIETARY_MODULE)) pr_warn("%s: module license taints kernel.\n", mod->name); -- cgit v1.2.3 From f3227ffda07470848abe3cfa2039b21816ce3090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 16 Dec 2024 18:25:10 +0100 Subject: module: Constify 'struct module_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These structs are never modified, move them to read-only memory. This makes the API clearer and also prepares for the constification of 'struct attribute' itself. While at it, also constify 'modinfo_attrs_count'. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Link: https://lore.kernel.org/r/20241216-sysfs-const-attr-module-v1-3-3790b53e0abf@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/internal.h | 4 ++-- kernel/module/main.c | 40 ++++++++++++++++++++-------------------- kernel/module/sysfs.c | 4 ++-- 3 files changed, 24 insertions(+), 24 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/internal.h b/kernel/module/internal.h index daef2be83902..ac73da5f15bc 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -47,8 +47,8 @@ struct kernel_symbol { extern struct mutex module_mutex; extern struct list_head modules; -extern struct module_attribute *modinfo_attrs[]; -extern size_t modinfo_attrs_count; +extern const struct module_attribute *const modinfo_attrs[]; +extern const size_t modinfo_attrs_count; /* Provided by the linker */ extern const struct kernel_symbol __start___ksymtab[]; diff --git a/kernel/module/main.c b/kernel/module/main.c index fd483d436e43..8154e1665cdb 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -538,7 +538,7 @@ static void setup_modinfo_##field(struct module *mod, const char *s) \ { \ mod->field = kstrdup(s, GFP_KERNEL); \ } \ -static ssize_t show_modinfo_##field(struct module_attribute *mattr, \ +static ssize_t show_modinfo_##field(const struct module_attribute *mattr, \ struct module_kobject *mk, char *buffer) \ { \ return scnprintf(buffer, PAGE_SIZE, "%s\n", mk->mod->field); \ @@ -552,7 +552,7 @@ static void free_modinfo_##field(struct module *mod) \ kfree(mod->field); \ mod->field = NULL; \ } \ -static struct module_attribute modinfo_##field = { \ +static const struct module_attribute modinfo_##field = { \ .attr = { .name = __stringify(field), .mode = 0444 }, \ .show = show_modinfo_##field, \ .setup = setup_modinfo_##field, \ @@ -842,13 +842,13 @@ void symbol_put_addr(void *addr) } EXPORT_SYMBOL_GPL(symbol_put_addr); -static ssize_t show_refcnt(struct module_attribute *mattr, +static ssize_t show_refcnt(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { return sprintf(buffer, "%i\n", module_refcount(mk->mod)); } -static struct module_attribute modinfo_refcnt = +static const struct module_attribute modinfo_refcnt = __ATTR(refcnt, 0444, show_refcnt, NULL); void __module_get(struct module *module) @@ -917,7 +917,7 @@ size_t module_flags_taint(unsigned long taints, char *buf) return l; } -static ssize_t show_initstate(struct module_attribute *mattr, +static ssize_t show_initstate(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { const char *state = "unknown"; @@ -938,10 +938,10 @@ static ssize_t show_initstate(struct module_attribute *mattr, return sprintf(buffer, "%s\n", state); } -static struct module_attribute modinfo_initstate = +static const struct module_attribute modinfo_initstate = __ATTR(initstate, 0444, show_initstate, NULL); -static ssize_t store_uevent(struct module_attribute *mattr, +static ssize_t store_uevent(const struct module_attribute *mattr, struct module_kobject *mk, const char *buffer, size_t count) { @@ -951,10 +951,10 @@ static ssize_t store_uevent(struct module_attribute *mattr, return rc ? rc : count; } -struct module_attribute module_uevent = +const struct module_attribute module_uevent = __ATTR(uevent, 0200, NULL, store_uevent); -static ssize_t show_coresize(struct module_attribute *mattr, +static ssize_t show_coresize(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { unsigned int size = mk->mod->mem[MOD_TEXT].size; @@ -966,11 +966,11 @@ static ssize_t show_coresize(struct module_attribute *mattr, return sprintf(buffer, "%u\n", size); } -static struct module_attribute modinfo_coresize = +static const struct module_attribute modinfo_coresize = __ATTR(coresize, 0444, show_coresize, NULL); #ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC -static ssize_t show_datasize(struct module_attribute *mattr, +static ssize_t show_datasize(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { unsigned int size = 0; @@ -980,11 +980,11 @@ static ssize_t show_datasize(struct module_attribute *mattr, return sprintf(buffer, "%u\n", size); } -static struct module_attribute modinfo_datasize = +static const struct module_attribute modinfo_datasize = __ATTR(datasize, 0444, show_datasize, NULL); #endif -static ssize_t show_initsize(struct module_attribute *mattr, +static ssize_t show_initsize(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { unsigned int size = 0; @@ -994,10 +994,10 @@ static ssize_t show_initsize(struct module_attribute *mattr, return sprintf(buffer, "%u\n", size); } -static struct module_attribute modinfo_initsize = +static const struct module_attribute modinfo_initsize = __ATTR(initsize, 0444, show_initsize, NULL); -static ssize_t show_taint(struct module_attribute *mattr, +static ssize_t show_taint(const struct module_attribute *mattr, struct module_kobject *mk, char *buffer) { size_t l; @@ -1007,10 +1007,10 @@ static ssize_t show_taint(struct module_attribute *mattr, return l; } -static struct module_attribute modinfo_taint = +static const struct module_attribute modinfo_taint = __ATTR(taint, 0444, show_taint, NULL); -struct module_attribute *modinfo_attrs[] = { +const struct module_attribute *const modinfo_attrs[] = { &module_uevent, &modinfo_version, &modinfo_srcversion, @@ -1027,7 +1027,7 @@ struct module_attribute *modinfo_attrs[] = { NULL, }; -size_t modinfo_attrs_count = ARRAY_SIZE(modinfo_attrs); +const size_t modinfo_attrs_count = ARRAY_SIZE(modinfo_attrs); static const char vermagic[] = VERMAGIC_STRING; @@ -1681,7 +1681,7 @@ static void module_license_taint_check(struct module *mod, const char *license) static void setup_modinfo(struct module *mod, struct load_info *info) { - struct module_attribute *attr; + const struct module_attribute *attr; int i; for (i = 0; (attr = modinfo_attrs[i]); i++) { @@ -1692,7 +1692,7 @@ static void setup_modinfo(struct module *mod, struct load_info *info) static void free_modinfo(struct module *mod) { - struct module_attribute *attr; + const struct module_attribute *attr; int i; for (i = 0; (attr = modinfo_attrs[i]); i++) { diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 456358e1fdc4..31e7f3055407 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -275,7 +275,7 @@ static int add_usage_links(struct module *mod) static void module_remove_modinfo_attrs(struct module *mod, int end) { - struct module_attribute *attr; + const struct module_attribute *attr; int i; for (i = 0; (attr = &mod->modinfo_attrs[i]); i++) { @@ -293,7 +293,7 @@ static void module_remove_modinfo_attrs(struct module *mod, int end) static int module_add_modinfo_attrs(struct module *mod) { - struct module_attribute *attr; + const struct module_attribute *attr; struct module_attribute *temp_attr; int error = 0; int i; -- cgit v1.2.3 From d8959b947a8dfab1047c6fd5e982808f65717bfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 27 Dec 2024 14:23:20 +0100 Subject: module: sysfs: Drop member 'module_sect_attrs::nsections' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The member is only used to iterate over all attributes in free_sect_attrs(). However the attribute group can already be used for that. Use the group and drop 'nsections'. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20241227-sysfs-const-bin_attr-module-v2-1-e267275f0f37@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/sysfs.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 31e7f3055407..31351591e8e0 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -26,7 +26,6 @@ struct module_sect_attr { struct module_sect_attrs { struct attribute_group grp; - unsigned int nsections; struct module_sect_attr attrs[]; }; @@ -62,10 +61,10 @@ static ssize_t module_sect_read(struct file *file, struct kobject *kobj, static void free_sect_attrs(struct module_sect_attrs *sect_attrs) { - unsigned int section; + struct bin_attribute **bin_attr; - for (section = 0; section < sect_attrs->nsections; section++) - kfree(sect_attrs->attrs[section].battr.attr.name); + for (bin_attr = sect_attrs->grp.bin_attrs; *bin_attr; bin_attr++) + kfree((*bin_attr)->attr.name); kfree(sect_attrs); } @@ -92,7 +91,6 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) sect_attrs->grp.name = "sections"; sect_attrs->grp.bin_attrs = (void *)sect_attrs + size[0]; - sect_attrs->nsections = 0; sattr = §_attrs->attrs[0]; gattr = §_attrs->grp.bin_attrs[0]; for (i = 0; i < info->hdr->e_shnum; i++) { @@ -108,7 +106,6 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) ret = -ENOMEM; goto out; } - sect_attrs->nsections++; sattr->battr.read = module_sect_read; sattr->battr.size = MODULE_SECT_READ_SIZE; sattr->battr.attr.mode = 0400; -- cgit v1.2.3 From 4b2c11e4aaf7e3d7fd9ce8e5995a32ff5e27d74f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 27 Dec 2024 14:23:21 +0100 Subject: module: sysfs: Drop member 'module_sect_attr::address' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'struct bin_attribute' already contains the member 'private' to pass custom data to the attribute handlers. Use that instead of the custom 'address' member. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20241227-sysfs-const-bin_attr-module-v2-2-e267275f0f37@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/sysfs.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 31351591e8e0..6941ecc941d7 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -21,7 +21,6 @@ #ifdef CONFIG_KALLSYMS struct module_sect_attr { struct bin_attribute battr; - unsigned long address; }; struct module_sect_attrs { @@ -34,8 +33,6 @@ static ssize_t module_sect_read(struct file *file, struct kobject *kobj, struct bin_attribute *battr, char *buf, loff_t pos, size_t count) { - struct module_sect_attr *sattr = - container_of(battr, struct module_sect_attr, battr); char bounce[MODULE_SECT_READ_SIZE + 1]; size_t wrote; @@ -52,7 +49,7 @@ static ssize_t module_sect_read(struct file *file, struct kobject *kobj, */ wrote = scnprintf(bounce, sizeof(bounce), "0x%px\n", kallsyms_show_value(file->f_cred) - ? (void *)sattr->address : NULL); + ? battr->private : NULL); count = min(count, wrote); memcpy(buf, bounce, count); @@ -99,7 +96,6 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) if (sect_empty(sec)) continue; sysfs_bin_attr_init(&sattr->battr); - sattr->address = sec->sh_addr; sattr->battr.attr.name = kstrdup(info->secstrings + sec->sh_name, GFP_KERNEL); if (!sattr->battr.attr.name) { @@ -107,6 +103,7 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) goto out; } sattr->battr.read = module_sect_read; + sattr->battr.private = (void *)sec->sh_addr; sattr->battr.size = MODULE_SECT_READ_SIZE; sattr->battr.attr.mode = 0400; *(gattr++) = &(sattr++)->battr; -- cgit v1.2.3 From 34f5ec0f8252e2ba83fa5da05d56b6fdb0ebef59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 27 Dec 2024 14:23:22 +0100 Subject: module: sysfs: Drop 'struct module_sect_attr' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is now an otherwise empty wrapper around a 'struct bin_attribute', not providing any functionality. Remove it. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20241227-sysfs-const-bin_attr-module-v2-3-e267275f0f37@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/sysfs.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 6941ecc941d7..89797c556e8c 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -19,13 +19,9 @@ * J. Corbet */ #ifdef CONFIG_KALLSYMS -struct module_sect_attr { - struct bin_attribute battr; -}; - struct module_sect_attrs { struct attribute_group grp; - struct module_sect_attr attrs[]; + struct bin_attribute attrs[]; }; #define MODULE_SECT_READ_SIZE (3 /* "0x", "\n" */ + (BITS_PER_LONG / 4)) @@ -69,8 +65,8 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) { unsigned int nloaded = 0, i, size[2]; struct module_sect_attrs *sect_attrs; - struct module_sect_attr *sattr; struct bin_attribute **gattr; + struct bin_attribute *sattr; int ret; /* Count loaded sections and allocate structures */ @@ -95,18 +91,18 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) if (sect_empty(sec)) continue; - sysfs_bin_attr_init(&sattr->battr); - sattr->battr.attr.name = + sysfs_bin_attr_init(sattr); + sattr->attr.name = kstrdup(info->secstrings + sec->sh_name, GFP_KERNEL); - if (!sattr->battr.attr.name) { + if (!sattr->attr.name) { ret = -ENOMEM; goto out; } - sattr->battr.read = module_sect_read; - sattr->battr.private = (void *)sec->sh_addr; - sattr->battr.size = MODULE_SECT_READ_SIZE; - sattr->battr.attr.mode = 0400; - *(gattr++) = &(sattr++)->battr; + sattr->read = module_sect_read; + sattr->private = (void *)sec->sh_addr; + sattr->size = MODULE_SECT_READ_SIZE; + sattr->attr.mode = 0400; + *(gattr++) = sattr++; } *gattr = NULL; @@ -186,7 +182,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) continue; if (info->sechdrs[i].sh_type == SHT_NOTE) { sysfs_bin_attr_init(nattr); - nattr->attr.name = mod->sect_attrs->attrs[loaded].battr.attr.name; + nattr->attr.name = mod->sect_attrs->attrs[loaded].attr.name; nattr->attr.mode = 0444; nattr->size = info->sechdrs[i].sh_size; nattr->private = (void *)info->sechdrs[i].sh_addr; -- cgit v1.2.3 From f47c0bebed44673c54a6a1870f04a2261db28ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 27 Dec 2024 14:23:23 +0100 Subject: module: sysfs: Simplify section attribute allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing allocation logic manually stuffs two allocations into one. This is hard to understand and of limited value, given that all the section names are allocated on their own anyways. Une one allocation per datastructure. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20241227-sysfs-const-bin_attr-module-v2-4-e267275f0f37@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/sysfs.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 89797c556e8c..05b1e3a6b644 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -58,34 +58,37 @@ static void free_sect_attrs(struct module_sect_attrs *sect_attrs) for (bin_attr = sect_attrs->grp.bin_attrs; *bin_attr; bin_attr++) kfree((*bin_attr)->attr.name); + kfree(sect_attrs->grp.bin_attrs); kfree(sect_attrs); } static int add_sect_attrs(struct module *mod, const struct load_info *info) { - unsigned int nloaded = 0, i, size[2]; struct module_sect_attrs *sect_attrs; struct bin_attribute **gattr; struct bin_attribute *sattr; + unsigned int nloaded = 0, i; int ret; /* Count loaded sections and allocate structures */ for (i = 0; i < info->hdr->e_shnum; i++) if (!sect_empty(&info->sechdrs[i])) nloaded++; - size[0] = ALIGN(struct_size(sect_attrs, attrs, nloaded), - sizeof(sect_attrs->grp.bin_attrs[0])); - size[1] = (nloaded + 1) * sizeof(sect_attrs->grp.bin_attrs[0]); - sect_attrs = kzalloc(size[0] + size[1], GFP_KERNEL); + sect_attrs = kzalloc(struct_size(sect_attrs, attrs, nloaded), GFP_KERNEL); if (!sect_attrs) return -ENOMEM; + gattr = kcalloc(nloaded + 1, sizeof(*gattr), GFP_KERNEL); + if (!gattr) { + kfree(sect_attrs); + return -ENOMEM; + } + /* Setup section attributes. */ sect_attrs->grp.name = "sections"; - sect_attrs->grp.bin_attrs = (void *)sect_attrs + size[0]; + sect_attrs->grp.bin_attrs = gattr; sattr = §_attrs->attrs[0]; - gattr = §_attrs->grp.bin_attrs[0]; for (i = 0; i < info->hdr->e_shnum; i++) { Elf_Shdr *sec = &info->sechdrs[i]; @@ -104,7 +107,6 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) sattr->attr.mode = 0400; *(gattr++) = sattr++; } - *gattr = NULL; ret = sysfs_create_group(&mod->mkobj.kobj, §_attrs->grp); if (ret) -- cgit v1.2.3 From 4723f16de64e1580ec5dc15173f8da0b316b2257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 27 Dec 2024 14:23:24 +0100 Subject: module: sysfs: Add notes attributes through attribute_group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A kobject is meant to manage the lifecycle of some resource. However the module sysfs code only creates a kobject to get a "notes" subdirectory in sysfs. This can be achieved easier and cheaper by using a sysfs group. Switch the notes attribute code to such a group, similar to how the section allocation in the same file already works. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20241227-sysfs-const-bin_attr-module-v2-5-e267275f0f37@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/sysfs.c | 54 ++++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 05b1e3a6b644..fc2ad7e17cbd 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -138,20 +138,13 @@ static void remove_sect_attrs(struct module *mod) */ struct module_notes_attrs { - struct kobject *dir; - unsigned int notes; - struct bin_attribute attrs[] __counted_by(notes); + struct attribute_group grp; + struct bin_attribute attrs[]; }; -static void free_notes_attrs(struct module_notes_attrs *notes_attrs, - unsigned int i) +static void free_notes_attrs(struct module_notes_attrs *notes_attrs) { - if (notes_attrs->dir) { - while (i-- > 0) - sysfs_remove_bin_file(notes_attrs->dir, - ¬es_attrs->attrs[i]); - kobject_put(notes_attrs->dir); - } + kfree(notes_attrs->grp.bin_attrs); kfree(notes_attrs); } @@ -159,6 +152,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) { unsigned int notes, loaded, i; struct module_notes_attrs *notes_attrs; + struct bin_attribute **gattr; struct bin_attribute *nattr; int ret; @@ -177,7 +171,15 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) if (!notes_attrs) return -ENOMEM; - notes_attrs->notes = notes; + gattr = kcalloc(notes + 1, sizeof(*gattr), GFP_KERNEL); + if (!gattr) { + kfree(notes_attrs); + return -ENOMEM; + } + + notes_attrs->grp.name = "notes"; + notes_attrs->grp.bin_attrs = gattr; + nattr = ¬es_attrs->attrs[0]; for (loaded = i = 0; i < info->hdr->e_shnum; ++i) { if (sect_empty(&info->sechdrs[i])) @@ -189,35 +191,35 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) nattr->size = info->sechdrs[i].sh_size; nattr->private = (void *)info->sechdrs[i].sh_addr; nattr->read = sysfs_bin_attr_simple_read; - ++nattr; + *(gattr++) = nattr++; } ++loaded; } - notes_attrs->dir = kobject_create_and_add("notes", &mod->mkobj.kobj); - if (!notes_attrs->dir) { - ret = -ENOMEM; + ret = sysfs_create_group(&mod->mkobj.kobj, ¬es_attrs->grp); + if (ret) goto out; - } - - for (i = 0; i < notes; ++i) { - ret = sysfs_create_bin_file(notes_attrs->dir, ¬es_attrs->attrs[i]); - if (ret) - goto out; - } mod->notes_attrs = notes_attrs; return 0; out: - free_notes_attrs(notes_attrs, i); + free_notes_attrs(notes_attrs); return ret; } static void remove_notes_attrs(struct module *mod) { - if (mod->notes_attrs) - free_notes_attrs(mod->notes_attrs, mod->notes_attrs->notes); + if (mod->notes_attrs) { + sysfs_remove_group(&mod->mkobj.kobj, + &mod->notes_attrs->grp); + /* + * We are positive that no one is using any notes attrs + * at this point. Deallocate immediately. + */ + free_notes_attrs(mod->notes_attrs); + mod->notes_attrs = NULL; + } } #else /* !CONFIG_KALLSYMS */ -- cgit v1.2.3 From b83815afaeecf4448cdd54b3f9dc2623ebbc3576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Fri, 27 Dec 2024 14:23:25 +0100 Subject: module: sysfs: Use const 'struct bin_attribute' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sysfs core is switching to 'const struct bin_attribute's. Prepare for that. Signed-off-by: Thomas Weißschuh Reviewed-by: Petr Pavlu Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20241227-sysfs-const-bin_attr-module-v2-6-e267275f0f37@weissschuh.net Signed-off-by: Petr Pavlu --- kernel/module/sysfs.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index fc2ad7e17cbd..f99616499e2e 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -26,7 +26,7 @@ struct module_sect_attrs { #define MODULE_SECT_READ_SIZE (3 /* "0x", "\n" */ + (BITS_PER_LONG / 4)) static ssize_t module_sect_read(struct file *file, struct kobject *kobj, - struct bin_attribute *battr, + const struct bin_attribute *battr, char *buf, loff_t pos, size_t count) { char bounce[MODULE_SECT_READ_SIZE + 1]; @@ -54,18 +54,18 @@ static ssize_t module_sect_read(struct file *file, struct kobject *kobj, static void free_sect_attrs(struct module_sect_attrs *sect_attrs) { - struct bin_attribute **bin_attr; + const struct bin_attribute *const *bin_attr; - for (bin_attr = sect_attrs->grp.bin_attrs; *bin_attr; bin_attr++) + for (bin_attr = sect_attrs->grp.bin_attrs_new; *bin_attr; bin_attr++) kfree((*bin_attr)->attr.name); - kfree(sect_attrs->grp.bin_attrs); + kfree(sect_attrs->grp.bin_attrs_new); kfree(sect_attrs); } static int add_sect_attrs(struct module *mod, const struct load_info *info) { struct module_sect_attrs *sect_attrs; - struct bin_attribute **gattr; + const struct bin_attribute **gattr; struct bin_attribute *sattr; unsigned int nloaded = 0, i; int ret; @@ -86,7 +86,7 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) /* Setup section attributes. */ sect_attrs->grp.name = "sections"; - sect_attrs->grp.bin_attrs = gattr; + sect_attrs->grp.bin_attrs_new = gattr; sattr = §_attrs->attrs[0]; for (i = 0; i < info->hdr->e_shnum; i++) { @@ -101,7 +101,7 @@ static int add_sect_attrs(struct module *mod, const struct load_info *info) ret = -ENOMEM; goto out; } - sattr->read = module_sect_read; + sattr->read_new = module_sect_read; sattr->private = (void *)sec->sh_addr; sattr->size = MODULE_SECT_READ_SIZE; sattr->attr.mode = 0400; @@ -144,7 +144,7 @@ struct module_notes_attrs { static void free_notes_attrs(struct module_notes_attrs *notes_attrs) { - kfree(notes_attrs->grp.bin_attrs); + kfree(notes_attrs->grp.bin_attrs_new); kfree(notes_attrs); } @@ -152,7 +152,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) { unsigned int notes, loaded, i; struct module_notes_attrs *notes_attrs; - struct bin_attribute **gattr; + const struct bin_attribute **gattr; struct bin_attribute *nattr; int ret; @@ -178,7 +178,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info) } notes_attrs->grp.name = "notes"; - notes_attrs->grp.bin_attrs = gattr; + notes_attrs->grp.bin_attrs_new = gattr; nattr = ¬es_attrs->attrs[0]; for (loaded = i = 0; i < info->hdr->e_shnum; ++i) { -- cgit v1.2.3 From 097fd001e1c42b5fd0a6d77cbd855ce373d7e7a2 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 5 Dec 2024 20:46:15 +0100 Subject: module: Split module_enable_rodata_ro() module_enable_rodata_ro() is called twice, once before module init to set rodata sections readonly and once after module init to set rodata_after_init section readonly. The second time, only the rodata_after_init section needs to be set to read-only, no need to re-apply it to already set rodata. Split module_enable_rodata_ro() in two. Signed-off-by: Christophe Leroy Tested-by: Daniel Gomez Reviewed-by: Luis Chamberlain Link: https://lore.kernel.org/r/e3b6ff0df7eac281c58bb02cecaeb377215daff3.1733427536.git.christophe.leroy@csgroup.eu Signed-off-by: Petr Pavlu --- kernel/module/internal.h | 3 ++- kernel/module/main.c | 4 ++-- kernel/module/strict_rwx.c | 13 +++++++++---- 3 files changed, 13 insertions(+), 7 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/internal.h b/kernel/module/internal.h index ac73da5f15bc..b35c0ec54a89 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -327,7 +327,8 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root * } #endif /* CONFIG_MODULES_TREE_LOOKUP */ -int module_enable_rodata_ro(const struct module *mod, bool after_init); +int module_enable_rodata_ro(const struct module *mod); +int module_enable_rodata_ro_after_init(const struct module *mod); int module_enable_data_nx(const struct module *mod); int module_enable_text_rox(const struct module *mod); int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs, diff --git a/kernel/module/main.c b/kernel/module/main.c index 8154e1665cdb..8acec43e97dc 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2953,7 +2953,7 @@ static noinline int do_init_module(struct module *mod) /* Switch to core kallsyms now init is done: kallsyms may be walking! */ rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms); #endif - ret = module_enable_rodata_ro(mod, true); + ret = module_enable_rodata_ro_after_init(mod); if (ret) goto fail_mutex_unlock; mod_tree_remove_init(mod); @@ -3123,7 +3123,7 @@ static int complete_formation(struct module *mod, struct load_info *info) module_bug_finalize(info->hdr, info->sechdrs, mod); module_cfi_finalize(info->hdr, info->sechdrs, mod); - err = module_enable_rodata_ro(mod, false); + err = module_enable_rodata_ro(mod); if (err) goto out_strict_rwx; err = module_enable_data_nx(mod); diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index 239e5013359d..74834ba15615 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -47,7 +47,7 @@ int module_enable_text_rox(const struct module *mod) return 0; } -int module_enable_rodata_ro(const struct module *mod, bool after_init) +int module_enable_rodata_ro(const struct module *mod) { int ret; @@ -61,12 +61,17 @@ int module_enable_rodata_ro(const struct module *mod, bool after_init) if (ret) return ret; - if (after_init) - return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); - return 0; } +int module_enable_rodata_ro_after_init(const struct module *mod) +{ + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) + return 0; + + return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro); +} + int module_enable_data_nx(const struct module *mod) { if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) -- cgit v1.2.3 From 110b1e070f1d50f5217bd2c758db094998bb7b77 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 5 Dec 2024 20:46:16 +0100 Subject: module: Don't fail module loading when setting ro_after_init section RO failed Once module init has succeded it is too late to cancel loading. If setting ro_after_init data section to read-only fails, all we can do is to inform the user through a warning. Reported-by: Thomas Gleixner Closes: https://lore.kernel.org/all/20230915082126.4187913-1-ruanjinjie@huawei.com/ Fixes: d1909c022173 ("module: Don't ignore errors from set_memory_XX()") Signed-off-by: Christophe Leroy Reviewed-by: Luis Chamberlain Link: https://lore.kernel.org/r/d6c81f38da76092de8aacc8c93c4c65cb0fe48b8.1733427536.git.christophe.leroy@csgroup.eu Signed-off-by: Petr Pavlu --- kernel/module/main.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'kernel/module') diff --git a/kernel/module/main.c b/kernel/module/main.c index 8acec43e97dc..8808b6906d5a 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2955,7 +2955,10 @@ static noinline int do_init_module(struct module *mod) #endif ret = module_enable_rodata_ro_after_init(mod); if (ret) - goto fail_mutex_unlock; + pr_warn("%s: module_enable_rodata_ro_after_init() returned %d, " + "ro_after_init data might still be writable\n", + mod->name, ret); + mod_tree_remove_init(mod); module_arch_freeing_init(mod); for_class_mod_mem_type(type, init) { @@ -2994,8 +2997,6 @@ static noinline int do_init_module(struct module *mod) return 0; -fail_mutex_unlock: - mutex_unlock(&module_mutex); fail_free_freeinit: kfree(freeinit); fail: -- cgit v1.2.3 From f3b93547b91ad849b58eb5ab2dd070950ad7beb3 Mon Sep 17 00:00:00 2001 From: Thorsten Leemhuis Date: Wed, 16 Oct 2024 16:18:41 +0200 Subject: module: sign with sha512 instead of sha1 by default Switch away from using sha1 for module signing by default and use the more modern sha512 instead, which is what among others Arch, Fedora, RHEL, and Ubuntu are currently using for their kernels. Sha1 has not been considered secure against well-funded opponents since 2005[1]; since 2011 the NIST and other organizations furthermore recommended its replacement[2]. This is why OpenSSL on RHEL9, Fedora Linux 41+[3], and likely some other current and future distributions reject the creation of sha1 signatures, which leads to a build error of allmodconfig configurations: 80A20474797F0000:error:03000098:digital envelope routines:do_sigver_init:invalid digest:crypto/evp/m_sigver.c:342: make[4]: *** [.../certs/Makefile:53: certs/signing_key.pem] Error 1 make[4]: *** Deleting file 'certs/signing_key.pem' make[4]: *** Waiting for unfinished jobs.... make[3]: *** [.../scripts/Makefile.build:478: certs] Error 2 make[2]: *** [.../Makefile:1936: .] Error 2 make[1]: *** [.../Makefile:224: __sub-make] Error 2 make[1]: Leaving directory '...' make: *** [Makefile:224: __sub-make] Error 2 This change makes allmodconfig work again and sets a default that is more appropriate for current and future users, too. Link: https://www.schneier.com/blog/archives/2005/02/cryptanalysis_o.html [1] Link: https://csrc.nist.gov/projects/hash-functions [2] Link: https://fedoraproject.org/wiki/Changes/OpenSSLDistrustsha1SigVer [3] Signed-off-by: Thorsten Leemhuis Reviewed-by: Sami Tolvanen Tested-by: kdevops [0] Link: https://github.com/linux-kdevops/linux-modules-kpd/actions/runs/11420092929/job/31775404330 [0] Link: https://lore.kernel.org/r/52ee32c0c92afc4d3263cea1f8a1cdc809728aff.1729088288.git.linux@leemhuis.info Signed-off-by: Petr Pavlu --- kernel/module/Kconfig | 1 + 1 file changed, 1 insertion(+) (limited to 'kernel/module') diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig index 7b329057997a..74fe976e3b01 100644 --- a/kernel/module/Kconfig +++ b/kernel/module/Kconfig @@ -231,6 +231,7 @@ comment "Do not forget to sign required modules with scripts/sign-file" choice prompt "Hash algorithm to sign modules" depends on MODULE_SIG || IMA_APPRAISE_MODSIG + default MODULE_SIG_SHA512 help This determines which sort of hashing algorithm will be used during signature generation. This algorithm _must_ be built into the kernel -- cgit v1.2.3