* Re: [PATCH v4 03/17] ima: efi: Drop unnecessary check for CONFIG_MODULE_SIG/CONFIG_KEXEC_SIG
From: Eric Biggers @ 2026-03-10 21:11 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-3-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:47PM +0100, Thomas Weißschuh wrote:
> When configuration settings are disabled the guarded functions are
> defined as empty stubs, so the check is unnecessary.
> The specific configuration option for set_module_sig_enforced() is
> about to change and removing the checks avoids some later churn.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> security/integrity/ima/ima_efi.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
- Eric
^ permalink raw reply
* Re: [PATCH v4 04/17] module: Make mod_verify_sig() static
From: Eric Biggers @ 2026-03-10 21:12 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-4-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:48PM +0100, Thomas Weißschuh wrote:
> It is not used outside of signing.c.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> kernel/module/internal.h | 1 -
> kernel/module/signing.c | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
- Eric
^ permalink raw reply
* Re: [PATCH v4 06/17] kbuild: add stamp file for vmlinux BTF data
From: Eric Biggers @ 2026-03-10 21:36 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-6-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:50PM +0100, Thomas Weißschuh wrote:
> The upcoming module hashes functionality will build the modules in
> between the generation of the BTF data and the final link of vmlinux.
> Having a dependency from the modules on vmlinux would make this
> impossible as it would mean having a cyclic dependency.
> Break this cyclic dependency by introducing a new target.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> scripts/Makefile.modfinal | 4 ++--
> scripts/link-vmlinux.sh | 6 ++++++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 149e12ff5700..adfef1e002a9 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -56,8 +56,8 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
> printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
>
> # Re-generate module BTFs if either module's .ko or vmlinux changed
> -%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
> - +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
> +%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/.tmp_vmlinux_btf.stamp) FORCE
> + +$(call if_changed_except,ld_ko_o,$(objtree)/.tmp_vmlinux_btf.stamp)
> ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +$(if $(newer-prereqs),$(call cmd,btf_ko))
> endif
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 4ab44c73da4d..8c98f8645a5c 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -111,6 +111,7 @@ vmlinux_link()
> gen_btf()
> {
> local btf_data=${1}.btf.o
> + local btf_stamp=.tmp_vmlinux_btf.stamp
>
> info BTF "${btf_data}"
> LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1}
> @@ -131,6 +132,11 @@ gen_btf()
> fi
> printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none
>
> + info STAMP $btf_stamp
> + if ! cmp --silent $btf_data $btf_stamp; then
> + cp $btf_data $btf_stamp
> + fi
A "stamp file" is traditionally an empty file that is written when some
build step has completed. The above code is instead copying the entire
.tmp_vmlinux1.btf.o file (megabytes in size) to .tmp_vmlinux_btf.stamp.
So, it's not clear to me why the stamp file is needed at all, versus
depending directly on .tmp_vmlinux1.btf.o.
I guess 'make' doesn't know about the dependencies of
.tmp_vmlinux1.btf.o. But the same is true of the stamp file, right? So
either way, how would 'make' know to finish rebuilding the file before
starting to execute the "Re-generate module BTFs" rule?
Also, passing the long option '--silent' to 'cmp' creates a dependency
on the GNU implementation of 'cmp', which isn't documented as a kernel
build dependency. Probably better to use the short option '-s'.
Also, the stamp file isn't being deleted by 'make clean'. It looks like
it would need to be added to cleanup() in link-vmlinux.sh.
- Eric
^ permalink raw reply
* Re: [PATCH v4 09/17] module: Make module loading policy usable without MODULE_SIG
From: Eric Biggers @ 2026-03-10 22:01 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-9-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:53PM +0100, Thomas Weißschuh wrote:
> The loading policy functionality will also be used by the hash-based
> module validation. Split it out from CONFIG_MODULE_SIG so it is usable
> by both.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> include/linux/module.h | 8 ++++----
> kernel/module/Kconfig | 5 ++++-
> kernel/module/main.c | 26 +++++++++++++++++++++++++-
> kernel/module/signing.c | 21 ---------------------
> 4 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index f288ca5cd95b..f9601cba47cd 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -444,7 +444,7 @@ struct module {
> const u32 *gpl_crcs;
> bool using_gplonly_symbols;
>
> -#ifdef CONFIG_MODULE_SIG
> +#ifdef CONFIG_MODULE_SIG_POLICY
> /* Signature was verified. */
> bool sig_ok;
> #endif
[...]
> +config MODULE_SIG_POLICY
> + def_bool MODULE_SIG
Maybe MODULE_AUTH_POLICY? Hash-based module authentication does not use
signatures.
This issue appears elsewhere in the code too. There are lots of places
that still refer to module signatures or "sigs", when really module
authentication is meant.
I'm not sure how far you want to go with the renaming, but it's
something to think about. It's confusing to use the term "signature" to
mean something that is not a signature.
- Eric
^ permalink raw reply
* Re: [PATCH v4 10/17] module: Move integrity checks into dedicated function
From: Eric Biggers @ 2026-03-10 22:06 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-10-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:54PM +0100, Thomas Weißschuh wrote:
> +static int module_integrity_check(struct load_info *info, int flags)
> +{
> + int err = 0;
> +
> + if (IS_ENABLED(CONFIG_MODULE_SIG))
> + err = module_sig_check(info, flags);
> +
> + return err;
> +}
Maybe module_authenticity_check()? The purpose is authenticity, not
merely integrity.
- Eric
^ permalink raw reply
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Eric Biggers @ 2026-03-11 1:12 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-15-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:59PM +0100, Thomas Weißschuh wrote:
> The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds. Either the module signing key
> is generated at build time, which makes the build unreproducible, or a
> static signing key is used, which precludes rebuilds by third parties
> and makes the whole build and packaging process much more complicated.
I think this actually undersells the feature. It's also much simpler
than the signature-based module authentication. The latter relies on
PKCS#7, X.509, ASN.1, OID registry, crypto_sig API, etc in addition to
the implementations of the actual signature algorithm (RSA / ECDSA /
ML-DSA) and at least one hash algorithm.
I've even seen a case where the vmlinux size decreases by almost 200KB
just by disabling module signing. That's not even counting the
signatures themselves, which ML-DSA has increased to 2-5 KB each.
The hashes are much simpler, even accounting for the Merkle tree proofs
that make them scalable. They're less likely to have vulnerabilities
like the PKCS#7 bugs the kernel has had historically. They also
eliminate the dependency on a lot of userspace libcrypto functionality
that has been causing portability problems, such as the CMS functions.
So I think this is how module authentication should have been done
originally, and I'm glad to see this is finally being fixed.
> +struct module_hashes_proof {
> + __be32 pos;
> + u8 hash_sigs[][MODULE_HASHES_HASH_SIZE];
> +} __packed;
Is the choice of big endian for consistency with struct
module_signature? Little endian is the usual choice in new code.
> diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
> index a45ce3b24403..3b510651830d 100644
> --- a/include/linux/module_signature.h
> +++ b/include/linux/module_signature.h
> @@ -18,6 +18,7 @@ enum pkey_id_type {
> PKEY_ID_PGP, /* OpenPGP generated key ID */
> PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
> PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
> + PKEY_ID_MERKLE, /* Merkle proof for modules */
I recommend making the hash algorithm explicit:
PKEY_ID_MERKLE_SHA256, /* SHA-256 merkle proof for modules */
While I wouldn't encourage the addition of another hash algorithm
(specifying one good algorithm for now is absolutely the right choice),
if someone ever does need to add another one, we'd want them to be
guided to simply introduce a new value of this enum rather than hack it
in some other way.
> +static void hash_entry(const void *left, const void *right, void *out)
Byte arrays should use u8 instead of void
> diff --git a/scripts/modules-merkle-tree.c b/scripts/modules-merkle-tree.c
[...]
> +struct file_entry {
> + char *name;
> + unsigned int pos;
> + unsigned char hash[EVP_MAX_MD_SIZE];
Considering that the hash algorithm is fixed, EVP_MAX_MD_SIZE can be
replaced with a tighter local definition:
#define MAX_HASH_SIZE 32
> +static struct file_entry *fh_list;
> +static size_t num_files;
> +
> +struct leaf_hash {
> + unsigned char hash[EVP_MAX_MD_SIZE];
> +};
> +
> +struct mtree {
> + struct leaf_hash **l;
> + unsigned int *entries;
> + unsigned int levels;
> +};
'struct leaf_hash' is confusing because it's actually used for the
hashes of internal nodes, not leaf nodes.
Maybe rename it to 'struct hash' and use it for both the hashes and leaf
nodes and internal nodes.
Also, clearer naming would improve readability, e.g.:
struct merkle_tree {
struct hash **level_hashes;
unsigned int level_size;
unsigned int num_levels;
};
> +static void hash_data(void *p, unsigned int pos, size_t size, void *ret_hash)
static void hash_data(const uint8_t *data, unsigned int pos,
size_t size, struct hash *ret_hash)
> + unsigned char magic = 0x01;
uint8_t
Also, when defining these magic numbers, maybe explicitly document that
they are domain separation prefixes:
uint8_t magic = 0x01; /* domain separation prefix */
> + unsigned int pos_be;
uint32_t
> +static void hash_entry(void *left, void *right, void *ret_hash)
Could use stronger typing:
static void hash_entry(const struct hash *left, const struct hash *right,
struct hash *ret_hash)
> +static struct mtree *build_merkle(struct file_entry *fh, size_t num)
Could use clearer names, and constify the file_entry array:
static struct merkle_tree *build_merkle(const struct file_entry *files,
size_t num_files)
> + /* First level of pairs */
> + for (unsigned int i = 0; i < num; i += 2) {
> + if (i == num - 1) {
> + /* Odd number of files, no pair. Hash with itself */
> + hash_entry(fh[i].hash, fh[i].hash, mt->l[0][i / 2].hash);
> + } else {
> + hash_entry(fh[i].hash, fh[i + 1].hash, mt->l[0][i / 2].hash);
> + }
> + }
> + for (unsigned int i = 1; i < mt->levels; i++) {
> + int odd = 0;
> +
> + if (le & 1) {
> + le++;
> + odd++;
> + }
> +
> + mt->entries[i] = le / 2;
> + mt->l[i] = xcalloc(sizeof(**mt->l), le);
> +
> + for (unsigned int n = 0; n < le; n += 2) {
> + if (n == le - 2 && odd) {
> + /* Odd number of pairs, no pair. Hash with itself */
> + hash_entry(mt->l[i - 1][n].hash, mt->l[i - 1][n].hash,
> + mt->l[i][n / 2].hash);
> + } else {
> + hash_entry(mt->l[i - 1][n].hash, mt->l[i - 1][n + 1].hash,
> + mt->l[i][n / 2].hash);
> + }
> + }
> + le = mt->entries[i];
> + }
There should be an assertion at the end that we ended up with exactly 1
hash in the root level.
It might also be possible to refactor this code such that the leaf nodes
and internal nodes are handled in the same loop, rather than handling
the leaf nodes as a special case.
> +static void write_merkle_root(struct mtree *mt, const char *fp)
fp => filename since it's a string, not e.g. a 'FILE *'.
> +{
> + char buf[1024];
> + unsigned int levels;
> + unsigned char *h;
> + FILE *f;
> +
> + if (mt) {
> + levels = mt->levels;
> + h = mt->l[mt->levels - 1][0].hash;
> + } else {
> + levels = 0;
> + h = xcalloc(1, hash_size);
> + }
> +
> + f = fopen(fp, "w");
> + if (!f)
> + err(1, "Failed to create %s", buf);
Above should log the name of the file. 'buf' should be removed.
> +static char *xstrdup_replace_suffix(const char *str, const char *new_suffix)
> +{
> + const char *current_suffix;
> + size_t base_len;
> +
> + current_suffix = strchr(str, '.');
> + if (!current_suffix)
> + errx(1, "No existing suffix in '%s'", str);
This doesn't handle base names that contain a period. strrchr() would
work if the old suffix always contains exactly one period. Otherwise
another solution would be needed to identify the old suffix.
> +static __attribute__((noreturn))
> +void format(void)
usage()
> +{
> + fprintf(stderr,
> + "Usage: scripts/modules-merkle-tree <root definition>\n");
> + exit(2);
This should show both parameters, <root hash> <new suffix>
But they probably should be flipped to put the output second.
Though, is <new suffix> needed at all? It looks like it doesn't
actually affect the output.
> + hash_evp = EVP_get_digestbyname("sha256");
EVP_sha256()
> + hash_size = EVP_MD_get_size(hash_evp);
The old name 'EVP_MD_size()' would have wider compatibility.
> + ERR(hash_size <= 0, "EVP_get_digestbyname");
Log message should say EVP_MD_size
> + for (unsigned int i = 0; i < num_files; i++) {
size_t, for consistency with the type of num_files
> + 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));
Maybe build_and_append_proof()?
- Eric
^ permalink raw reply
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Eric Biggers @ 2026-03-11 1:18 UTC (permalink / raw)
To: Petr Pavlu
Cc: Thomas Weißschuh, Nathan Chancellor, Arnd Bergmann,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Paul Moore,
James Morris, Serge E. Hallyn, Jonathan Corbet,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Naveen N Rao, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Nicolas Schier, Daniel Gomez, Aaron Tomlin,
Christophe Leroy (CS GROUP), Nicolas Schier, Nicolas Bouchinet,
Xiu Jianfeng, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <fab2af64-e396-45f9-8876-feff4002e04b@suse.com>
On Tue, Feb 03, 2026 at 01:19:20PM +0100, Petr Pavlu wrote:
> > +static unsigned int get_pow2(unsigned int val)
> > +{
> > + return 31 - __builtin_clz(val);
> > +}
> > +
> > +static unsigned int roundup_pow2(unsigned int val)
> > +{
> > + return 1 << (get_pow2(val - 1) + 1);
> > +}
> > +
> > +static unsigned int log2_roundup(unsigned int val)
> > +{
> > + return get_pow2(roundup_pow2(val));
> > +}
>
> In the edge case when the kernel is built with only one module, the code
> calls log2_roundup(1) -> roundup_pow2(1) -> get_pow2(0) ->
> __builtin_clz(0). The return value of __builtin_clz() is undefined if
> the input is zero.
A suggestion:
static unsigned int log2_roundup(unsigned int val)
{
if (val <= 1)
return 0;
return 32 - __builtin_clz(val - 1);
}
- Eric
^ permalink raw reply
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Sebastian Andrzej Siewior @ 2026-03-11 8:50 UTC (permalink / raw)
To: Eric Biggers
Cc: Thomas Weißschuh, Nathan Chancellor, Arnd Bergmann,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Naveen N Rao, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Nicolas Schier, Daniel Gomez, Aaron Tomlin,
Christophe Leroy (CS GROUP), Nicolas Schier, Nicolas Bouchinet,
Xiu Jianfeng, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
linux-kbuild, linux-kernel, linux-arch, linux-modules,
linux-security-module, linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20260311011218.GA212983@quark>
On 2026-03-10 18:12:18 [-0700], Eric Biggers wrote:
> > diff --git a/scripts/modules-merkle-tree.c b/scripts/modules-merkle-tree.c
> [...]
>
> > +struct file_entry {
> > + char *name;
> > + unsigned int pos;
> > + unsigned char hash[EVP_MAX_MD_SIZE];
>
> Considering that the hash algorithm is fixed, EVP_MAX_MD_SIZE can be
> replaced with a tighter local definition:
>
> #define MAX_HASH_SIZE 32
>
> > +static struct file_entry *fh_list;
> > +static size_t num_files;
> > +
> > +struct leaf_hash {
> > + unsigned char hash[EVP_MAX_MD_SIZE];
> > +};
> > +
> > +struct mtree {
> > + struct leaf_hash **l;
> > + unsigned int *entries;
> > + unsigned int levels;
> > +};
>
> 'struct leaf_hash' is confusing because it's actually used for the
> hashes of internal nodes, not leaf nodes.
You could still consider the internal nodes as leafs.
> Maybe rename it to 'struct hash' and use it for both the hashes and leaf
> nodes and internal nodes.
>
> Also, clearer naming would improve readability, e.g.:
>
> struct merkle_tree {
> struct hash **level_hashes;
> unsigned int level_size;
> unsigned int num_levels;
> };
but this could improve it, indeed.
> > + hash_evp = EVP_get_digestbyname("sha256");
>
> EVP_sha256()
I would suggest to use EVP_MD_fetch() instead.
> > + hash_size = EVP_MD_get_size(hash_evp);
>
> The old name 'EVP_MD_size()' would have wider compatibility.
EVP_MD_fetch() and EVP_MD_get_size() are openssl 3.0.0+ and nothing
below 3.0.0 is considered supported (while 3.0.0 is EOL 07 Sep 2026).
Sebastian
^ permalink raw reply
* Re: [PATCH v6] ima_fs: Correctly create securityfs files for unsupported hash algos
From: Roberto Sassu @ 2026-03-11 9:25 UTC (permalink / raw)
To: dima, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Silvia Sisinni,
Enrico Bravi
Cc: Jonathan McDowell, linux-integrity, linux-security-module,
linux-kernel, stable, Dmitry Safonov
In-Reply-To: <20260310-ima-oob-v6-1-dc111c846ff4@arista.com>
On Tue, 2026-03-10 at 17:40 +0000, Dmitry Safonov via B4 Relay wrote:
> From: Dmitry Safonov <dima@arista.com>
>
> ima_tpm_chip->allocated_banks[i].crypto_id is initialized to
> HASH_ALGO__LAST if the TPM algorithm is not supported. However there
> are places relying on the algorithm to be valid because it is accessed
> by hash_algo_name[].
>
> On 6.12.40 I observe the following read out-of-bounds in hash_algo_name:
> ==================================================================
> BUG: KASAN: global-out-of-bounds in create_securityfs_measurement_lists+0x396/0x440
> Read of size 8 at addr ffffffff83e18138 by task swapper/0/1
>
> CPU: 4 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.40 #3
> Call Trace:
> <TASK>
> dump_stack_lvl+0x61/0x90
> print_report+0xc4/0x580
> ? kasan_addr_to_slab+0x26/0x80
> ? create_securityfs_measurement_lists+0x396/0x440
> kasan_report+0xc2/0x100
> ? create_securityfs_measurement_lists+0x396/0x440
> create_securityfs_measurement_lists+0x396/0x440
> ima_fs_init+0xa3/0x300
> ima_init+0x7d/0xd0
> init_ima+0x28/0x100
> do_one_initcall+0xa6/0x3e0
> kernel_init_freeable+0x455/0x740
> kernel_init+0x24/0x1d0
> ret_from_fork+0x38/0x80
> ret_from_fork_asm+0x11/0x20
> </TASK>
>
> The buggy address belongs to the variable:
> hash_algo_name+0xb8/0x420
>
> Memory state around the buggy address:
> ffffffff83e18000: 00 01 f9 f9 f9 f9 f9 f9 00 01 f9 f9 f9 f9 f9 f9
> ffffffff83e18080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffffffff83e18100: 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 00 05 f9 f9
> ^
> ffffffff83e18180: f9 f9 f9 f9 00 00 00 00 00 00 00 04 f9 f9 f9 f9
> ffffffff83e18200: 00 00 00 00 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9
> ==================================================================
>
> Seems like the TPM chip supports sha3_256, which isn't yet in
> tpm_algorithms:
> tpm tpm0: TPM with unsupported bank algorithm 0x0027
>
> That's TPM_ALG_SHA3_256 == 0x0027 from "Trusted Platform Module 2.0
> Library Part 2: Structures", page 51 [1].
> See also the related U-Boot algorithms update [2].
>
> Thus solve the problem by creating a file name with "_tpm_alg_<ID>"
> postfix if the crypto algorithm isn't initialized.
>
> This is how it looks on the test machine (patch ported to v6.12 release):
> # ls -1 /sys/kernel/security/ima/
> ascii_runtime_measurements
> ascii_runtime_measurements_tpm_alg_27
> ascii_runtime_measurements_sha1
> ascii_runtime_measurements_sha256
> binary_runtime_measurements
> binary_runtime_measurements_tpm_alg_27
> binary_runtime_measurements_sha1
> binary_runtime_measurements_sha256
> policy
> runtime_measurements_count
> violations
>
> [1]: https://trustedcomputinggroup.org/wp-content/uploads/Trusted-Platform-Module-2.0-Library-Part-2-Version-184_pub.pdf
> [2]: https://lists.denx.de/pipermail/u-boot/2024-July/558835.html
>
> Fixes: 9fa8e7625008 ("ima: add crypto agility support for template-hash algorithm")
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> Cc: Enrico Bravi <enrico.bravi@polito.it>
> Cc: Silvia Sisinni <silvia.sisinni@polito.it>
> Cc: Roberto Sassu <roberto.sassu@huawei.com>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Tested-by: Roberto Sassu <roberto.sassu@huawei.com>
Thanks
Roberto
> ---
> Changes in v6:
> - Change subject now that securityfs files are created (Mimi Zohar)
> - Added a link to TCG document and the related U-Boot changes
> - Link to v5: https://lore.kernel.org/r/20260223-ima-oob-v5-1-91cc1064e767@arista.com
>
> Changes in v5:
> - Use lower-case for sysfs file name (as suggested-by Jonathan and Roberto)
> - Don't use email quotes for patch description (Roberto)
> - Re-word the patch description (suggested-by Roberto)
> - Link to v4: https://lore.kernel.org/r/20260127-ima-oob-v4-1-bf0cd7f9b4d4@arista.com
>
> Changes in v4:
> - Use ima_tpm_chip->allocated_banks[algo_idx].digest_size instead of hash_digest_size[algo]
> (Roberto Sassu)
> - Link to v3: https://lore.kernel.org/r/20260127-ima-oob-v3-1-1dd09f4c2a6a@arista.com
> Testing note: I test it on v6.12.40 kernel backport, which slightly differs as
> lookup_template_data_hash_algo() was yet present.
>
> Changes in v3:
> - Now fix the spelling *for real* (sorry, messed it up in v2)
> - Link to v2: https://lore.kernel.org/r/20260127-ima-oob-v2-1-f38a18c850cf@arista.com
>
> Changes in v2:
> - Instead of skipping unknown algorithms, add files under their TPM_ALG_ID (Roberto Sassu)
> - Fix spelling (Roberto Sassu)
> - Copy @stable on the fix
> - Link to v1: https://lore.kernel.org/r/20260127-ima-oob-v1-1-2d42f3418e57@arista.com
> ---
> security/integrity/ima/ima_fs.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 23d3a14b8ce3..ca4931a95098 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -398,16 +398,24 @@ static int __init create_securityfs_measurement_lists(void)
> char file_name[NAME_MAX + 1];
> struct dentry *dentry;
>
> - sprintf(file_name, "ascii_runtime_measurements_%s",
> - hash_algo_name[algo]);
> + if (algo == HASH_ALGO__LAST)
> + sprintf(file_name, "ascii_runtime_measurements_tpm_alg_%x",
> + ima_tpm_chip->allocated_banks[i].alg_id);
> + else
> + sprintf(file_name, "ascii_runtime_measurements_%s",
> + hash_algo_name[algo]);
> dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
> ima_dir, (void *)(uintptr_t)i,
> &ima_ascii_measurements_ops);
> if (IS_ERR(dentry))
> return PTR_ERR(dentry);
>
> - sprintf(file_name, "binary_runtime_measurements_%s",
> - hash_algo_name[algo]);
> + if (algo == HASH_ALGO__LAST)
> + sprintf(file_name, "binary_runtime_measurements_tpm_alg_%x",
> + ima_tpm_chip->allocated_banks[i].alg_id);
> + else
> + sprintf(file_name, "binary_runtime_measurements_%s",
> + hash_algo_name[algo]);
> dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
> ima_dir, (void *)(uintptr_t)i,
> &ima_measurements_ops);
>
> ---
> base-commit: 343f51842f4ed7143872f3aa116a214a5619a4b9
> change-id: 20260127-ima-oob-9fa83a634d7b
>
> Best regards,
^ permalink raw reply
* Re: [PATCH v4 06/17] kbuild: add stamp file for vmlinux BTF data
From: Thomas Weißschuh @ 2026-03-11 12:58 UTC (permalink / raw)
To: Eric Biggers
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260310213606.GD120274@quark>
On 2026-03-10 14:36:06-0700, Eric Biggers wrote:
> On Tue, Jan 13, 2026 at 01:28:50PM +0100, Thomas Weißschuh wrote:
> > The upcoming module hashes functionality will build the modules in
> > between the generation of the BTF data and the final link of vmlinux.
> > Having a dependency from the modules on vmlinux would make this
> > impossible as it would mean having a cyclic dependency.
> > Break this cyclic dependency by introducing a new target.
> >
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > scripts/Makefile.modfinal | 4 ++--
> > scripts/link-vmlinux.sh | 6 ++++++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> > index 149e12ff5700..adfef1e002a9 100644
> > --- a/scripts/Makefile.modfinal
> > +++ b/scripts/Makefile.modfinal
> > @@ -56,8 +56,8 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
> > printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> >
> > # Re-generate module BTFs if either module's .ko or vmlinux changed
> > -%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
> > - +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
> > +%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/.tmp_vmlinux_btf.stamp) FORCE
> > + +$(call if_changed_except,ld_ko_o,$(objtree)/.tmp_vmlinux_btf.stamp)
> > ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> > +$(if $(newer-prereqs),$(call cmd,btf_ko))
> > endif
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index 4ab44c73da4d..8c98f8645a5c 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -111,6 +111,7 @@ vmlinux_link()
> > gen_btf()
> > {
> > local btf_data=${1}.btf.o
> > + local btf_stamp=.tmp_vmlinux_btf.stamp
> >
> > info BTF "${btf_data}"
> > LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1}
> > @@ -131,6 +132,11 @@ gen_btf()
> > fi
> > printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none
> >
> > + info STAMP $btf_stamp
> > + if ! cmp --silent $btf_data $btf_stamp; then
> > + cp $btf_data $btf_stamp
> > + fi
This patch will be gone from the next revision of the series.
Making use of the recently introduced vmlinux.unstripped,
as suggested by Petr, removes any modifications to link-vmlinux.sh
and the issue that this patch tried to address.
> A "stamp file" is traditionally an empty file that is written when some
> build step has completed. The above code is instead copying the entire
> .tmp_vmlinux1.btf.o file (megabytes in size) to .tmp_vmlinux_btf.stamp.
The goal here was not to only have a reference timestamp, but
specifically the reference file contents.
Note: The duplicated vmlinux.unstripped in its current form is by far
larger than .tmp_vmlinux1.btf.o.
> So, it's not clear to me why the stamp file is needed at all, versus
> depending directly on .tmp_vmlinux1.btf.o.
>
> I guess 'make' doesn't know about the dependencies of
> .tmp_vmlinux1.btf.o. But the same is true of the stamp file, right? So
> either way, how would 'make' know to finish rebuilding the file before
> starting to execute the "Re-generate module BTFs" rule?
The problem was not the ordering, this is handled within link-vmlinux.sh.
IIRC originally without this patch even no-op rebuilds would end up
rebuilding the modules. Using .tmp_vmlinux1.btf.o may have worked too.
But in v4, the patch "kbuild: generate module BTF based on
vmlinux.unstripped" was added, which also solves this problem.
> Also, passing the long option '--silent' to 'cmp' creates a dependency
> on the GNU implementation of 'cmp', which isn't documented as a kernel
> build dependency. Probably better to use the short option '-s'.
Ack.
> Also, the stamp file isn't being deleted by 'make clean'. It looks like
> it would need to be added to cleanup() in link-vmlinux.sh.
Ack.
Thomas
^ permalink raw reply
* Re: [PATCH v4 09/17] module: Make module loading policy usable without MODULE_SIG
From: Thomas Weißschuh @ 2026-03-11 12:59 UTC (permalink / raw)
To: Eric Biggers
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260310220146.GE120274@quark>
On 2026-03-10 15:01:46-0700, Eric Biggers wrote:
> On Tue, Jan 13, 2026 at 01:28:53PM +0100, Thomas Weißschuh wrote:
> > The loading policy functionality will also be used by the hash-based
> > module validation. Split it out from CONFIG_MODULE_SIG so it is usable
> > by both.
> >
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > include/linux/module.h | 8 ++++----
> > kernel/module/Kconfig | 5 ++++-
> > kernel/module/main.c | 26 +++++++++++++++++++++++++-
> > kernel/module/signing.c | 21 ---------------------
> > 4 files changed, 33 insertions(+), 27 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index f288ca5cd95b..f9601cba47cd 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -444,7 +444,7 @@ struct module {
> > const u32 *gpl_crcs;
> > bool using_gplonly_symbols;
> >
> > -#ifdef CONFIG_MODULE_SIG
> > +#ifdef CONFIG_MODULE_SIG_POLICY
> > /* Signature was verified. */
> > bool sig_ok;
> > #endif
> [...]
> > +config MODULE_SIG_POLICY
> > + def_bool MODULE_SIG
>
> Maybe MODULE_AUTH_POLICY? Hash-based module authentication does not use
> signatures.
>
> This issue appears elsewhere in the code too. There are lots of places
> that still refer to module signatures or "sigs", when really module
> authentication is meant.
>
> I'm not sure how far you want to go with the renaming, but it's
> something to think about. It's confusing to use the term "signature" to
> mean something that is not a signature.
Ack. "authentication" is much better, I'll use that.
Thomas
^ permalink raw reply
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Thomas Weißschuh @ 2026-03-11 13:19 UTC (permalink / raw)
To: Eric Biggers
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260311011218.GA212983@quark>
On 2026-03-10 18:12:18-0700, Eric Biggers wrote:
> On Tue, Jan 13, 2026 at 01:28:59PM +0100, Thomas Weißschuh wrote:
> > The current signature-based module integrity checking has some drawbacks
> > in combination with reproducible builds. Either the module signing key
> > is generated at build time, which makes the build unreproducible, or a
> > static signing key is used, which precludes rebuilds by third parties
> > and makes the whole build and packaging process much more complicated.
>
> I think this actually undersells the feature.
(...)
> So I think this is how module authentication should have been done
> originally, and I'm glad to see this is finally being fixed.
Thanks, that is nice to hear.
> > +struct module_hashes_proof {
> > + __be32 pos;
> > + u8 hash_sigs[][MODULE_HASHES_HASH_SIZE];
> > +} __packed;
>
> Is the choice of big endian for consistency with struct
> module_signature? Little endian is the usual choice in new code.
Yes, it's for consistency. But I am fine with either way. Given that
this is essentially an internal ABI, we could always change it later.
> > diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
> > index a45ce3b24403..3b510651830d 100644
> > --- a/include/linux/module_signature.h
> > +++ b/include/linux/module_signature.h
> > @@ -18,6 +18,7 @@ enum pkey_id_type {
> > PKEY_ID_PGP, /* OpenPGP generated key ID */
> > PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
> > PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
> > + PKEY_ID_MERKLE, /* Merkle proof for modules */
>
> I recommend making the hash algorithm explicit:
>
> PKEY_ID_MERKLE_SHA256, /* SHA-256 merkle proof for modules */
>
> While I wouldn't encourage the addition of another hash algorithm
> (specifying one good algorithm for now is absolutely the right choice),
> if someone ever does need to add another one, we'd want them to be
> guided to simply introduce a new value of this enum rather than hack it
> in some other way.
The idea here was that this will only ever be used for module built as
part of the kernel build. So the actual implementation could change freely
without affecting anything.
But I don't have hard feelings about it.
> > +static void hash_entry(const void *left, const void *right, void *out)
>
> Byte arrays should use u8 instead of void
Ack.
> > diff --git a/scripts/modules-merkle-tree.c b/scripts/modules-merkle-tree.c
> [...]
>
> > +struct file_entry {
> > + char *name;
> > + unsigned int pos;
> > + unsigned char hash[EVP_MAX_MD_SIZE];
>
> Considering that the hash algorithm is fixed, EVP_MAX_MD_SIZE can be
> replaced with a tighter local definition:
Ack.
> #define MAX_HASH_SIZE 32
IMO it shouldn't even mention 'MAX', as there is only one hash
algorithm.
(...)
> > +{
> > + fprintf(stderr,
> > + "Usage: scripts/modules-merkle-tree <root definition>\n");
> > + exit(2);
>
> This should show both parameters, <root hash> <new suffix>
Ack.
> But they probably should be flipped to put the output second.
Ack.
> Though, is <new suffix> needed at all? It looks like it doesn't
> actually affect the output.
It will be required for compatibility with INSTALL_MOD_STRIP,
two patches later. I'll move this code into the later patch.
> > + hash_evp = EVP_get_digestbyname("sha256");
>
> EVP_sha256()
(...)
Ack to all other remarks.
Thomas
^ permalink raw reply
* [PATCH v3 1/3] ima: Remove ima_h_table structure
From: Roberto Sassu @ 2026-03-11 17:19 UTC (permalink / raw)
To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
From: Roberto Sassu <roberto.sassu@huawei.com>
With the upcoming change of dynamically allocating and replacing the hash
table, we would need to keep the counters for number of measurements
entries and violations.
Since anyway, those counters don't belong there, remove the ima_h_table
structure instead and move the counters and the hash table as a separate
variables.
Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Changelog:
v2:
- Not present in this version
v1:
- Not present in this version
---
security/integrity/ima/ima.h | 9 +++------
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_fs.c | 19 +++++++++----------
security/integrity/ima/ima_kexec.c | 2 +-
security/integrity/ima/ima_queue.c | 17 ++++++++++-------
5 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c38a9eb945b6..1f2c81ec0fba 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -298,12 +298,9 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
*/
extern spinlock_t ima_queue_lock;
-struct ima_h_table {
- atomic_long_t len; /* number of stored measurements in the list */
- atomic_long_t violations;
- struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
-};
-extern struct ima_h_table ima_htable;
+extern atomic_long_t ima_num_entries;
+extern atomic_long_t ima_num_violations;
+extern struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
static inline unsigned int ima_hash_key(u8 *digest)
{
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 0916f24f005f..122d127e108d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -146,7 +146,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
int result;
/* can overflow, only indicator */
- atomic_long_inc(&ima_htable.violations);
+ atomic_long_inc(&ima_num_violations);
result = ima_alloc_init_template(&event_data, &entry, NULL);
if (result < 0) {
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index ca4931a95098..aaa460d70ff7 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -38,8 +38,8 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
-static ssize_t ima_show_htable_value(char __user *buf, size_t count,
- loff_t *ppos, atomic_long_t *val)
+static ssize_t ima_show_counter(char __user *buf, size_t count, loff_t *ppos,
+ atomic_long_t *val)
{
char tmpbuf[32]; /* greater than largest 'long' string value */
ssize_t len;
@@ -48,15 +48,14 @@ static ssize_t ima_show_htable_value(char __user *buf, size_t count,
return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
}
-static ssize_t ima_show_htable_violations(struct file *filp,
- char __user *buf,
- size_t count, loff_t *ppos)
+static ssize_t ima_show_num_violations(struct file *filp, char __user *buf,
+ size_t count, loff_t *ppos)
{
- return ima_show_htable_value(buf, count, ppos, &ima_htable.violations);
+ return ima_show_counter(buf, count, ppos, &ima_num_violations);
}
-static const struct file_operations ima_htable_violations_ops = {
- .read = ima_show_htable_violations,
+static const struct file_operations ima_num_violations_ops = {
+ .read = ima_show_num_violations,
.llseek = generic_file_llseek,
};
@@ -64,7 +63,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,
char __user *buf,
size_t count, loff_t *ppos)
{
- return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
+ return ima_show_counter(buf, count, ppos, &ima_num_entries);
}
@@ -545,7 +544,7 @@ int __init ima_fs_init(void)
}
dentry = securityfs_create_file("violations", S_IRUSR | S_IRGRP,
- ima_dir, NULL, &ima_htable_violations_ops);
+ ima_dir, NULL, &ima_num_violations_ops);
if (IS_ERR(dentry)) {
ret = PTR_ERR(dentry);
goto out;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 36a34c54de58..5801649fbbef 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -43,7 +43,7 @@ void ima_measure_kexec_event(const char *event_name)
int n;
buf_size = ima_get_binary_runtime_size();
- len = atomic_long_read(&ima_htable.len);
+ len = atomic_long_read(&ima_num_entries);
n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
"kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 319522450854..4837fc6d9ada 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -32,11 +32,14 @@ static unsigned long binary_runtime_size;
static unsigned long binary_runtime_size = ULONG_MAX;
#endif
+/* num of stored meas. in the list */
+atomic_long_t ima_num_entries = ATOMIC_LONG_INIT(0);
+/* num of violations in the list */
+atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
+
/* key: inode (before secure-hashing a file) */
-struct ima_h_table ima_htable = {
- .len = ATOMIC_LONG_INIT(0),
- .violations = ATOMIC_LONG_INIT(0),
- .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
+struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE] = {
+ [0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
};
/* mutex protects atomicity of extending measurement list
@@ -61,7 +64,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
key = ima_hash_key(digest_value);
rcu_read_lock();
- hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
+ hlist_for_each_entry_rcu(qe, &ima_htable[key], hnext) {
rc = memcmp(qe->entry->digests[ima_hash_algo_idx].digest,
digest_value, hash_digest_size[ima_hash_algo]);
if ((rc == 0) && (qe->entry->pcr == pcr)) {
@@ -113,10 +116,10 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
INIT_LIST_HEAD(&qe->later);
list_add_tail_rcu(&qe->later, &ima_measurements);
- atomic_long_inc(&ima_htable.len);
+ atomic_long_inc(&ima_num_entries);
if (update_htable) {
key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
- hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
+ hlist_add_head_rcu(&qe->hnext, &ima_htable[key]);
}
if (binary_runtime_size != ULONG_MAX) {
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/3] ima: Replace static htable queue with dynamically allocated array
From: Roberto Sassu @ 2026-03-11 17:19 UTC (permalink / raw)
To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260311171956.2317781-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
The IMA hash table is a fixed-size array of hlist_head buckets:
struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
IMA_MEASURE_HTABLE_SIZE is (1 << IMA_HASH_BITS) = 1024 buckets, each a
struct hlist_head (one pointer, 8 bytes on 64-bit). That is 8 KiB allocated
in BSS for every kernel, regardless of whether IMA is ever used, and
regardless of how many measurements are actually made.
Replace the fixed-size array with a RCU-protected pointer to a dynamically
allocated array that is initialized in ima_init_htable(), which is called
from ima_init() during early boot. ima_init_htable() calls the static
function ima_alloc_replace_htable() which, other than initializing the hash
table the first time, can also hot-swap the existing hash table with a
blank one.
The allocation in ima_alloc_replace_htable() uses kcalloc() so the buckets
are zero-initialised (equivalent to HLIST_HEAD_INIT { .first = NULL }).
Callers of ima_alloc_replace_htable() must call synchronize_rcu() and free
the returned hash table.
Finally, access the hash table with rcu_dereference() in
ima_lookup_digest_entry() (reader side) and with
rcu_dereference_protected() in ima_add_digest_entry() (writer side).
No functional change: bucket count, hash function, and all locking remain
identical.
Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Changelog:
v2:
- Not present in this version
v1:
- Not present in this version
---
security/integrity/ima/ima.h | 3 +-
security/integrity/ima/ima_init.c | 5 +++
security/integrity/ima/ima_queue.c | 49 +++++++++++++++++++++++++++---
3 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1f2c81ec0fba..ccd037d49de7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -285,6 +285,7 @@ bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
int ima_restore_measurement_entry(struct ima_template_entry *entry);
int ima_restore_measurement_list(loff_t bufsize, void *buf);
int ima_measurements_show(struct seq_file *m, void *v);
+int __init ima_init_htable(void);
unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
@@ -300,7 +301,7 @@ extern spinlock_t ima_queue_lock;
extern atomic_long_t ima_num_entries;
extern atomic_long_t ima_num_violations;
-extern struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
+extern struct hlist_head __rcu *ima_htable;
static inline unsigned int ima_hash_key(u8 *digest)
{
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index a2f34f2d8ad7..7e0aa09a12e6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -140,6 +140,11 @@ int __init ima_init(void)
rc = ima_init_digests();
if (rc != 0)
return rc;
+
+ rc = ima_init_htable();
+ if (rc != 0)
+ return rc;
+
rc = ima_add_boot_aggregate(); /* boot aggregate must be first entry */
if (rc != 0)
return rc;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 4837fc6d9ada..2050b9d21e70 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -38,9 +38,7 @@ atomic_long_t ima_num_entries = ATOMIC_LONG_INIT(0);
atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
/* key: inode (before secure-hashing a file) */
-struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE] = {
- [0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
-};
+struct hlist_head __rcu *ima_htable;
/* mutex protects atomicity of extending measurement list
* and extending the TPM PCR aggregate. Since tpm_extend can take
@@ -54,17 +52,54 @@ static DEFINE_MUTEX(ima_extend_list_mutex);
*/
static bool ima_measurements_suspended;
+/* Callers must call synchronize_rcu() and free the hash table. */
+static struct hlist_head *ima_alloc_replace_htable(void)
+{
+ struct hlist_head *old_htable, *new_htable;
+
+ /* Initializing to zeros is equivalent to call HLIST_HEAD_INIT. */
+ new_htable = kcalloc(IMA_MEASURE_HTABLE_SIZE, sizeof(struct hlist_head),
+ GFP_KERNEL);
+ if (!new_htable)
+ return ERR_PTR(-ENOMEM);
+
+ old_htable = rcu_replace_pointer(ima_htable, new_htable,
+ lockdep_is_held(&ima_extend_list_mutex));
+
+ return old_htable;
+}
+
+int __init ima_init_htable(void)
+{
+ struct hlist_head *old_htable;
+
+ mutex_lock(&ima_extend_list_mutex);
+ old_htable = ima_alloc_replace_htable();
+ mutex_unlock(&ima_extend_list_mutex);
+
+ /* Synchronize_rcu() and kfree() not necessary, only for robustness. */
+ synchronize_rcu();
+
+ if (IS_ERR(old_htable))
+ return PTR_ERR(old_htable);
+
+ kfree(old_htable);
+ return 0;
+}
+
/* lookup up the digest value in the hash table, and return the entry */
static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
int pcr)
{
struct ima_queue_entry *qe, *ret = NULL;
+ struct hlist_head *htable;
unsigned int key;
int rc;
key = ima_hash_key(digest_value);
rcu_read_lock();
- hlist_for_each_entry_rcu(qe, &ima_htable[key], hnext) {
+ htable = rcu_dereference(ima_htable);
+ hlist_for_each_entry_rcu(qe, &htable[key], hnext) {
rc = memcmp(qe->entry->digests[ima_hash_algo_idx].digest,
digest_value, hash_digest_size[ima_hash_algo]);
if ((rc == 0) && (qe->entry->pcr == pcr)) {
@@ -104,6 +139,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
bool update_htable)
{
struct ima_queue_entry *qe;
+ struct hlist_head *htable;
unsigned int key;
qe = kmalloc_obj(*qe);
@@ -116,10 +152,13 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
INIT_LIST_HEAD(&qe->later);
list_add_tail_rcu(&qe->later, &ima_measurements);
+ htable = rcu_dereference_protected(ima_htable,
+ lockdep_is_held(&ima_extend_list_mutex));
+
atomic_long_inc(&ima_num_entries);
if (update_htable) {
key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
- hlist_add_head_rcu(&qe->hnext, &ima_htable[key]);
+ hlist_add_head_rcu(&qe->hnext, &htable[key]);
}
if (binary_runtime_size != ULONG_MAX) {
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/3] ima: Add support for staging measurements for deletion
From: Roberto Sassu @ 2026-03-11 17:19 UTC (permalink / raw)
To: corbet, skhan, zohar, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260311171956.2317781-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Introduce the ability of staging the IMA measurement list for deletion.
Staging means moving the current content of the measurement list to a
separate location, and allowing users to read and delete it. This causes
the measurement list to be atomically truncated before new measurements can
be added. Staging can be done only once at a time. In the event of kexec(),
staging is reverted and staged entries will be carried over to the new
kernel.
Staged measurements can be deleted entirely, or partially, with the
non-deleted ones added back to the IMA measurements list. This allows the
remote attestation agents to easily separate the measurements that where
verified (staged and deleted) from those that weren't due to the race
between taking a TPM quote and reading the measurements list.
User space is responsible to concatenate the staged IMA measurements list
portions (excluding the measurements added back to the IMA measurements
list) following the temporal order in which the operations were done,
together with the current measurement list. Then, it can send the collected
data to the remote verifiers.
The benefit of staging and deleting is the ability to free precious kernel
memory, in exchange of delegating user space to reconstruct the full
measurement list from the chunks. No trust needs to be given to user space,
since the integrity of the measurement list is protected by the TPM.
By default, staging the measurements list does not alter the hash table.
When staging and deleting are done, IMA is still able to detect collisions
on the staged and later deleted measurement entries, by keeping the entry
digests (only template data are freed).
However, since during the measurements list serialization only the SHA1
digest is passed, and since there are no template data to recalculate the
other digests from, the hash table is currently not populated with digests
from staged/deleted entries after kexec().
Introduce the new kernel option ima_flush_htable to decide whether or not
the digests of staged measurement entries are flushed from the hash table,
when they are deleted. Flushing the hash table is supported only when
deleting all the staged measurements, since in that case the old hash table
can be quickly swapped with a blank one (otherwise entries would have to be
removed one by one for partial deletion).
Then, introduce ascii_runtime_measurements_<algo>_staged and
binary_runtime_measurements_<algo>_staged interfaces to stage and delete
the measurements. Use 'echo A > <IMA interface>' and
'echo D > <IMA interface>' to respectively stage and delete the entire
measurements list. Use 'echo N > <IMA interface>', with N between 1 and
ULONG_MAX - 1, to delete the selected staged portion of the measurements
list.
The ima_measure_users counter (protected by the ima_measure_mutex mutex)
has been introduced to protect access to the measurements list and the
staged part. The open method of all the measurement interfaces has been
extended to allow only one writer at a time or, in alternative, multiple
readers. The write permission is used to stage and delete the measurements,
the read permission to read them. Write requires also the CAP_SYS_ADMIN
capability.
Finally, introduce the binary_lists enum and make binary_runtime_size
and ima_num_entries as arrays, to keep track of their values for the
current IMA measurements list (BINARY), current list plus staged
measurements (BINARY_STAGED) and the cumulative list since IMA
initialization (BINARY_FULL).
Use BINARY in ima_show_measurements_count(), BINARY_STAGED in
ima_add_kexec_buffer() and BINARY_FULL in ima_measure_kexec_event().
It should be noted that the BINARY_FULL counter is not passed through
kexec. Thus, the number of entries included in the kexec critical data
records refers to the entries since the previous kexec records.
Note: This code derives from the Alt-IMA Huawei project, whose license is
GPL-2.0 OR MIT.
Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Changelog
v2:
- Forbid partial deletion when flushing hash table (suggested by Mimi)
- Ignore ima_flush_htable if CONFIG_IMA_DISABLE_HTABLE is enabled
- BINARY_SIZE_* renamed to BINARY_* for better clarity
- Removed ima_measurements_staged_exist and testing list empty instead
- ima_queue_stage_trim() and ima_queue_delete_staged_trimmed() renamed to
ima_queue_stage() and ima_queue_delete_staged()
- New delete interval [1, ULONG_MAX - 1]
- Rename ima_measure_lock to ima_measure_mutex
- Move seq_open() and seq_release() outside the ima_measure_mutex lock
- Drop ima_measurements_staged_read() and use seq_read() instead
- Optimize create_securityfs_measurement_lists() changes
- New file name format with _staged suffix at the end of the file name
- Use _rcu list variant in ima_dump_measurement_list()
- Remove support for direct trimming and splice the remaining entries to
the active list (suggested by Mimi)
- Hot swap the hash table if flushing is requested
v1:
- Support for direct trimming without staging
- Support unstaging on kexec (requested by Gregory Lumen)
---
.../admin-guide/kernel-parameters.txt | 4 +
security/integrity/ima/ima.h | 17 +-
security/integrity/ima/ima_fs.c | 266 ++++++++++++++++--
security/integrity/ima/ima_kexec.c | 43 ++-
security/integrity/ima/ima_queue.c | 205 +++++++++++++-
5 files changed, 484 insertions(+), 51 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cb850e5290c2..7a377812aa0a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2345,6 +2345,10 @@ Kernel parameters
Use the canonical format for the binary runtime
measurements, instead of host native format.
+ ima_flush_htable [IMA]
+ Flush the IMA hash table when deleting all the
+ staged measurement entries.
+
ima_hash= [IMA]
Format: { md5 | sha1 | rmd160 | sha256 | sha384
| sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ccd037d49de7..e8aaf1e62139 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -28,6 +28,15 @@ enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
+/*
+ * BINARY: current binary measurements list
+ * BINARY_STAGED: current binary measurements list + staged entries
+ * BINARY_FULL: binary measurements list since IMA init (lost after kexec)
+ */
+enum binary_lists {
+ BINARY, BINARY_STAGED, BINARY_FULL, BINARY__LAST
+};
+
/* digest size for IMA, fits SHA1 or MD5 */
#define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
#define IMA_EVENT_NAME_LEN_MAX 255
@@ -118,6 +127,7 @@ struct ima_queue_entry {
struct ima_template_entry *entry;
};
extern struct list_head ima_measurements; /* list of all measurements */
+extern struct list_head ima_measurements_staged; /* list of staged meas. */
/* Some details preceding the binary serialized measurement list */
struct ima_kexec_hdr {
@@ -282,11 +292,13 @@ struct ima_template_desc *ima_template_desc_current(void);
struct ima_template_desc *ima_template_desc_buf(void);
struct ima_template_desc *lookup_template_desc(const char *name);
bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
+int ima_queue_stage(void);
+int ima_queue_delete_staged(unsigned long req_value);
int ima_restore_measurement_entry(struct ima_template_entry *entry);
int ima_restore_measurement_list(loff_t bufsize, void *buf);
int ima_measurements_show(struct seq_file *m, void *v);
int __init ima_init_htable(void);
-unsigned long ima_get_binary_runtime_size(void);
+unsigned long ima_get_binary_runtime_size(enum binary_lists binary_list);
int ima_init_template(void);
void ima_init_template_list(void);
int __init ima_init_digests(void);
@@ -299,9 +311,10 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
*/
extern spinlock_t ima_queue_lock;
-extern atomic_long_t ima_num_entries;
+extern atomic_long_t ima_num_entries[BINARY__LAST];
extern atomic_long_t ima_num_violations;
extern struct hlist_head __rcu *ima_htable;
+extern struct mutex ima_extend_list_mutex;
static inline unsigned int ima_hash_key(u8 *digest)
{
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index aaa460d70ff7..cf85b0892275 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -24,7 +24,17 @@
#include "ima.h"
+/*
+ * Requests:
+ * 'A\n': stage the entire measurements list
+ * 'D\n': delete all staged measurements
+ * '[1, ULONG_MAX - 1]\n' delete N measurements entries and unstage the rest
+ */
+#define STAGED_REQ_LENGTH 21
+
static DEFINE_MUTEX(ima_write_mutex);
+static DEFINE_MUTEX(ima_measure_mutex);
+static long ima_measure_users;
bool ima_canonical_fmt;
static int __init default_canonical_fmt_setup(char *str)
@@ -63,7 +73,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,
char __user *buf,
size_t count, loff_t *ppos)
{
- return ima_show_counter(buf, count, ppos, &ima_num_entries);
+ return ima_show_counter(buf, count, ppos, &ima_num_entries[BINARY]);
}
@@ -73,14 +83,15 @@ static const struct file_operations ima_measurements_count_ops = {
};
/* returns pointer to hlist_node */
-static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
+static void *_ima_measurements_start(struct seq_file *m, loff_t *pos,
+ struct list_head *head)
{
loff_t l = *pos;
struct ima_queue_entry *qe;
/* we need a lock since pos could point beyond last element */
rcu_read_lock();
- list_for_each_entry_rcu(qe, &ima_measurements, later) {
+ list_for_each_entry_rcu(qe, head, later) {
if (!l--) {
rcu_read_unlock();
return qe;
@@ -90,7 +101,18 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
return NULL;
}
-static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
+static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
+{
+ return _ima_measurements_start(m, pos, &ima_measurements);
+}
+
+static void *ima_measurements_staged_start(struct seq_file *m, loff_t *pos)
+{
+ return _ima_measurements_start(m, pos, &ima_measurements_staged);
+}
+
+static void *_ima_measurements_next(struct seq_file *m, void *v, loff_t *pos,
+ struct list_head *head)
{
struct ima_queue_entry *qe = v;
@@ -102,7 +124,18 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
rcu_read_unlock();
(*pos)++;
- return (&qe->later == &ima_measurements) ? NULL : qe;
+ return (&qe->later == head) ? NULL : qe;
+}
+
+static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
+{
+ return _ima_measurements_next(m, v, pos, &ima_measurements);
+}
+
+static void *ima_measurements_staged_next(struct seq_file *m, void *v,
+ loff_t *pos)
+{
+ return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
}
static void ima_measurements_stop(struct seq_file *m, void *v)
@@ -198,16 +231,145 @@ static const struct seq_operations ima_measurments_seqops = {
.show = ima_measurements_show
};
+static int ima_measure_lock(bool write)
+{
+ mutex_lock(&ima_measure_mutex);
+ if ((write && ima_measure_users != 0) ||
+ (!write && ima_measure_users < 0)) {
+ mutex_unlock(&ima_measure_mutex);
+ return -EBUSY;
+ }
+
+ if (write)
+ ima_measure_users--;
+ else
+ ima_measure_users++;
+ mutex_unlock(&ima_measure_mutex);
+ return 0;
+}
+
+static void ima_measure_unlock(bool write)
+{
+ mutex_lock(&ima_measure_mutex);
+ if (write)
+ ima_measure_users++;
+ else
+ ima_measure_users--;
+ mutex_unlock(&ima_measure_mutex);
+}
+
+static int _ima_measurements_open(struct inode *inode, struct file *file,
+ const struct seq_operations *seq_ops)
+{
+ bool write = (file->f_mode & FMODE_WRITE);
+ int ret;
+
+ if (write && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ ret = ima_measure_lock(write);
+ if (ret < 0)
+ return ret;
+
+ ret = seq_open(file, seq_ops);
+ if (ret < 0)
+ ima_measure_unlock(write);
+
+ return ret;
+}
+
static int ima_measurements_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &ima_measurments_seqops);
+ return _ima_measurements_open(inode, file, &ima_measurments_seqops);
+}
+
+static int ima_measurements_release(struct inode *inode, struct file *file)
+{
+ bool write = (file->f_mode & FMODE_WRITE);
+ int ret;
+
+ ret = seq_release(inode, file);
+
+ ima_measure_unlock(write);
+
+ return ret;
}
static const struct file_operations ima_measurements_ops = {
.open = ima_measurements_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = ima_measurements_release,
+};
+
+static const struct seq_operations ima_measurments_staged_seqops = {
+ .start = ima_measurements_staged_start,
+ .next = ima_measurements_staged_next,
+ .stop = ima_measurements_stop,
+ .show = ima_measurements_show
+};
+
+static int ima_measurements_staged_open(struct inode *inode, struct file *file)
+{
+ return _ima_measurements_open(inode, file,
+ &ima_measurments_staged_seqops);
+}
+
+static ssize_t ima_measurements_staged_write(struct file *file,
+ const char __user *buf,
+ size_t datalen, loff_t *ppos)
+{
+ char req[STAGED_REQ_LENGTH];
+ unsigned long req_value;
+ int ret;
+
+ if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
+ return -EINVAL;
+
+ if (copy_from_user(req, buf, datalen) != 0)
+ return -EFAULT;
+
+ if (req[datalen - 1] != '\n')
+ return -EINVAL;
+
+ req[datalen - 1] = '\0';
+
+ switch (req[0]) {
+ case 'A':
+ if (datalen != 2)
+ return -EINVAL;
+
+ ret = ima_queue_stage();
+ break;
+ case 'D':
+ if (datalen != 2)
+ return -EINVAL;
+
+ ret = ima_queue_delete_staged(ULONG_MAX);
+ break;
+ default:
+ ret = kstrtoul(req, 10, &req_value);
+ if (ret < 0)
+ return ret;
+
+ if (req_value == ULONG_MAX)
+ return -ERANGE;
+
+ ret = ima_queue_delete_staged(req_value);
+ }
+
+ if (ret < 0)
+ return ret;
+
+ return datalen;
+}
+
+static const struct file_operations ima_measurements_staged_ops = {
+ .open = ima_measurements_staged_open,
+ .read = seq_read,
+ .write = ima_measurements_staged_write,
+ .llseek = seq_lseek,
+ .release = ima_measurements_release,
};
void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
@@ -272,14 +434,37 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &ima_ascii_measurements_seqops);
+ return _ima_measurements_open(inode, file,
+ &ima_ascii_measurements_seqops);
}
static const struct file_operations ima_ascii_measurements_ops = {
.open = ima_ascii_measurements_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = ima_measurements_release,
+};
+
+static const struct seq_operations ima_ascii_measurements_staged_seqops = {
+ .start = ima_measurements_staged_start,
+ .next = ima_measurements_staged_next,
+ .stop = ima_measurements_stop,
+ .show = ima_ascii_measurements_show
+};
+
+static int ima_ascii_measurements_staged_open(struct inode *inode,
+ struct file *file)
+{
+ return _ima_measurements_open(inode, file,
+ &ima_ascii_measurements_staged_seqops);
+}
+
+static const struct file_operations ima_ascii_measurements_staged_ops = {
+ .open = ima_ascii_measurements_staged_open,
+ .read = seq_read,
+ .write = ima_measurements_staged_write,
+ .llseek = seq_lseek,
+ .release = ima_measurements_release,
};
static ssize_t ima_read_policy(char *path)
@@ -385,10 +570,21 @@ static const struct seq_operations ima_policy_seqops = {
};
#endif
-static int __init create_securityfs_measurement_lists(void)
+static int __init create_securityfs_measurement_lists(bool staging)
{
+ const struct file_operations *ascii_ops = &ima_ascii_measurements_ops;
+ const struct file_operations *binary_ops = &ima_measurements_ops;
+ mode_t permissions = S_IRUSR | S_IRGRP;
+ const char *file_suffix = "";
int count = NR_BANKS(ima_tpm_chip);
+ if (staging) {
+ ascii_ops = &ima_ascii_measurements_staged_ops;
+ binary_ops = &ima_measurements_staged_ops;
+ file_suffix = "_staged";
+ permissions |= S_IWUSR | S_IWGRP;
+ }
+
if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
count++;
@@ -398,26 +594,33 @@ static int __init create_securityfs_measurement_lists(void)
struct dentry *dentry;
if (algo == HASH_ALGO__LAST)
- sprintf(file_name, "ascii_runtime_measurements_tpm_alg_%x",
- ima_tpm_chip->allocated_banks[i].alg_id);
+ snprintf(file_name, sizeof(file_name),
+ "ascii_runtime_measurements_tpm_alg_%x%s",
+ ima_tpm_chip->allocated_banks[i].alg_id,
+ file_suffix);
else
- sprintf(file_name, "ascii_runtime_measurements_%s",
- hash_algo_name[algo]);
- dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
+ snprintf(file_name, sizeof(file_name),
+ "ascii_runtime_measurements_%s%s",
+ hash_algo_name[algo], file_suffix);
+ dentry = securityfs_create_file(file_name, permissions,
ima_dir, (void *)(uintptr_t)i,
- &ima_ascii_measurements_ops);
+ ascii_ops);
if (IS_ERR(dentry))
return PTR_ERR(dentry);
if (algo == HASH_ALGO__LAST)
- sprintf(file_name, "binary_runtime_measurements_tpm_alg_%x",
- ima_tpm_chip->allocated_banks[i].alg_id);
+ snprintf(file_name, sizeof(file_name),
+ "binary_runtime_measurements_tpm_alg_%x%s",
+ ima_tpm_chip->allocated_banks[i].alg_id,
+ file_suffix);
else
- sprintf(file_name, "binary_runtime_measurements_%s",
- hash_algo_name[algo]);
- dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
+ snprintf(file_name, sizeof(file_name),
+ "binary_runtime_measurements_%s%s",
+ hash_algo_name[algo], file_suffix);
+
+ dentry = securityfs_create_file(file_name, permissions,
ima_dir, (void *)(uintptr_t)i,
- &ima_measurements_ops);
+ binary_ops);
if (IS_ERR(dentry))
return PTR_ERR(dentry);
}
@@ -517,7 +720,10 @@ int __init ima_fs_init(void)
goto out;
}
- ret = create_securityfs_measurement_lists();
+ ret = create_securityfs_measurement_lists(false);
+ if (ret == 0)
+ ret = create_securityfs_measurement_lists(true);
+
if (ret != 0)
goto out;
@@ -535,6 +741,20 @@ int __init ima_fs_init(void)
goto out;
}
+ dentry = securityfs_create_symlink("binary_runtime_measurements_staged",
+ ima_dir, "binary_runtime_measurements_sha1_staged", NULL);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ dentry = securityfs_create_symlink("ascii_runtime_measurements_staged",
+ ima_dir, "ascii_runtime_measurements_sha1_staged", NULL);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
dentry = securityfs_create_file("runtime_measurements_count",
S_IRUSR | S_IRGRP, ima_dir, NULL,
&ima_measurements_count_ops);
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 5801649fbbef..70ee3a039df2 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -42,8 +42,8 @@ void ima_measure_kexec_event(const char *event_name)
long len;
int n;
- buf_size = ima_get_binary_runtime_size();
- len = atomic_long_read(&ima_num_entries);
+ buf_size = ima_get_binary_runtime_size(BINARY_FULL);
+ len = atomic_long_read(&ima_num_entries[BINARY_FULL]);
n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
"kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
@@ -80,6 +80,17 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
return 0;
}
+static int ima_dump_measurement(struct ima_kexec_hdr *khdr,
+ struct ima_queue_entry *qe)
+{
+ if (ima_kexec_file.count >= ima_kexec_file.size)
+ return -EINVAL;
+
+ khdr->count++;
+ ima_measurements_show(&ima_kexec_file, qe);
+ return 0;
+}
+
static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
unsigned long segment_size)
{
@@ -95,17 +106,26 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
memset(&khdr, 0, sizeof(khdr));
khdr.version = 1;
- /* This is an append-only list, no need to hold the RCU read lock */
- list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
- if (ima_kexec_file.count < ima_kexec_file.size) {
- khdr.count++;
- ima_measurements_show(&ima_kexec_file, qe);
- } else {
- ret = -EINVAL;
+ /* It can race with ima_queue_stage() and ima_queue_delete_staged(). */
+ mutex_lock(&ima_extend_list_mutex);
+
+ list_for_each_entry_rcu(qe, &ima_measurements_staged, later,
+ lockdep_is_held(&ima_extend_list_mutex)) {
+ ret = ima_dump_measurement(&khdr, qe);
+ if (ret < 0)
break;
- }
}
+ list_for_each_entry_rcu(qe, &ima_measurements, later,
+ lockdep_is_held(&ima_extend_list_mutex)) {
+ if (!ret)
+ ret = ima_dump_measurement(&khdr, qe);
+ if (ret < 0)
+ break;
+ }
+
+ mutex_unlock(&ima_extend_list_mutex);
+
/*
* fill in reserved space with some buffer details
* (eg. version, buffer size, number of measurements)
@@ -159,7 +179,8 @@ void ima_add_kexec_buffer(struct kimage *image)
else
extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
- binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
+ binary_runtime_size = ima_get_binary_runtime_size(BINARY_STAGED) +
+ extra_memory;
if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
kexec_segment_size = ULONG_MAX;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 2050b9d21e70..08cd60fa959e 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -22,29 +22,48 @@
#define AUDIT_CAUSE_LEN_MAX 32
+bool ima_flush_htable;
+static int __init ima_flush_htable_setup(char *str)
+{
+ if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
+ pr_warn("Hash table not enabled, ignoring request to flush\n");
+ return 1;
+ }
+
+ ima_flush_htable = true;
+ return 1;
+}
+__setup("ima_flush_htable", ima_flush_htable_setup);
+
/* pre-allocated array of tpm_digest structures to extend a PCR */
static struct tpm_digest *digests;
LIST_HEAD(ima_measurements); /* list of all measurements */
+LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
#ifdef CONFIG_IMA_KEXEC
-static unsigned long binary_runtime_size;
+static unsigned long binary_runtime_size[BINARY__LAST];
#else
-static unsigned long binary_runtime_size = ULONG_MAX;
+static unsigned long binary_runtime_size[BINARY__LAST] = {
+ [0 ... BINARY__LAST - 1] = ULONG_MAX
+};
#endif
/* num of stored meas. in the list */
-atomic_long_t ima_num_entries = ATOMIC_LONG_INIT(0);
+atomic_long_t ima_num_entries[BINARY__LAST] = {
+ [0 ... BINARY__LAST - 1] = ATOMIC_LONG_INIT(0)
+};
+
/* num of violations in the list */
atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
/* key: inode (before secure-hashing a file) */
struct hlist_head __rcu *ima_htable;
-/* mutex protects atomicity of extending measurement list
+/* mutex protects atomicity of extending and staging measurement list
* and extending the TPM PCR aggregate. Since tpm_extend can take
* long (and the tpm driver uses a mutex), we can't use the spinlock.
*/
-static DEFINE_MUTEX(ima_extend_list_mutex);
+DEFINE_MUTEX(ima_extend_list_mutex);
/*
* Used internally by the kernel to suspend measurements.
@@ -140,7 +159,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
{
struct ima_queue_entry *qe;
struct hlist_head *htable;
- unsigned int key;
+ unsigned int key, i;
qe = kmalloc_obj(*qe);
if (qe == NULL) {
@@ -155,19 +174,25 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
htable = rcu_dereference_protected(ima_htable,
lockdep_is_held(&ima_extend_list_mutex));
- atomic_long_inc(&ima_num_entries);
+ for (i = 0; i < BINARY__LAST; i++)
+ atomic_long_inc(&ima_num_entries[i]);
+
if (update_htable) {
key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
hlist_add_head_rcu(&qe->hnext, &htable[key]);
}
- if (binary_runtime_size != ULONG_MAX) {
- int size;
+ for (i = 0; i < BINARY__LAST; i++) {
+ if (binary_runtime_size[i] != ULONG_MAX) {
+ int size;
- size = get_binary_runtime_size(entry);
- binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
- binary_runtime_size + size : ULONG_MAX;
+ size = get_binary_runtime_size(entry);
+ binary_runtime_size[i] =
+ (binary_runtime_size[i] < ULONG_MAX - size) ?
+ binary_runtime_size[i] + size : ULONG_MAX;
+ }
}
+
return 0;
}
@@ -176,12 +201,18 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
* entire binary_runtime_measurement list, including the ima_kexec_hdr
* structure.
*/
-unsigned long ima_get_binary_runtime_size(void)
+unsigned long ima_get_binary_runtime_size(enum binary_lists binary_list)
{
- if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
+ unsigned long val;
+
+ mutex_lock(&ima_extend_list_mutex);
+ val = binary_runtime_size[binary_list];
+ mutex_unlock(&ima_extend_list_mutex);
+
+ if (val >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
return ULONG_MAX;
else
- return binary_runtime_size + sizeof(struct ima_kexec_hdr);
+ return val + sizeof(struct ima_kexec_hdr);
}
static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
@@ -262,6 +293,150 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
return result;
}
+int ima_queue_stage(void)
+{
+ int ret = 0;
+
+ mutex_lock(&ima_extend_list_mutex);
+ if (!list_empty(&ima_measurements_staged)) {
+ ret = -EEXIST;
+ goto out_unlock;
+ }
+
+ if (list_empty(&ima_measurements)) {
+ ret = -ENOENT;
+ goto out_unlock;
+ }
+
+ list_replace(&ima_measurements, &ima_measurements_staged);
+ INIT_LIST_HEAD(&ima_measurements);
+ atomic_long_set(&ima_num_entries[BINARY], 0);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ binary_runtime_size[BINARY] = 0;
+out_unlock:
+ mutex_unlock(&ima_extend_list_mutex);
+ return ret;
+}
+
+int ima_queue_delete_staged(unsigned long req_value)
+{
+ unsigned long req_value_copy = req_value;
+ unsigned long size_to_remove = 0, num_to_remove = 0;
+ struct ima_queue_entry *qe, *qe_tmp;
+ struct list_head *cut_pos = NULL;
+ LIST_HEAD(ima_measurements_trim);
+ struct hlist_head *old_queue = NULL;
+ unsigned int i;
+
+ if (req_value == 0) {
+ pr_err("Must delete at least one entry\n");
+ return -EINVAL;
+ }
+
+ if (req_value < ULONG_MAX && ima_flush_htable) {
+ pr_err("Deleting staged N measurements not supported when flushing the hash table is requested\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Safe walk (no concurrent write), not under ima_extend_list_mutex
+ * for performance reasons.
+ */
+ list_for_each_entry(qe, &ima_measurements_staged, later) {
+ size_to_remove += get_binary_runtime_size(qe->entry);
+ num_to_remove++;
+
+ if (req_value < ULONG_MAX && --req_value_copy == 0) {
+ /* qe->later always points to a valid list entry. */
+ cut_pos = &qe->later;
+ break;
+ }
+ }
+
+ if (req_value < ULONG_MAX && req_value_copy > 0)
+ return -ENOENT;
+
+ mutex_lock(&ima_extend_list_mutex);
+ if (list_empty(&ima_measurements_staged)) {
+ mutex_unlock(&ima_extend_list_mutex);
+ return -ENOENT;
+ }
+
+ if (req_value < ULONG_MAX) {
+ /*
+ * ima_dump_measurement_list() does not modify the list,
+ * cut_pos remains the same even if it was computed before
+ * the lock.
+ */
+ __list_cut_position(&ima_measurements_trim,
+ &ima_measurements_staged, cut_pos);
+ } else {
+ list_replace(&ima_measurements_staged, &ima_measurements_trim);
+ INIT_LIST_HEAD(&ima_measurements_staged);
+ }
+
+ atomic_long_sub(num_to_remove, &ima_num_entries[BINARY_STAGED]);
+ atomic_long_add(atomic_long_read(&ima_num_entries[BINARY_STAGED]),
+ &ima_num_entries[BINARY]);
+ atomic_long_set(&ima_num_entries[BINARY_STAGED],
+ atomic_long_read(&ima_num_entries[BINARY]));
+
+ if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
+ binary_runtime_size[BINARY_STAGED] -= size_to_remove;
+ binary_runtime_size[BINARY] +=
+ binary_runtime_size[BINARY_STAGED];
+ binary_runtime_size[BINARY_STAGED] =
+ binary_runtime_size[BINARY];
+ }
+
+ if (ima_flush_htable) {
+ old_queue = ima_alloc_replace_htable();
+ if (IS_ERR(old_queue)) {
+ mutex_unlock(&ima_extend_list_mutex);
+ return PTR_ERR(old_queue);
+ }
+ }
+
+ /*
+ * Splice (prepend) any remaining non-deleted staged entries to the
+ * active list (RCU not needed, there cannot be concurrent readers).
+ */
+ list_splice(&ima_measurements_staged, &ima_measurements);
+ INIT_LIST_HEAD(&ima_measurements_staged);
+ mutex_unlock(&ima_extend_list_mutex);
+
+ if (ima_flush_htable) {
+ synchronize_rcu();
+ kfree(old_queue);
+ }
+
+ list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_trim, later) {
+ /*
+ * Safe to free template_data here without synchronize_rcu()
+ * because the only htable reader, ima_lookup_digest_entry(),
+ * accesses only entry->digests, not template_data. If new
+ * htable readers are added that access template_data, a
+ * synchronize_rcu() is required here.
+ */
+ for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
+ kfree(qe->entry->template_data[i].data);
+ qe->entry->template_data[i].data = NULL;
+ qe->entry->template_data[i].len = 0;
+ }
+
+ list_del(&qe->later);
+
+ /* No leak if !ima_flush_htable, referenced by ima_htable. */
+ if (ima_flush_htable) {
+ kfree(qe->entry->digests);
+ kfree(qe->entry);
+ kfree(qe);
+ }
+ }
+
+ return 0;
+}
+
int ima_restore_measurement_entry(struct ima_template_entry *entry)
{
int result = 0;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Eric Biggers @ 2026-03-11 21:14 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <5726fc65-7d24-4353-b341-81b785f2575c@t-8ch.de>
On Wed, Mar 11, 2026 at 02:19:02PM +0100, Thomas Weißschuh wrote:
> > > diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
> > > index a45ce3b24403..3b510651830d 100644
> > > --- a/include/linux/module_signature.h
> > > +++ b/include/linux/module_signature.h
> > > @@ -18,6 +18,7 @@ enum pkey_id_type {
> > > PKEY_ID_PGP, /* OpenPGP generated key ID */
> > > PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
> > > PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
> > > + PKEY_ID_MERKLE, /* Merkle proof for modules */
> >
> > I recommend making the hash algorithm explicit:
> >
> > PKEY_ID_MERKLE_SHA256, /* SHA-256 merkle proof for modules */
> >
> > While I wouldn't encourage the addition of another hash algorithm
> > (specifying one good algorithm for now is absolutely the right choice),
> > if someone ever does need to add another one, we'd want them to be
> > guided to simply introduce a new value of this enum rather than hack it
> > in some other way.
>
> The idea here was that this will only ever be used for module built as
> part of the kernel build. So the actual implementation could change freely
> without affecting anything.
>
> But I don't have hard feelings about it.
Ah, okay. That's even better then: if someone adds another algorithm it
would simply be a kconfig option.
It seems 'struct module_signature' itself is intended to be a stable
ABI, though. So I think there's an opportunity for confusion here. It
might be worth leaving a note somewhere that the format of the
PKEY_ID_MERKLE portion of the struct does not need to be kept stable and
can freely change in each kernel build.
- Eric
^ permalink raw reply
* [PATCH] ima: remove buggy support for asynchronous hashes
From: Eric Biggers @ 2026-03-12 5:39 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin
Cc: linux-crypto, linux-kernel, Eric Biggers
IMA computes hashes using the crypto_shash or crypto_ahash API. The
latter is used only when ima.ahash_minsize is set on the command line,
and its purpose is ostensibly to make the hash computation faster.
However, going off the CPU to a crypto engine and back again is actually
quite slow, especially compared with the acceleration that is built into
modern CPUs and the kernel now enables by default for most algorithms.
Typical performance results for SHA-256 on a modern platform can be
found at https://lore.kernel.org/linux-crypto/20250615184638.GA1480@sol/
Partly for this reason, several other kernel subsystems have already
dropped support for the crypto_ahash API.
The other problem with crypto_ahash is that bugs are also common, not
just in the underlying drivers, but also in the code using it, since it
is very difficult to use correctly. Just from a quick review, here are
some of the bugs I noticed in IMA's ahash code:
- [Use after free] ima_alloc_atfm() isn't thread-safe and can trigger a
use-after-free if multiple threads try to initialize the global
ima_ahash_tfm at the same time.
- [Deadlock] If only one buffer is allocated and there is an error
reading from the file, then ahash_wait() is executed twice, causing a
deadlock in wait_for_completion().
- [Crash or incorrect hash computed] calc_buffer_ahash_atfm() is
sometimes passed stack buffers which can be vmalloc addresses, but it
puts them in a scatterlist assuming they are linear addresses. This
causes the hashing to be done on the wrong physical address.
- [Truncation to 32-bit length] ima_alloc_pages() incorrectly assumes an
loff_t value fits in an unsigned long. calc_buffer_ahash_atfm()
incorrectly assumes that a loff_t value fits in an unsigned int.
So, not exactly a great track record so far, even disregarding driver
bugs which are an even larger problem. Fortunately, in practice it's
unlikely that many users are actually setting the ima.ahash_minsize
kernel command-line parameter which enables this code. However, given
that this code is almost certainly no longer useful (if it ever was),
let's just remove it instead of attempting to fix all these issues.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
.../admin-guide/kernel-parameters.txt | 17 -
security/integrity/ima/ima_crypto.c | 382 +-----------------
2 files changed, 9 insertions(+), 390 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 55ffc0f8858a..a6e75f0a293b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2393,27 +2393,10 @@ Kernel parameters
ima_template_fmt=
[IMA] Define a custom template format.
Format: { "field1|...|fieldN" }
- ima.ahash_minsize= [IMA] Minimum file size for asynchronous hash usage
- Format: <min_file_size>
- Set the minimal file size for using asynchronous hash.
- If left unspecified, ahash usage is disabled.
-
- ahash performance varies for different data sizes on
- different crypto accelerators. This option can be used
- to achieve the best performance for a particular HW.
-
- ima.ahash_bufsize= [IMA] Asynchronous hash buffer size
- Format: <bufsize>
- Set hashing buffer size. Default: 4k.
-
- ahash performance varies for different chunk sizes on
- different crypto accelerators. This option can be used
- to achieve best performance for particular HW.
-
ima= [IMA] Enable or disable IMA
Format: { "off" | "on" }
Default: "on"
Note that disabling IMA is limited to kdump kernel.
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index aff61643415d..d43ed65c8e07 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -9,55 +9,19 @@
* File: ima_crypto.c
* Calculates md5/sha1 file hash, template hash, boot-aggreate hash
*/
#include <linux/kernel.h>
-#include <linux/moduleparam.h>
-#include <linux/ratelimit.h>
#include <linux/file.h>
#include <linux/crypto.h>
-#include <linux/scatterlist.h>
#include <linux/err.h>
#include <linux/slab.h>
#include <crypto/hash.h>
#include "ima.h"
-/* minimum file size for ahash use */
-static unsigned long ima_ahash_minsize;
-module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
-MODULE_PARM_DESC(ahash_minsize, "Minimum file size for ahash use");
-
-/* default is 0 - 1 page. */
-static int ima_maxorder;
-static unsigned int ima_bufsize = PAGE_SIZE;
-
-static int param_set_bufsize(const char *val, const struct kernel_param *kp)
-{
- unsigned long long size;
- int order;
-
- size = memparse(val, NULL);
- order = get_order(size);
- if (order > MAX_PAGE_ORDER)
- return -EINVAL;
- ima_maxorder = order;
- ima_bufsize = PAGE_SIZE << order;
- return 0;
-}
-
-static const struct kernel_param_ops param_ops_bufsize = {
- .set = param_set_bufsize,
- .get = param_get_uint,
-};
-#define param_check_bufsize(name, p) __param_check(name, p, unsigned int)
-
-module_param_named(ahash_bufsize, ima_bufsize, bufsize, 0644);
-MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size");
-
static struct crypto_shash *ima_shash_tfm;
-static struct crypto_ahash *ima_ahash_tfm;
int ima_sha1_idx __ro_after_init;
int ima_hash_algo_idx __ro_after_init;
/*
* Additional number of slots reserved, as needed, for SHA1
@@ -218,238 +182,10 @@ static void ima_free_tfm(struct crypto_shash *tfm)
return;
crypto_free_shash(tfm);
}
-/**
- * ima_alloc_pages() - Allocate contiguous pages.
- * @max_size: Maximum amount of memory to allocate.
- * @allocated_size: Returned size of actual allocation.
- * @last_warn: Should the min_size allocation warn or not.
- *
- * Tries to do opportunistic allocation for memory first trying to allocate
- * max_size amount of memory and then splitting that until zero order is
- * reached. Allocation is tried without generating allocation warnings unless
- * last_warn is set. Last_warn set affects only last allocation of zero order.
- *
- * By default, ima_maxorder is 0 and it is equivalent to kmalloc(GFP_KERNEL)
- *
- * Return pointer to allocated memory, or NULL on failure.
- */
-static void *ima_alloc_pages(loff_t max_size, size_t *allocated_size,
- int last_warn)
-{
- void *ptr;
- int order = ima_maxorder;
- gfp_t gfp_mask = __GFP_RECLAIM | __GFP_NOWARN | __GFP_NORETRY;
-
- if (order)
- order = min(get_order(max_size), order);
-
- for (; order; order--) {
- ptr = (void *)__get_free_pages(gfp_mask, order);
- if (ptr) {
- *allocated_size = PAGE_SIZE << order;
- return ptr;
- }
- }
-
- /* order is zero - one page */
-
- gfp_mask = GFP_KERNEL;
-
- if (!last_warn)
- gfp_mask |= __GFP_NOWARN;
-
- ptr = (void *)__get_free_pages(gfp_mask, 0);
- if (ptr) {
- *allocated_size = PAGE_SIZE;
- return ptr;
- }
-
- *allocated_size = 0;
- return NULL;
-}
-
-/**
- * ima_free_pages() - Free pages allocated by ima_alloc_pages().
- * @ptr: Pointer to allocated pages.
- * @size: Size of allocated buffer.
- */
-static void ima_free_pages(void *ptr, size_t size)
-{
- if (!ptr)
- return;
- free_pages((unsigned long)ptr, get_order(size));
-}
-
-static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
-{
- struct crypto_ahash *tfm = ima_ahash_tfm;
- int rc;
-
- if (algo < 0 || algo >= HASH_ALGO__LAST)
- algo = ima_hash_algo;
-
- if (algo != ima_hash_algo || !tfm) {
- tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
- if (!IS_ERR(tfm)) {
- if (algo == ima_hash_algo)
- ima_ahash_tfm = tfm;
- } else {
- rc = PTR_ERR(tfm);
- pr_err("Can not allocate %s (reason: %d)\n",
- hash_algo_name[algo], rc);
- }
- }
- return tfm;
-}
-
-static void ima_free_atfm(struct crypto_ahash *tfm)
-{
- if (tfm != ima_ahash_tfm)
- crypto_free_ahash(tfm);
-}
-
-static inline int ahash_wait(int err, struct crypto_wait *wait)
-{
-
- err = crypto_wait_req(err, wait);
-
- if (err)
- pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
-
- return err;
-}
-
-static int ima_calc_file_hash_atfm(struct file *file,
- struct ima_digest_data *hash,
- struct crypto_ahash *tfm)
-{
- loff_t i_size, offset;
- char *rbuf[2] = { NULL, };
- int rc, rbuf_len, active = 0, ahash_rc = 0;
- struct ahash_request *req;
- struct scatterlist sg[1];
- struct crypto_wait wait;
- size_t rbuf_size[2];
-
- hash->length = crypto_ahash_digestsize(tfm);
-
- req = ahash_request_alloc(tfm, GFP_KERNEL);
- if (!req)
- return -ENOMEM;
-
- crypto_init_wait(&wait);
- ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP,
- crypto_req_done, &wait);
-
- rc = ahash_wait(crypto_ahash_init(req), &wait);
- if (rc)
- goto out1;
-
- i_size = i_size_read(file_inode(file));
-
- if (i_size == 0)
- goto out2;
-
- /*
- * Try to allocate maximum size of memory.
- * Fail if even a single page cannot be allocated.
- */
- rbuf[0] = ima_alloc_pages(i_size, &rbuf_size[0], 1);
- if (!rbuf[0]) {
- rc = -ENOMEM;
- goto out1;
- }
-
- /* Only allocate one buffer if that is enough. */
- if (i_size > rbuf_size[0]) {
- /*
- * Try to allocate secondary buffer. If that fails fallback to
- * using single buffering. Use previous memory allocation size
- * as baseline for possible allocation size.
- */
- rbuf[1] = ima_alloc_pages(i_size - rbuf_size[0],
- &rbuf_size[1], 0);
- }
-
- for (offset = 0; offset < i_size; offset += rbuf_len) {
- if (!rbuf[1] && offset) {
- /* Not using two buffers, and it is not the first
- * read/request, wait for the completion of the
- * previous ahash_update() request.
- */
- rc = ahash_wait(ahash_rc, &wait);
- if (rc)
- goto out3;
- }
- /* read buffer */
- rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
- rc = integrity_kernel_read(file, offset, rbuf[active],
- rbuf_len);
- if (rc != rbuf_len) {
- if (rc >= 0)
- rc = -EINVAL;
- /*
- * Forward current rc, do not overwrite with return value
- * from ahash_wait()
- */
- ahash_wait(ahash_rc, &wait);
- goto out3;
- }
-
- if (rbuf[1] && offset) {
- /* Using two buffers, and it is not the first
- * read/request, wait for the completion of the
- * previous ahash_update() request.
- */
- rc = ahash_wait(ahash_rc, &wait);
- if (rc)
- goto out3;
- }
-
- sg_init_one(&sg[0], rbuf[active], rbuf_len);
- ahash_request_set_crypt(req, sg, NULL, rbuf_len);
-
- ahash_rc = crypto_ahash_update(req);
-
- if (rbuf[1])
- active = !active; /* swap buffers, if we use two */
- }
- /* wait for the last update request to complete */
- rc = ahash_wait(ahash_rc, &wait);
-out3:
- ima_free_pages(rbuf[0], rbuf_size[0]);
- ima_free_pages(rbuf[1], rbuf_size[1]);
-out2:
- if (!rc) {
- ahash_request_set_crypt(req, NULL, hash->digest, 0);
- rc = ahash_wait(crypto_ahash_final(req), &wait);
- }
-out1:
- ahash_request_free(req);
- return rc;
-}
-
-static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash)
-{
- struct crypto_ahash *tfm;
- int rc;
-
- tfm = ima_alloc_atfm(hash->algo);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- rc = ima_calc_file_hash_atfm(file, hash, tfm);
-
- ima_free_atfm(tfm);
-
- return rc;
-}
-
static int ima_calc_file_hash_tfm(struct file *file,
struct ima_digest_data *hash,
struct crypto_shash *tfm)
{
loff_t i_size, offset = 0;
@@ -497,45 +233,19 @@ static int ima_calc_file_hash_tfm(struct file *file,
if (!rc)
rc = crypto_shash_final(shash, hash->digest);
return rc;
}
-static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)
-{
- struct crypto_shash *tfm;
- int rc;
-
- tfm = ima_alloc_tfm(hash->algo);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- rc = ima_calc_file_hash_tfm(file, hash, tfm);
-
- ima_free_tfm(tfm);
-
- return rc;
-}
-
/*
* ima_calc_file_hash - calculate file hash
- *
- * Asynchronous hash (ahash) allows using HW acceleration for calculating
- * a hash. ahash performance varies for different data sizes on different
- * crypto accelerators. shash performance might be better for smaller files.
- * The 'ima.ahash_minsize' module parameter allows specifying the best
- * minimum file size for using ahash on the system.
- *
- * If the ima.ahash_minsize parameter is not specified, this function uses
- * shash for the hash calculation. If ahash fails, it falls back to using
- * shash.
*/
int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
{
- loff_t i_size;
int rc;
struct file *f = file;
bool new_file_instance = false;
+ struct crypto_shash *tfm;
/*
* For consistency, fail file's opened with the O_DIRECT flag on
* filesystems mounted with/without DAX option.
*/
@@ -555,20 +265,17 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
return PTR_ERR(f);
new_file_instance = true;
}
- i_size = i_size_read(file_inode(f));
-
- if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
- rc = ima_calc_file_ahash(f, hash);
- if (!rc)
- goto out;
+ tfm = ima_alloc_tfm(hash->algo);
+ if (IS_ERR(tfm)) {
+ rc = PTR_ERR(tfm);
+ } else {
+ rc = ima_calc_file_hash_tfm(f, hash, tfm);
+ ima_free_tfm(tfm);
}
-
- rc = ima_calc_file_shash(f, hash);
-out:
if (new_file_instance)
fput(f);
return rc;
}
@@ -653,67 +360,10 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
return rc;
}
return rc;
}
-static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
- struct ima_digest_data *hash,
- struct crypto_ahash *tfm)
-{
- struct ahash_request *req;
- struct scatterlist sg;
- struct crypto_wait wait;
- int rc, ahash_rc = 0;
-
- hash->length = crypto_ahash_digestsize(tfm);
-
- req = ahash_request_alloc(tfm, GFP_KERNEL);
- if (!req)
- return -ENOMEM;
-
- crypto_init_wait(&wait);
- ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP,
- crypto_req_done, &wait);
-
- rc = ahash_wait(crypto_ahash_init(req), &wait);
- if (rc)
- goto out;
-
- sg_init_one(&sg, buf, len);
- ahash_request_set_crypt(req, &sg, NULL, len);
-
- ahash_rc = crypto_ahash_update(req);
-
- /* wait for the update request to complete */
- rc = ahash_wait(ahash_rc, &wait);
- if (!rc) {
- ahash_request_set_crypt(req, NULL, hash->digest, 0);
- rc = ahash_wait(crypto_ahash_final(req), &wait);
- }
-out:
- ahash_request_free(req);
- return rc;
-}
-
-static int calc_buffer_ahash(const void *buf, loff_t len,
- struct ima_digest_data *hash)
-{
- struct crypto_ahash *tfm;
- int rc;
-
- tfm = ima_alloc_atfm(hash->algo);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- rc = calc_buffer_ahash_atfm(buf, len, hash, tfm);
-
- ima_free_atfm(tfm);
-
- return rc;
-}
-
static int calc_buffer_shash_tfm(const void *buf, loff_t size,
struct ima_digest_data *hash,
struct crypto_shash *tfm)
{
SHASH_DESC_ON_STACK(shash, tfm);
@@ -740,12 +390,12 @@ static int calc_buffer_shash_tfm(const void *buf, loff_t size,
if (!rc)
rc = crypto_shash_final(shash, hash->digest);
return rc;
}
-static int calc_buffer_shash(const void *buf, loff_t len,
- struct ima_digest_data *hash)
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+ struct ima_digest_data *hash)
{
struct crypto_shash *tfm;
int rc;
tfm = ima_alloc_tfm(hash->algo);
@@ -756,24 +406,10 @@ static int calc_buffer_shash(const void *buf, loff_t len,
ima_free_tfm(tfm);
return rc;
}
-int ima_calc_buffer_hash(const void *buf, loff_t len,
- struct ima_digest_data *hash)
-{
- int rc;
-
- if (ima_ahash_minsize && len >= ima_ahash_minsize) {
- rc = calc_buffer_ahash(buf, len, hash);
- if (!rc)
- return 0;
- }
-
- return calc_buffer_shash(buf, len, hash);
-}
-
static void ima_pcrread(u32 idx, struct tpm_digest *d)
{
if (!ima_tpm_chip)
return;
base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
--
2.53.0
^ permalink raw reply related
* [PATCH] integrity: Fix spelling mistake TRUSTED_KEYRING
From: Philipp Hahn @ 2026-03-12 9:35 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg
Cc: Philipp Hahn, linux-integrity, linux-security-module,
linux-kernel
Fix minor spelling mistake "kerne{d -> l}".
Fixes: 9dc92c45177ab ("integrity: Define a trusted platform keyring")
Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
---
security/integrity/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 916d4f2bfc441..328ea9f32035a 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -60,7 +60,7 @@ config INTEGRITY_PLATFORM_KEYRING
help
Provide a separate, distinct keyring for platform trusted keys, which
the kernel automatically populates during initialization from values
- provided by the platform for verifying the kexec'ed kerned image
+ provided by the platform for verifying the kexec'ed kernel image
and, possibly, the initramfs signature.
config INTEGRITY_MACHINE_KEYRING
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] integrity: Eliminate weak definition of arch_get_secureboot()
From: Arnd Bergmann @ 2026-03-12 15:03 UTC (permalink / raw)
To: Nathan Chancellor, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg
Cc: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Paul Moore, James Morris, Serge E. Hallyn, Coiby Xu, linux-kernel,
linuxppc-dev, linux-s390, linux-integrity, linux-security-module,
llvm
In-Reply-To: <20260309-integrity-drop-weak-arch-get-secureboot-v1-1-6460d5c4bb89@kernel.org>
On Mon, Mar 9, 2026, at 21:37, Nathan Chancellor wrote:
> security/integrity/secure_boot.c contains a single __weak function,
> which breaks recordmcount when building with clang:
>
> $ make -skj"$(nproc)" ARCH=powerpc LLVM=1 ppc64_defconfig
> security/integrity/secure_boot.o
> Cannot find symbol for section 2: .text.
> security/integrity/secure_boot.o: failed
>
> Introduce a Kconfig symbol, CONFIG_HAVE_ARCH_GET_SECUREBOOT, to indicate
> that an architecture provides a definition of arch_get_secureboot().
> Provide a static inline stub when this symbol is not defined to achieve
> the same effect as the __weak function, allowing secure_boot.c to be
> removed altogether. Move the s390 definition of arch_get_secureboot()
> out of the CONFIG_KEXEC_FILE block to ensure it is always available, as
> it does not actually depend on KEXEC_FILE.
>
> Fixes: 31a6a07eefeb ("integrity: Make arch_ima_get_secureboot integrity-wide")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH] integrity: Eliminate weak definition of arch_get_secureboot()
From: Mimi Zohar @ 2026-03-12 16:07 UTC (permalink / raw)
To: Arnd Bergmann, Nathan Chancellor, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Alexander Egorenkov
Cc: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Paul Moore, James Morris, Serge E. Hallyn, Coiby Xu, linux-kernel,
linuxppc-dev, linux-s390, linux-integrity, linux-security-module,
llvm
In-Reply-To: <d2089740-16d8-4ca4-a61c-8c381f8e30a0@app.fastmail.com>
On Thu, 2026-03-12 at 16:03 +0100, Arnd Bergmann wrote:
> On Mon, Mar 9, 2026, at 21:37, Nathan Chancellor wrote:
> > security/integrity/secure_boot.c contains a single __weak function,
> > which breaks recordmcount when building with clang:
> >
> > $ make -skj"$(nproc)" ARCH=powerpc LLVM=1 ppc64_defconfig
> > security/integrity/secure_boot.o
> > Cannot find symbol for section 2: .text.
> > security/integrity/secure_boot.o: failed
> >
> > Introduce a Kconfig symbol, CONFIG_HAVE_ARCH_GET_SECUREBOOT, to indicate
> > that an architecture provides a definition of arch_get_secureboot().
> > Provide a static inline stub when this symbol is not defined to achieve
> > the same effect as the __weak function, allowing secure_boot.c to be
> > removed altogether. Move the s390 definition of arch_get_secureboot()
> > out of the CONFIG_KEXEC_FILE block to ensure it is always available, as
> > it does not actually depend on KEXEC_FILE.
> >
> > Fixes: 31a6a07eefeb ("integrity: Make arch_ima_get_secureboot integrity-wide")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
I pushed out the patch to next-integrity, but am a bit concerned about the
definition:
+config HAVE_ARCH_GET_SECUREBOOT
+ def_bool EFI
+
Has anyone actually tested this patch on s390, not just compiled it? If so, I'd
appreciate a tested-by tag.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH] integrity: Eliminate weak definition of arch_get_secureboot()
From: Nathan Chancellor @ 2026-03-12 20:55 UTC (permalink / raw)
To: Mimi Zohar
Cc: Arnd Bergmann, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Alexander Egorenkov, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Paul Moore, James Morris, Serge E. Hallyn, Coiby Xu, linux-kernel,
linuxppc-dev, linux-s390, linux-integrity, linux-security-module,
llvm
In-Reply-To: <a985c90d9df8ba0fc63f65117cc8e884f70e6035.camel@linux.ibm.com>
On Thu, Mar 12, 2026 at 12:07:41PM -0400, Mimi Zohar wrote:
> I pushed out the patch to next-integrity, but am a bit concerned about the
> definition:
>
> +config HAVE_ARCH_GET_SECUREBOOT
> + def_bool EFI
> +
What is concerning about the definition with regards to s390?
> Has anyone actually tested this patch on s390, not just compiled it? If so, I'd
> appreciate a tested-by tag.
It would be good to test (if it is possible to test in QEMU, I am happy
to attempt to do so). As far as I can tell, 31a6a07eefeb placed
arch_get_secureboot() in such a way that the __weak definition would be
used when CONFIG_KEXEC_FILE was disabled, even though ipl_secure_flag
should always be available, which this patch avoids.
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH] ima: remove buggy support for asynchronous hashes
From: Mimi Zohar @ 2026-03-12 23:29 UTC (permalink / raw)
To: Eric Biggers, linux-integrity, Roberto Sassu, Dmitry Kasatkin
Cc: linux-crypto, linux-kernel
In-Reply-To: <20260312053933.53012-1-ebiggers@kernel.org>
On Wed, 2026-03-11 at 22:39 -0700, Eric Biggers wrote:
> IMA computes hashes using the crypto_shash or crypto_ahash API. The
> latter is used only when ima.ahash_minsize is set on the command line,
> and its purpose is ostensibly to make the hash computation faster.
>
> However, going off the CPU to a crypto engine and back again is actually
> quite slow, especially compared with the acceleration that is built into
> modern CPUs and the kernel now enables by default for most algorithms.
> Typical performance results for SHA-256 on a modern platform can be
> found at https://lore.kernel.org/linux-crypto/20250615184638.GA1480@sol/
>
> Partly for this reason, several other kernel subsystems have already
> dropped support for the crypto_ahash API.
The performance benefit was the ability of reading and filling a buffer from
disk, which was slow, while the other buffer was sent to the crypto engine.
I'm all for removing extraneous code. I'll give it a couple of days, before
queuing the patch in case there are any objections.
thanks,
Mimi
^ permalink raw reply
* [PATCH] tpm: fix tpm disabling if NULL name changes
From: James Bottomley @ 2026-03-13 14:31 UTC (permalink / raw)
To: linux-integrity; +Cc: Jarkko Sakkinen
There's a logic error in the earlier fix which means that if the NULL
name comparison fails, the tpm isn't disabled because rc remains zero.
Fix this by setting it to an error.
Cc: stable@vger.kernel.org # 6.12
Fixes: cc7d8594342a ("tpm: Rollback tpm2_load_null()")
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
I didn't actually discover this problem until I was trying to do a
reset attack demo with an updated kernel.
drivers/char/tpm/tpm2-sessions.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-
sessions.c
index 3b1cf1ca0420..bd1c0456e775 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -961,6 +961,7 @@ static int tpm2_load_null(struct tpm_chip *chip,
u32 *null_key)
/* Deduce from the name change TPM interference: */
dev_err(&chip->dev, "null key integrity check failed\n");
tpm2_flush_context(chip, tmp_null_key);
+ rc = -ENODEV;
err:
if (rc) {
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] integrity: Eliminate weak definition of arch_get_secureboot()
From: Mimi Zohar @ 2026-03-13 15:35 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Arnd Bergmann, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Alexander Egorenkov, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Paul Moore, James Morris, Serge E. Hallyn, Coiby Xu, linux-kernel,
linuxppc-dev, linux-s390, linux-integrity, linux-security-module,
llvm
In-Reply-To: <20260312205533.GC2747807@ax162>
On Thu, 2026-03-12 at 13:55 -0700, Nathan Chancellor wrote:
> On Thu, Mar 12, 2026 at 12:07:41PM -0400, Mimi Zohar wrote:
> > I pushed out the patch to next-integrity, but am a bit concerned about the
> > definition:
> >
> > +config HAVE_ARCH_GET_SECUREBOOT
> > + def_bool EFI
> > +
>
> What is concerning about the definition with regards to s390?
>
> > Has anyone actually tested this patch on s390, not just compiled it? If so, I'd
> > appreciate a tested-by tag.
>
> It would be good to test (if it is possible to test in QEMU, I am happy
> to attempt to do so). As far as I can tell, 31a6a07eefeb placed
> arch_get_secureboot() in such a way that the __weak definition would be
> used when CONFIG_KEXEC_FILE was disabled, even though ipl_secure_flag
> should always be available, which this patch avoids.
Thanks, Nathan. Fortunately I got access to an s390 and was able to test. It
seems to be working.
Mimi
^ permalink raw reply
* [PATCH v2 0/2] vfs: follow-on fixes for i_ino widening
From: Jeff Layton @ 2026-03-13 18:44 UTC (permalink / raw)
To: Ryusuke Konishi, Viacheslav Dubeyko, Christian Brauner,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-nilfs, linux-kernel, linux-integrity, linux-security-module,
Jeff Layton, kernel test robot
Just some patches to fix follow-on issues reported after the
inode->i_ino widening series. Christian, could you toss these
onto the vfs-7.1.kino branch?
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- rename variable in nilfs2 patch from "rem" to "index"
- reword comment and commit log for better accuracy in EVM patch
---
Jeff Layton (2):
nilfs2: fix 64-bit division operations in nilfs_bmap_find_target_in_group()
EVM: add comment describing why ino field is still unsigned long
fs/nilfs2/bmap.c | 9 ++++++---
security/integrity/evm/evm_crypto.c | 6 ++++++
2 files changed, 12 insertions(+), 3 deletions(-)
---
base-commit: 9840bb66e7e5dffd72b03201318f154a10b06b4a
change-id: 20260310-iino-u64-424fa570d850
Best regards,
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox