Re: [PATCH v5 14/14] kbuild: make CONFIG_MODULE_HASHES compatible with module stripping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The string comparison in `scripts/Makefile.modinst` unconditionally fails, completely breaking `make modules_install` when `CONFIG_MODULE_HASHES` is enabled.
- [High] Module hashes are calculated on unstripped modules if `INSTALL_MOD_STRIP` is not explicitly passed during the build phase (`make modules`), leading to unbootable modules.
- [Medium] The build system unconditionally generates redundant `.ko.stripped` files for every module, even if `CONFIG_MODULE_HASHES` is disabled or no stripping is requested.
--

commit c60e91b7cfcdafd6de940881990bcbadd0ee0179
Author: Thomas Weißschuh <linux@weissschuh.net>

kbuild: make CONFIG_MODULE_HASHES compatible with module stripping

This commit aims to fix module loading when CONFIG_MODULE_HASHES is used
alongside module stripping. It records the INSTALL_MOD_STRIP value in Kconfig
and validates consistency between build and install time. It also introduces
stripped module variants during compilation to ensure the generated hashes
match the final installed files.

> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index 99482fac07018..a5b3807e2304d 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig

[ ... ]

> @@ -423,6 +423,11 @@ config MODULE_HASHES
>  
>  	  Also see the warning in MODULE_SIG about stripping modules.
>  
> +# To validate the consistency of INSTALL_MOD_STRIP for MODULE_HASHES
> +config MODULE_INSTALL_STRIP
> +	string
> +	default "$(INSTALL_MOD_STRIP)"
> +

Is it intended for config MODULE_INSTALL_STRIP to lack a depends on
MODULE_HASHES statement? Without it, the config appears to be evaluated
and saved as a quoted string even when hashes are not used.

[ ... ]

> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 44a382689a5aa..9924a7bb73c5e 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal

[ ... ]

> +%.ko.stripped: %.ko $(wildcard include/config/MODULE_INSTALL_STRIP)
> +	$(call cmd,install_mod)
> +	$(call cmd,strip_mod)

Does cmd_strip_mod evaluate to a no-op during the build phase if
INSTALL_MOD_STRIP is not explicitly passed to the build command?

Since users typically pass INSTALL_MOD_STRIP=1 only during the module
installation step and not during compilation, this might cause hashes to be
calculated on unstripped modules during the build. If the modules are then
properly stripped during installation, could this result in a hash mismatch
that prevents module loading?

> +
> +ifneq ($(CONFIG_MODULE_INSTALL_STRIP),)
> +modules.order: $(modules:%.o=%.ko.stripped)
> +endif

Since Kconfig strings are exported to Make with quotes, won't this check
evaluate to true even if the string is empty, as the value will be "" rather
than completely empty?

This looks like it would force the generation of .ko.stripped targets for
all modules, adding unnecessary overhead to the build even if module hashes
are disabled or no stripping is requested.

[ ... ]

> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index b95f613e23c89..fd1fb89bb0bd7 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -68,8 +68,8 @@ __modinst: $(install-y)
>  
>  ifdef CONFIG_MODULE_HASHES
>  ifeq ($(KBUILD_EXTMOD),)
> -ifdef INSTALL_MOD_STRIP
> -$(error CONFIG_MODULE_HASHES and INSTALL_MOD_STRIP are mutually exclusive)
> +ifneq ($(INSTALL_MOD_STRIP),$(CONFIG_MODULE_INSTALL_STRIP))
> +$(error Inconsistent values for INSTALL_MOD_STRIP between build and installation)
>  endif

Kconfig string options are exported to Make with literal double quotes,
such as CONFIG_MODULE_INSTALL_STRIP="1", while the INSTALL_MOD_STRIP
variable passed via the command line or environment is typically unquoted.

Will this literal comparison always evaluate as unequal, meaning 1 != "1",
and therefore prevent module installation when CONFIG_MODULE_HASHES is enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505-module-hashes-v5-0-e174a5a49fce@weissschuh.net?part=14