Re: [PATCH v2 2/2] module: remove srcversion module attribute
Petr Pavlu <[email protected]> Tue, 4 Aug 2026 16:35:38 +0200
| Newsgroups | gmane.linux.kbuild.devel,gmane.linux.kernel,gmane.linux.kernel.modules |
|---|---|
| Message-ID | <[email protected]> |
On 7/31/26 3:02 PM, Greg Kroah-Hartman wrote: > Now that module versions are gone, there is no need for srcversion > either. So drop the logic for this from both the kernel and the helper > module tools. This explanation seems somewhat misleading to me, given that MODULE_SRCVERSION_ALL exists. IMO the main motivation for removing srcversion should be that it has nowadays been replaced by build IDs. I suggest something like the following: """ The srcversion modinfo field was primarily used to identify binaries. Nowadays, modules contain a build ID, which is the standard mechanism for this purpose. The information is exposed via /sys/module/<module>/notes/.note.gnu.build-id. Drop the logic for srcversion from both the kernel and modpost. """ Lucas also recently added support for modinfo to print build IDs [1]. > > Cc: Luis Chamberlain <[email protected]> > Cc: Petr Pavlu <[email protected]> > Cc: Daniel Gomez <[email protected]> > Cc: Sami Tolvanen <[email protected]> > Cc: Aaron Tomlin <[email protected]> > Cc: Shyam Saini <[email protected]> > Cc: Kees Cook <[email protected]> > Cc: Thorsten Blum <[email protected]> > Cc: Christoph Hellwig <[email protected]> > Signed-off-by: Greg Kroah-Hartman <[email protected]> > --- > Documentation/ABI/stable/sysfs-module | 6 ------ > drivers/block/drbd/drbd_buildtag.c | 7 +------ > include/linux/module.h | 1 - > kernel/module/Kconfig | 11 ----------- > kernel/module/main.c | 30 ------------------------------ > scripts/Makefile.modpost | 1 - > scripts/mod/modpost.c | 24 ------------------------ > scripts/mod/modpost.h | 1 - > 8 files changed, 1 insertion(+), 80 deletions(-) > > diff --git a/Documentation/ABI/stable/sysfs-module b/Documentation/ABI/stable/sysfs-module > index 7a3cdd8a8d9f..5c028af9921a 100644 > --- a/Documentation/ABI/stable/sysfs-module > +++ b/Documentation/ABI/stable/sysfs-module > @@ -33,9 +33,3 @@ Description: > Note: If the module is built into the kernel, or if the > CONFIG_MODULE_UNLOAD kernel configuration value is not enabled, > this file will not be present. > - > -What: /sys/module/<MODULENAME>/srcversion > -Date: Jun 2005 > -Description: > - If the module source has MODULE_VERSION, this file will contain > - the checksum of the source code. > diff --git a/drivers/block/drbd/drbd_buildtag.c b/drivers/block/drbd/drbd_buildtag.c > index cd0389488f63..22f1e842f9ce 100644 > --- a/drivers/block/drbd/drbd_buildtag.c > +++ b/drivers/block/drbd/drbd_buildtag.c > @@ -10,13 +10,8 @@ const char *drbd_buildtag(void) > > static char buildtag[38] = "\0uilt-in"; > > - if (buildtag[0] == 0) { > -#ifdef MODULE > - sprintf(buildtag, "srcversion: %-24s", THIS_MODULE->srcversion); > -#else > + if (buildtag[0] == 0) > buildtag[0] = 'b'; > -#endif > - } > > return buildtag; > } THIS_MODULE->srcversion could be replaced with module_buildid(THIS_MODULE). I'll leave it to the DRBD maintainers to decide what they prefer. > diff --git a/include/linux/module.h b/include/linux/module.h > index 209b6a388be7..cafb99479966 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -377,7 +377,6 @@ struct module { > /* Sysfs stuff. */ > struct module_kobject mkobj; > struct module_attribute *modinfo_attrs; > - const char *srcversion; > const char *imported_namespaces; > struct kobject *holders_dir; > > diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig > index 43b1bb01fd27..e27f12778e24 100644 > --- a/kernel/module/Kconfig > +++ b/kernel/module/Kconfig > @@ -238,17 +238,6 @@ config BASIC_MODVERSIONS > > endif # MODVERSIONS > > -config MODULE_SRCVERSION_ALL > - bool "Source checksum for all modules" > - help > - Modules which contain a MODULE_VERSION get an extra "srcversion" > - field inserted into their modinfo section, which contains a > - sum of the source files which made it. This helps maintainers > - see exactly which source was used to build a module (since > - others sometimes change the module source without updating > - the version). With this option, such a "srcversion" field > - will be created for all modules. If unsure, say N. > - Various default and test configs set MODULE_SRCVERSION_ALL=y, which should be removed. linux> git grep CONFIG_MODULE_SRCVERSION_ALL arch/arm/configs/imx_v6_v7_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/arm/configs/omap2plus_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/arm/configs/pxa_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/bigsur_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/decstation_64_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/decstation_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/decstation_r4k_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/ip22_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/ip27_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/ip28_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/ip30_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/malta_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/malta_kvm_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/malta_qemu_32r6_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/maltaaprp_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/maltasmvp_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/maltasmvp_eva_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/maltaup_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/maltaup_xpa_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/mtx1_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/pic32mzda_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/mips/configs/sb1250_swarm_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/powerpc/configs/g5_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/powerpc/configs/powernv_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/powerpc/configs/ppc64_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/powerpc/configs/ppc64e_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/powerpc/configs/ppc6xx_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/powerpc/configs/tqm8xx_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/s390/configs/debug_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/s390/configs/defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/sh/configs/magicpanelr2_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y arch/sparc/configs/sparc64_defconfig:CONFIG_MODULE_SRCVERSION_ALL=y tools/testing/selftests/bpf/config:CONFIG_MODULE_SRCVERSION_ALL=y > config MODULE_SIG > bool "Module signature verification" > select MODULE_SIG_FORMAT > diff --git a/kernel/module/main.c b/kernel/module/main.c > index da8be9788d13..5f590ce90f2d 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -575,35 +575,6 @@ bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr) > > #endif /* CONFIG_SMP */ > > -#define MODINFO_ATTR(field) \ > -static void setup_modinfo_##field(struct module *mod, const char *s) \ > -{ \ > - mod->field = kstrdup(s, GFP_KERNEL); \ > -} \ > -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); \ > -} \ > -static int modinfo_##field##_exists(struct module *mod) \ > -{ \ > - return mod->field != NULL; \ > -} \ > -static void free_modinfo_##field(struct module *mod) \ > -{ \ > - kfree(mod->field); \ > - mod->field = NULL; \ > -} \ > -static const struct module_attribute modinfo_##field = { \ > - .attr = { .name = __stringify(field), .mode = 0444 }, \ > - .show = show_modinfo_##field, \ > - .setup = setup_modinfo_##field, \ > - .test = modinfo_##field##_exists, \ > - .free = free_modinfo_##field, \ > -}; > - > -MODINFO_ATTR(srcversion); > - > static void setup_modinfo_import_ns(struct module *mod, const char *s) > { > mod->imported_namespaces = NULL; > @@ -1083,7 +1054,6 @@ static const struct module_attribute modinfo_taint = > > const struct module_attribute *const modinfo_attrs[] = { > &module_uevent, > - &modinfo_srcversion, > &modinfo_import_ns, > &modinfo_initstate, > &modinfo_coresize, > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > index d7d45067d08b..b6c1467cf527 100644 > --- a/scripts/Makefile.modpost > +++ b/scripts/Makefile.modpost > @@ -45,7 +45,6 @@ modpost-args = \ > $(if $(CONFIG_MODVERSIONS),-m) \ > $(if $(CONFIG_BASIC_MODVERSIONS),-b) \ > $(if $(CONFIG_EXTENDED_MODVERSIONS),-x) \ > - $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \ > $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \ > $(if $(KBUILD_MODPOST_WARN),-w) \ > $(if $(KBUILD_NSDEPS),-d modules.nsdeps) \ > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index a7b72a81d248..2e969be7f414 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -33,8 +33,6 @@ > static bool module_enabled; > /* Are we using CONFIG_MODVERSIONS? */ > static bool modversions; > -/* Is CONFIG_MODULE_SRCVERSION_ALL set? */ > -static bool all_versions; > /* Is CONFIG_BASIC_MODVERSIONS set? */ > static bool basic_modversions; > /* Is CONFIG_EXTENDED_MODVERSIONS set? */ > @@ -1584,7 +1582,6 @@ static void mod_set_crcs(struct module *mod) > static void read_symbols(const char *modname) > { > const char *symname; > - char *version; > char *license; > char *namespace; > struct module *mod; > @@ -1645,13 +1642,6 @@ static void read_symbols(const char *modname) > > check_sec_ref(mod, &info); > > - if (!mod->is_vmlinux) { > - version = get_modinfo(&info, "version"); > - if (version || all_versions) > - get_src_version(mod->name, mod->srcversion, > - sizeof(mod->srcversion) - 1); > - } > - Sashiko correctly points out [2] that, with the removal of the get_src_version() call in modpost.c, the entire scripts/mod/sumversion.c can be removed. Additionally, since sumversion.c was the only external caller of get_basename(), read_text_file(), get_line() in modpost.c, these functions can now optionally be made static. > parse_elf_finish(&info); > > if (modversions) { > @@ -2035,15 +2025,6 @@ static void add_depends(struct buffer *b, struct module *mod) > buf_printf(b, "\");\n"); > } > > -static void add_srcversion(struct buffer *b, struct module *mod) > -{ > - if (mod->srcversion[0]) { > - buf_printf(b, "\n"); > - buf_printf(b, "MODULE_INFO(srcversion, \"%s\");\n", > - mod->srcversion); > - } > -} > - > static void write_buf(struct buffer *b, const char *fname) > { > FILE *file; > @@ -2149,8 +2130,6 @@ static void write_mod_c_file(struct module *mod) > free(alias); > } > > - add_srcversion(&buf, mod); > - > ret = snprintf(fname, sizeof(fname), "%s.mod.c", mod->name); > if (ret >= sizeof(fname)) { > error("%s: too long path was truncated\n", fname); > @@ -2327,9 +2306,6 @@ int main(int argc, char **argv) > case 'o': > dump_write = optarg; > break; > - case 'a': > - all_versions = true; > - break; > case 'T': > files_source = optarg; > break; Sashiko correctly notes that the 'a' character should be removed from the preceding getopt() call. > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h > index 2aecb8f25c87..2c2f1d2b4021 100644 > --- a/scripts/mod/modpost.h > +++ b/scripts/mod/modpost.h > @@ -126,7 +126,6 @@ struct module { > bool seen; > bool has_init; > bool has_cleanup; > - char srcversion[25]; > // Missing namespace dependencies > struct list_head missing_namespaces; > // Actual imported namespaces > [1] https://github.com/kmod-project/kmod/commit/dc1fe2572a41eec540f8309d8a62043ce60c9f33 [2] https://lore.kernel.org/linux-modules/[email protected]/ -- Thanks, Petr