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

Reply via email to