[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <16c446c001a96a9878ddec9726430d7001c3f47b.camel@huaweicloud.com>
Date: Tue, 27 Jan 2026 15:28:28 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: dima@...sta.com, Mimi Zohar <zohar@...ux.ibm.com>, Roberto Sassu
<roberto.sassu@...wei.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
Eric Snowberg <eric.snowberg@...cle.com>, Paul Moore <paul@...l-moore.com>,
James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
Silvia Sisinni <silvia.sisinni@...ito.it>, Enrico Bravi
<enrico.bravi@...ito.it>
Cc: linux-integrity@...r.kernel.org, linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org, Dmitry Safonov
<0x7f454c46@...il.com>
Subject: Re: [PATCH v2] ima_fs: Avoid creating measurement lists for
unsupported hash algos
On Tue, 2026-01-27 at 14:18 +0000, Dmitry Safonov via B4 Relay wrote:
> From: Dmitry Safonov <dima@...sta.com>
>
> ima_init_crypto() skips initializing ima_algo_array[i] if the alogorithm
> from ima_tpm_chip->allocated_banks[i].crypto_id is not supported.
> It seems avoid adding the unsupported algorithm to ima_algo_array will
> break all the logic that relies on indexing by NR_BANKS(ima_tpm_chip).
>
> 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
> >
> > The buggy address belongs to the physical page:
> > page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x107ce18
> > flags: 0x8000000000002000(reserved|zone=2)
> > raw: 8000000000002000 ffffea0041f38608 ffffea0041f38608 0000000000000000
> > raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > 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
>
> Grepping HASH_ALGO__LAST in security/integrity/ima/ shows that is
> the check other logic relies on, so add files under TPM_ALG_<ID>
> and print 0 as their hash_digest_size.
>
> This is how it looks on the test machine I have:
> > # 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
>
> Fixes: 9fa8e7625008 ("ima: add crypto agility support for template-hash algorithm")
> Signed-off-by: Dmitry Safonov <dima@...sta.com>
> Cc: Enrico Bravi <enrico.bravi@...ito.it>
> Cc: Silvia Sisinni <silvia.sisinni@...ito.it>
> Cc: Roberto Sassu <roberto.sassu@...wei.com>
> Cc: Mimi Zohar <zohar@...ux.ibm.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 | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 012a58959ff0..3b442e3f84d0 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -160,7 +160,10 @@ int ima_measurements_show(struct seq_file *m, void *v)
> ima_putc(m, &pcr, sizeof(e->pcr));
>
> /* 2nd: template digest */
> - ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
> + if (algo == HASH_ALGO__LAST)
> + ima_putc(m, "0", 1);
> + else
> + ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
>
> /* 3rd: template name size */
> namelen = !ima_canonical_fmt ? strlen(template_name) :
> @@ -252,7 +255,10 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
> seq_printf(m, "%2d ", e->pcr);
>
> /* 2nd: template hash */
> - ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
> + if (algo == HASH_ALGO__LAST)
> + ima_putc(m, "0", 1);
> + else
> + ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
No need, the last one is ok with ima_tpm_chip->allocated_banks[algo_idx].digest_size.
Roberto
>
> /* 3th: template name */
> seq_printf(m, " %s", template_name);
> @@ -404,16 +410,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: 63804fed149a6750ffd28610c5c1c98cce6bd377
> change-id: 20260127-ima-oob-9fa83a634d7b
>
> Best regards,
Powered by blists - more mailing lists