All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH v5 12/14] module: Introduce hash-based integrity checking
Date: Tue, 05 May 2026 09:49:06 +0000	[thread overview]
Message-ID: <20260505094907.649D0C2BCB4@smtp.kernel.org> (raw)
In-Reply-To: <20260505-module-hashes-v5-12-e174a5a49fce@weissschuh.net>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Build corruption due to concurrent Kbuild executions of the `modules` target.
- [High] Standalone module builds and installations are fundamentally broken.
- [Low] Memory leak in the `modules-merkle-tree` host utility.
--

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

module: Introduce hash-based integrity checking

This commit introduces a hash-based integrity checking mechanism to support
reproducible builds. It embeds a Merkle tree root of all builtin modules
into vmlinux, while out-of-tree modules continue to be validated through
signatures.

> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index 9ba45e5b32b18..68708a039a621 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
[ ... ]
> @@ -116,6 +122,11 @@ quiet_cmd_sign :=
>        cmd_sign := :
>  endif
>  
> +ifeq ($(KBUILD_EXTMOD)|$(CONFIG_MODULE_HASHES),|y)
> +quiet_cmd_sign = MERKLE [M] $@
> +      cmd_sign = cat $(objtree)/$*.merkle >> $@
> +endif

If modules are built and installed independently of a full vmlinux compilation
using make modules followed by make modules_install, will this cat command
abort with a missing file error? 

The .merkle files appear to only be generated by scripts/modules-merkle-tree
during the vmlinux link stage, which might leave standalone module builds
without the required proof files.

> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> index 6cc661e5292b0..a0332c06bde1d 100644
> --- a/scripts/Makefile.vmlinux
> +++ b/scripts/Makefile.vmlinux
[ ... ]
> @@ -78,6 +78,33 @@ ifdef CONFIG_BUILDTIME_TABLE_SORT
>  vmlinux.unstripped: scripts/sorttable
>  endif
>  
> +ifdef CONFIG_MODULE_HASHES
> +targets += .tmp_module_hashes.c
> +
> +modules.order: vmlinux.unstripped FORCE
> +	$(Q)echo "  MAKE    modules"
> +	$(Q)$(MAKE) -f $(srctree)/Makefile modules

Can this recursive make invocation race with the top-level modules target if
a user runs make -j vmlinux modules? 

Since both targets share the modpost prerequisite, it seems Kbuild could
evaluate both concurrently. This might cause two independent instances of
scripts/Makefile.modfinal to compile and link the exact same .mod.o and
.ko files at the same time, leading to unpredictable file truncations.

> diff --git a/scripts/modules-merkle-tree.c b/scripts/modules-merkle-tree.c
> new file mode 100644
> index 0000000000000..10e3455d5d7af
> --- /dev/null
> +++ b/scripts/modules-merkle-tree.c
[ ... ]
> +	mt = build_merkle(fh_list, num_files);
> +	write_merkle_root(mt, argv[1]);
> +	for (size_t i = 0; i < num_files; i++) {
> +		char *signame;
> +		int fd;
> +
> +		signame = xstrdup_replace_suffix(fh_list[i].name, kmod_suffix, ".merkle");
> +
> +		fd = open(signame, O_WRONLY | O_CREAT | O_TRUNC, 0644);
> +		if (fd < 0)
> +			err(1, "Can't create %s", signame);
> +
> +		build_proof(mt, i, fd);
> +		append_module_signature_magic(fd, lseek(fd, 0, SEEK_CUR));
> +		if (close(fd))
> +			err(1, "Can't write %s", signame);
> +	}

Does this code leak the signame string? The xstrdup_replace_suffix helper
allocates memory that does not appear to be freed before the end of the
loop, which might leave memory stranded for each kernel module processed.

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

  reply	other threads:[~2026-05-05  9:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05  9:05 [PATCH v5 00/14] module: Introduce hash-based integrity checking Thomas Weißschuh
2026-05-05  9:05 ` [PATCH v5 01/14] kbuild: generate module BTF based on vmlinux.unstripped Thomas Weißschuh
2026-05-05  9:05 ` [PATCH v5 02/14] lockdown: Make the relationship to MODULE_SIG a dependency Thomas Weißschuh
2026-05-05  9:32   ` sashiko-bot
2026-05-05 12:27     ` Nicolas Bouchinet
2026-05-05  9:05 ` [PATCH v5 03/14] kbuild: rename the strip_relocs command Thomas Weißschuh
2026-05-05  9:05 ` [PATCH v5 04/14] module: Drop pointless debugging message Thomas Weißschuh
2026-05-05  9:05 ` [PATCH v5 05/14] module: Make mod_verify_sig() static Thomas Weißschuh
2026-05-05  9:05 ` [PATCH v5 06/14] module: Switch load_info::len to size_t Thomas Weißschuh
2026-05-26  9:47   ` Petr Pavlu
2026-05-26 11:35     ` Thomas Weißschuh
2026-05-05  9:05 ` [PATCH v5 07/14] module: Make module authentication usable without MODULE_SIG Thomas Weißschuh
2026-05-05  9:40   ` sashiko-bot
2026-05-26 10:53   ` Petr Pavlu
2026-05-26 11:38     ` Thomas Weißschuh
2026-05-26 12:27       ` kpcyrd
2026-05-05  9:05 ` [PATCH v5 08/14] module: Move authentication logic into dedicated new file Thomas Weißschuh
2026-05-26 11:58   ` Petr Pavlu
2026-05-05  9:05 ` [PATCH v5 09/14] module: Move signature type check out of mod_check_sig() Thomas Weißschuh
2026-05-26 13:03   ` Petr Pavlu
2026-05-05  9:05 ` [PATCH v5 10/14] module: Prepare for additional module authentication mechanisms Thomas Weißschuh
2026-05-26 13:14   ` Petr Pavlu
2026-05-05  9:05 ` [PATCH v5 11/14] module: update timestamp of modules.order after modules are built Thomas Weißschuh
2026-05-05  9:41   ` sashiko-bot
2026-05-05  9:05 ` [PATCH v5 12/14] module: Introduce hash-based integrity checking Thomas Weißschuh
2026-05-05  9:49   ` sashiko-bot [this message]
2026-05-05  9:05 ` [PATCH v5 13/14] kbuild: move handling of module stripping to Makefile.lib Thomas Weißschuh
2026-05-05  9:35   ` sashiko-bot
2026-05-05  9:05 ` [PATCH v5 14/14] kbuild: make CONFIG_MODULE_HASHES compatible with module stripping Thomas Weißschuh
2026-05-05 10:04   ` sashiko-bot
2026-05-18 21:55 ` [PATCH v5 00/14] module: Introduce hash-based integrity checking Sami Tolvanen
2026-05-19 18:19   ` Thomas Weißschuh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260505094907.649D0C2BCB4@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=sashiko@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.