* Re: [RFC][PATCH v3 1/2] ima: measure loaded policy after write on securityfs policy file
From: Mimi Zohar @ 2026-06-10 14:32 UTC (permalink / raw)
To: Enrico Bravi, linux-integrity, dmitry.kasatkin, roberto.sassu
Cc: eric.snowberg
In-Reply-To: <20260526135118.289633-2-enrico.bravi@polito.it>
On Tue, 2026-05-26 at 15:51 +0200, Enrico Bravi wrote:
> IMA policy can be written multiple times in the securityfs policy file
> at runtime if CONFIG_IMA_WRITE_POLICY=y. When IMA_APPRAISE_POLICY is
> required, the policy needs to be signed to be loaded, writing the absolute
> path of the file containing the new policy:
>
> echo /path/of/custom_ima_policy > /sys/kernel/security/ima/policy
>
> When this is not required, policy can be written directly, rule by rule:
>
> echo -e "measure func=BPRM_CHECK mask=MAY_EXEC\n" \
> "audit func=BPRM_CHECK mask=MAY_EXEC\n" \
> > /sys/kernel/security/ima/policy
>
> In this case, a new policy can be loaded without being measured or
> appraised.
>
> Add a new critical data record to measure the textual policy
> representation when it becomes effective.
> To verify the template data hash value, convert the buffer policy data
> to binary:
> grep "ima_policy_loaded" \
> /sys/kernel/security/integrity/ima/ascii_runtime_measurements | \
> tail -1 | cut -d' ' -f 6 | xxd -r -p | sha256sum
>
> Signed-off-by: Enrico Bravi <enrico.bravi@polito.it>
Thanks, Enrico. Just a few inline comments.
> ---
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_efi.c | 2 ++
> security/integrity/ima/ima_fs.c | 1 +
> security/integrity/ima/ima_policy.c | 55 +++++++++++++++++++++++++++--
> 4 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 89ebe98ffc5e..a223d3f30d88 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -425,6 +425,7 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos);
> void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos);
> void ima_policy_stop(struct seq_file *m, void *v);
> int ima_policy_show(struct seq_file *m, void *v);
> +void ima_measure_loaded_policy(void);
>
> /* Appraise integrity measurements */
> #define IMA_APPRAISE_ENFORCE 0x01
> diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
> index 138029bfcce1..8e9f85ec9a86 100644
> --- a/security/integrity/ima/ima_efi.c
> +++ b/security/integrity/ima/ima_efi.c
> @@ -60,6 +60,8 @@ static const char * const sb_arch_rules[] = {
> #endif
> #if IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING) && IS_ENABLED(CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY)
> "appraise func=POLICY_CHECK appraise_type=imasig",
> +#else
> + "measure func=CRITICAL_DATA label=ima_policy",
> #endif
None of the other arch "measure" policy rules are conditional. Should the new
"measure" rule be limited?
> "measure func=MODULE_CHECK",
> NULL
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 012a58959ff0..75cb308cf01f 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -476,6 +476,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
> }
>
> ima_update_policy();
> + ima_measure_loaded_policy();
> #if !defined(CONFIG_IMA_WRITE_POLICY) && !defined(CONFIG_IMA_READ_POLICY)
> securityfs_remove(file->f_path.dentry);
> #elif defined(CONFIG_IMA_WRITE_POLICY)
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index bf2d7ba4c14a..e0b4dae922b6 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -17,6 +17,7 @@
> #include <linux/slab.h>
> #include <linux/rculist.h>
> #include <linux/seq_file.h>
> +#include <linux/vmalloc.h>
> #include <linux/ima.h>
>
> #include "ima.h"
> @@ -2022,7 +2023,6 @@ const char *const func_tokens[] = {
> __ima_hooks(__ima_hook_stringify)
> };
>
> -#ifdef CONFIG_IMA_READ_POLICY
Removing the ifdef, here, does not affect viewing the IMA measurement lists, but
allows copying and measuring the policy rules. Please include a comment in the
patch description.
> enum {
> mask_exec = 0, mask_write, mask_read, mask_append
> };
> @@ -2324,7 +2324,6 @@ int ima_policy_show(struct seq_file *m, void *v)
> seq_puts(m, "\n");
> return 0;
> }
> -#endif /* CONFIG_IMA_READ_POLICY */
>
> #if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
> /*
> @@ -2381,3 +2380,55 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
> return found;
> }
> #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> +
Please add kernel-doc here, something like:
/**
* ima_measure_loaded_policy - measure the active IMA policy ruleset
*
* Must be called with ima_write_mutex held, as it performs two
* separate RCU read passes over ima_rules and relies on the mutex
* to prevent concurrent policy updates between them.
*/
> +void ima_measure_loaded_policy(void)
> +{
> + const char *event_name = "ima_policy_loaded";
> + const char *op = "measure_loaded_ima_policy";
> + const char *audit_cause = "ENOMEM";
> + struct ima_rule_entry *rule_entry;
> + struct list_head *ima_rules_tmp;
> + struct seq_file file;
> + int result = -ENOMEM;
> + size_t file_len;
> + char rule[255];
The 255-byte buffer may be insufficient for custom policy rules that include
additional fields such as LSM labels and other file metadata, unlike the simpler
built-in and architecture-specific rules. Please increase the buffer size to
accommodate the worst-case serialized rule length.
> +
> + /* calculate IMA policy rules memory size */
> + file.buf = rule;
> + file.read_pos = 0;
> + file.size = 255;
> + file.count = 0;
> +
Please add "lockdep_assert_held(&ima_write_mutex);" here.
> + rcu_read_lock();
> + ima_rules_tmp = rcu_dereference(ima_rules);
> + list_for_each_entry_rcu(rule_entry, ima_rules_tmp, list) {
> + ima_policy_show(&file, rule_entry);
> + file_len += file.count;
> + file.count = 0;
> + }
Variables defined on the stack need to be initialized before being used. Please
iniitalize file_len to zero.
> + rcu_read_unlock();
> +
> + /* copy IMA policy rules to a buffer for measuring */
> + file.buf = vmalloc(file_len);
> + if (!file.buf) {
> + integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, event_name,
> + op, audit_cause, result, 1);
> + return;
> + }
> +
> + file.read_pos = 0;
> + file.size = file_len;
> + file.count = 0;
> +
> + rcu_read_lock();
> + ima_rules_tmp = rcu_dereference(ima_rules);
> + list_for_each_entry_rcu(rule_entry, ima_rules_tmp, list) {
> + ima_policy_show(&file, rule_entry);
> + }
> + rcu_read_unlock();
> +
> + ima_measure_critical_data("ima_policy", event_name, file.buf,
> + file.count, false, NULL, 0);
> +
> + vfree(file.buf);
> +}
Thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v1] tpm_crb: Check ACPI_COMPANION() against NULL during probe
From: Jarkko Sakkinen @ 2026-06-09 16:43 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, Andy Shevchenko,
LKML, Linux ACPI
In-Reply-To: <CAJZ5v0jAh2rGxUn1ajXamDH0n9-zPtnh7pos=iRC1DwWi7pi=A@mail.gmail.com>
On Tue, Jun 09, 2026 at 06:14:47PM +0200, Rafael J. Wysocki wrote:
> On Tue, Jun 9, 2026 at 5:45 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Mon, Jun 08, 2026 at 07:35:26PM +0200, Rafael J. Wysocki wrote:
> > > On Tue, May 19, 2026 at 11:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > >
> > > > On Sat, May 16, 2026 at 3:15 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > >
> > > > > On Tue, May 12, 2026 at 06:16:23PM +0200, Rafael J. Wysocki wrote:
> > > > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > > > >
> > > > > > Every platform driver can be forced to match a device that doesn't match
> > > > > > its list of device IDs because of device_match_driver_override(), so
> > > > > > platform drivers that rely on the existence of a device's ACPI companion
> > > > > > object need to verify its presence.
> > > > > >
> > > > > > Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> > > > > > tpm_crb driver.
> > > > > >
> > > > > > Fixes: 48fe2cddc85c ("tpm_crb: Convert ACPI driver to a platform one")
> > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > > > ---
> > > > > > drivers/char/tpm/tpm_crb.c | 6 +++++-
> > > > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > > @@ -786,8 +786,8 @@ static int crb_map_pluton(struct device
> > > > > > static int crb_acpi_probe(struct platform_device *pdev)
> > > > > > {
> > > > > > struct device *dev = &pdev->dev;
> > > > > > - struct acpi_device *device = ACPI_COMPANION(dev);
> > > > > > struct acpi_table_tpm2 *buf;
> > > > > > + struct acpi_device *device;
> > > > > > struct crb_priv *priv;
> > > > > > struct tpm_chip *chip;
> > > > > > struct tpm2_crb_smc *crb_smc;
> > > > > > @@ -797,6 +797,10 @@ static int crb_acpi_probe(struct platfor
> > > > > > u32 sm;
> > > > > > int rc;
> > > > > >
> > > > > > + device = ACPI_COMPANION(dev);
> > > > > > + if (!device)
> > > > > > + return -ENODEV;
> > > > > > +
> > > > > > status = acpi_get_table(ACPI_SIG_TPM2, 1,
> > > > > > (struct acpi_table_header **) &buf);
> > > > > > if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > >
> > > > Thanks!
> > > >
> > > > So do you want me to pick up this one?
> > >
> > > I took the silence as consent and picked it up. If you'd rather route
> > > it differently, please let me know.
> >
> > OK my bad in commes, sorry. I did already pick this up and it is
> > going to my next PR.
>
> OK, I'll drop it then, thanks!
NP, and apologies for causing extra trouble!
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v1] tpm_crb: Check ACPI_COMPANION() against NULL during probe
From: Rafael J. Wysocki @ 2026-06-09 16:14 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Rafael J. Wysocki, Peter Huewe, Jason Gunthorpe, linux-integrity,
Andy Shevchenko, LKML, Linux ACPI
In-Reply-To: <aig1IKKZYid2BQIB@kernel.org>
On Tue, Jun 9, 2026 at 5:45 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Mon, Jun 08, 2026 at 07:35:26PM +0200, Rafael J. Wysocki wrote:
> > On Tue, May 19, 2026 at 11:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Sat, May 16, 2026 at 3:15 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > >
> > > > On Tue, May 12, 2026 at 06:16:23PM +0200, Rafael J. Wysocki wrote:
> > > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > > >
> > > > > Every platform driver can be forced to match a device that doesn't match
> > > > > its list of device IDs because of device_match_driver_override(), so
> > > > > platform drivers that rely on the existence of a device's ACPI companion
> > > > > object need to verify its presence.
> > > > >
> > > > > Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> > > > > tpm_crb driver.
> > > > >
> > > > > Fixes: 48fe2cddc85c ("tpm_crb: Convert ACPI driver to a platform one")
> > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > > ---
> > > > > drivers/char/tpm/tpm_crb.c | 6 +++++-
> > > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > @@ -786,8 +786,8 @@ static int crb_map_pluton(struct device
> > > > > static int crb_acpi_probe(struct platform_device *pdev)
> > > > > {
> > > > > struct device *dev = &pdev->dev;
> > > > > - struct acpi_device *device = ACPI_COMPANION(dev);
> > > > > struct acpi_table_tpm2 *buf;
> > > > > + struct acpi_device *device;
> > > > > struct crb_priv *priv;
> > > > > struct tpm_chip *chip;
> > > > > struct tpm2_crb_smc *crb_smc;
> > > > > @@ -797,6 +797,10 @@ static int crb_acpi_probe(struct platfor
> > > > > u32 sm;
> > > > > int rc;
> > > > >
> > > > > + device = ACPI_COMPANION(dev);
> > > > > + if (!device)
> > > > > + return -ENODEV;
> > > > > +
> > > > > status = acpi_get_table(ACPI_SIG_TPM2, 1,
> > > > > (struct acpi_table_header **) &buf);
> > > > > if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
> > > > >
> > > > >
> > > > >
> > > >
> > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > >
> > > Thanks!
> > >
> > > So do you want me to pick up this one?
> >
> > I took the silence as consent and picked it up. If you'd rather route
> > it differently, please let me know.
>
> OK my bad in commes, sorry. I did already pick this up and it is
> going to my next PR.
OK, I'll drop it then, thanks!
^ permalink raw reply
* Re: [PATCH v1] tpm_crb: Check ACPI_COMPANION() against NULL during probe
From: Jarkko Sakkinen @ 2026-06-09 15:45 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, Andy Shevchenko,
LKML, Linux ACPI
In-Reply-To: <CAJZ5v0iQO_ozXQpzj4qKLegRzMURPDjJV47ynfL6OAUUg_8f4w@mail.gmail.com>
On Mon, Jun 08, 2026 at 07:35:26PM +0200, Rafael J. Wysocki wrote:
> On Tue, May 19, 2026 at 11:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Sat, May 16, 2026 at 3:15 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > >
> > > On Tue, May 12, 2026 at 06:16:23PM +0200, Rafael J. Wysocki wrote:
> > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > >
> > > > Every platform driver can be forced to match a device that doesn't match
> > > > its list of device IDs because of device_match_driver_override(), so
> > > > platform drivers that rely on the existence of a device's ACPI companion
> > > > object need to verify its presence.
> > > >
> > > > Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> > > > tpm_crb driver.
> > > >
> > > > Fixes: 48fe2cddc85c ("tpm_crb: Convert ACPI driver to a platform one")
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > ---
> > > > drivers/char/tpm/tpm_crb.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > @@ -786,8 +786,8 @@ static int crb_map_pluton(struct device
> > > > static int crb_acpi_probe(struct platform_device *pdev)
> > > > {
> > > > struct device *dev = &pdev->dev;
> > > > - struct acpi_device *device = ACPI_COMPANION(dev);
> > > > struct acpi_table_tpm2 *buf;
> > > > + struct acpi_device *device;
> > > > struct crb_priv *priv;
> > > > struct tpm_chip *chip;
> > > > struct tpm2_crb_smc *crb_smc;
> > > > @@ -797,6 +797,10 @@ static int crb_acpi_probe(struct platfor
> > > > u32 sm;
> > > > int rc;
> > > >
> > > > + device = ACPI_COMPANION(dev);
> > > > + if (!device)
> > > > + return -ENODEV;
> > > > +
> > > > status = acpi_get_table(ACPI_SIG_TPM2, 1,
> > > > (struct acpi_table_header **) &buf);
> > > > if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
> > > >
> > > >
> > > >
> > >
> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> >
> > Thanks!
> >
> > So do you want me to pick up this one?
>
> I took the silence as consent and picked it up. If you'd rather route
> it differently, please let me know.
OK my bad in commes, sorry. I did already pick this up and it is
going to my next PR.
>
> Thanks!
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
From: Jarkko Sakkinen @ 2026-06-09 15:44 UTC (permalink / raw)
To: Benoit HOUYERE
Cc: Serge E. Hallyn, linux-integrity@vger.kernel.org,
Frédéric Jouen, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, open list, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, Laurent CHARPENTIER
In-Reply-To: <DB5PR10MB766844D5478D63272584A12AFE1C2@DB5PR10MB7668.EURPRD10.PROD.OUTLOOK.COM>
On Mon, Jun 08, 2026 at 02:46:28PM +0000, Benoit HOUYERE wrote:
> Hello Serge and Jarkko,
>
>
> We have detected a regression with this fix. Indeed, it miss one zero on TPM_LONG_LONG. Initial value was 300000 and not 30000.
>
> > + {TPM2_CC_CREATE_PRIMARY, 30000},
> > + {TPM2_CC_CREATE, 30000},
> > + {TPM2_CC_CREATE_LOADED, 30000},
>
> > +enum tpm2_durations {
> > TPM2_DURATION_SHORT = 20,
> > - TPM2_DURATION_MEDIUM = 750,
> > TPM2_DURATION_LONG = 2000,
> > - TPM2_DURATION_LONG_LONG = 300000,
> > TPM2_DURATION_DEFAULT = 120000,
> > };
>
> Best Regards, Cordialement, Cordialmente, Hälsningar, 最好的问候, Mit besten Grüßen, 真心を込めて, 진심으로
>
>
> Benoit HOUYERE | Tel: +33 6 14 22 81 30
> TPM specialist
This has been fixed in 478a3f949a43822dc6ce089345ae80e8dcde3300.
>
> -----Original Message-----
> From: Serge E. Hallyn <serge@hallyn.com>
> Sent: Friday, September 19, 2025 5:49 AM
> To: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: linux-integrity@vger.kernel.org; Frédéric Jouen <fjouen@sealsq.com>; Peter Huewe <peterhuewe@gmx.de>; Jason Gunthorpe <jgg@ziepe.ca>; James Bottomley <James.Bottomley@hansenpartnership.com>; Mimi Zohar <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn <serge@hallyn.com>; open list <linux-kernel@vger.kernel.org>; open list:KEYS-TRUSTED <keyrings@vger.kernel.org>; open list:SECURITY SUBSYSTEM <linux-security-module@vger.kernel.org>
> Subject: Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
>
> On Thu, Sep 18, 2025 at 10:30:18PM +0300, Jarkko Sakkinen wrote:
> > The current shenanigans for duration calculation introduce too much
> > complexity for a trivial problem, and further the code is hard to
> > patch and maintain.
> >
> > Address these issues with a flat look-up table, which is easy to
> > understand and patch. If leaf driver specific patching is required in
> > future, it is easy enough to make a copy of this table during driver
> > initialization and add the chip parameter back.
> >
> > 'chip->duration' is retained for TPM 1.x.
> >
> > As the first entry for this new behavior address TCG spec update
> > mentioned in this issue:
> >
> > https://github.com/raspberrypi/linux/issues/7054
> >
> > Therefore, for TPM_SelfTest the duration is set to 3000 ms.
> >
> > This does not categorize a as bug, given that this is introduced to
> > the spec after the feature was originally made.
> >
> > Cc: Frédéric Jouen <fjouen@sealsq.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> fwiw (which shouldn't be much) looks good to me, but two questions, one here and one below.
>
> First, it looks like in the existing code it is possible for a tpm2 chip to set its own timeouts and then set the TPM_CHIP_FLAG_HAVE_TIMEOUTS flag to avoid using the defaults, but I don't see anything using that in-tree. Is it possible that there are out of tree drivers that will be sabotaged here? Or am I misunderstanding that completely?
>
> > ---
> > v2:
> > - Add the missing msec_to_jiffies() calls.
> > - Drop redundant stuff.
> > ---
> > drivers/char/tpm/tpm-interface.c | 2 +-
> > drivers/char/tpm/tpm.h | 2 +-
> > drivers/char/tpm/tpm2-cmd.c | 127 ++++++++-----------------------
> > include/linux/tpm.h | 5 +-
> > 4 files changed, 37 insertions(+), 99 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index b71725827743..c9f173001d0e 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -52,7 +52,7 @@ MODULE_PARM_DESC(suspend_pcr, unsigned long
> > tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal) {
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > - return tpm2_calc_ordinal_duration(chip, ordinal);
> > + return tpm2_calc_ordinal_duration(ordinal);
> > else
> > return tpm1_calc_ordinal_duration(chip, ordinal); } diff --git
> > a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> > 7bb87fa5f7a1..2726bd38e5ac 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -299,7 +299,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32
> > property_id, ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
> > int tpm2_auto_startup(struct tpm_chip *chip); void
> > tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type); -unsigned
> > long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > +unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
> > int tpm2_probe(struct tpm_chip *chip); int
> > tpm2_get_cc_attrs_tbl(struct tpm_chip *chip); int tpm2_find_cc(struct
> > tpm_chip *chip, u32 cc); diff --git a/drivers/char/tpm/tpm2-cmd.c
> > b/drivers/char/tpm/tpm2-cmd.c index 524d802ede26..7d77f6fbc152 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -28,120 +28,57 @@ static struct tpm2_hash tpm2_hash_map[] = {
> >
> > int tpm2_get_timeouts(struct tpm_chip *chip) {
> > - /* Fixed timeouts for TPM2 */
> > chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
> > chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
> > chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
> > chip->timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D);
> > -
> > - /* PTP spec timeouts */
> > - chip->duration[TPM_SHORT] = msecs_to_jiffies(TPM2_DURATION_SHORT);
> > - chip->duration[TPM_MEDIUM] = msecs_to_jiffies(TPM2_DURATION_MEDIUM);
> > - chip->duration[TPM_LONG] = msecs_to_jiffies(TPM2_DURATION_LONG);
> > -
> > - /* Key creation commands long timeouts */
> > - chip->duration[TPM_LONG_LONG] =
> > - msecs_to_jiffies(TPM2_DURATION_LONG_LONG);
> > -
> > chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
> > -
> > return 0;
> > }
> >
> > -/**
> > - * tpm2_ordinal_duration_index() - returns an index to the chip
> > duration table
> > - * @ordinal: TPM command ordinal.
> > - *
> > - * The function returns an index to the chip duration table
> > - * (enum tpm_duration), that describes the maximum amount of
> > - * time the chip could take to return the result for a particular ordinal.
> > - *
> > - * The values of the MEDIUM, and LONG durations are taken
> > - * from the PC Client Profile (PTP) specification (750, 2000 msec)
> > - *
> > - * LONG_LONG is for commands that generates keys which empirically
> > takes
> > - * a longer time on some systems.
> > - *
> > - * Return:
> > - * * TPM_MEDIUM
> > - * * TPM_LONG
> > - * * TPM_LONG_LONG
> > - * * TPM_UNDEFINED
> > +/*
> > + * Contains the maximum durations in milliseconds for TPM2 commands.
> > */
> > -static u8 tpm2_ordinal_duration_index(u32 ordinal) -{
> > - switch (ordinal) {
> > - /* Startup */
> > - case TPM2_CC_STARTUP: /* 144 */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_SELF_TEST: /* 143 */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_GET_RANDOM: /* 17B */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_SEQUENCE_UPDATE: /* 15C */
> > - return TPM_MEDIUM;
> > - case TPM2_CC_SEQUENCE_COMPLETE: /* 13E */
> > - return TPM_MEDIUM;
> > - case TPM2_CC_EVENT_SEQUENCE_COMPLETE: /* 185 */
> > - return TPM_MEDIUM;
> > - case TPM2_CC_HASH_SEQUENCE_START: /* 186 */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_VERIFY_SIGNATURE: /* 177 */
> > - return TPM_LONG_LONG;
> > -
> > - case TPM2_CC_PCR_EXTEND: /* 182 */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_HIERARCHY_CONTROL: /* 121 */
> > - return TPM_LONG;
> > - case TPM2_CC_HIERARCHY_CHANGE_AUTH: /* 129 */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_GET_CAPABILITY: /* 17A */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_NV_READ: /* 14E */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_CREATE_PRIMARY: /* 131 */
> > - return TPM_LONG_LONG;
> > - case TPM2_CC_CREATE: /* 153 */
> > - return TPM_LONG_LONG;
> > - case TPM2_CC_CREATE_LOADED: /* 191 */
> > - return TPM_LONG_LONG;
> > -
> > - default:
> > - return TPM_UNDEFINED;
> > - }
> > -}
> > +static const struct {
> > + unsigned long ordinal;
> > + unsigned long duration;
> > +} tpm2_ordinal_duration_map[] = {
> > + {TPM2_CC_STARTUP, 750},
> > + {TPM2_CC_SELF_TEST, 3000},
>
> I assume you intended to increase TPM2_CC_SELF_TEST from 2000 to 3000 here? But it's not mentioned in the commit, so making sure...
>
> > + {TPM2_CC_GET_RANDOM, 2000},
> > + {TPM2_CC_SEQUENCE_UPDATE, 750},
> > + {TPM2_CC_SEQUENCE_COMPLETE, 750},
> > + {TPM2_CC_EVENT_SEQUENCE_COMPLETE, 750},
> > + {TPM2_CC_HASH_SEQUENCE_START, 750},
> > + {TPM2_CC_VERIFY_SIGNATURE, 30000},
> > + {TPM2_CC_PCR_EXTEND, 750},
> > + {TPM2_CC_HIERARCHY_CONTROL, 2000},
> > + {TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
> > + {TPM2_CC_GET_CAPABILITY, 750},
> > + {TPM2_CC_NV_READ, 2000},
> > + {TPM2_CC_CREATE_PRIMARY, 30000},
> > + {TPM2_CC_CREATE, 30000},
> > + {TPM2_CC_CREATE_LOADED, 30000},
> > +};
> >
> > /**
> > - * tpm2_calc_ordinal_duration() - calculate the maximum command duration
> > - * @chip: TPM chip to use.
> > + * tpm2_calc_ordinal_duration() - Calculate the maximum command
> > + duration
> > * @ordinal: TPM command ordinal.
> > *
> > - * The function returns the maximum amount of time the chip could
> > take
> > - * to return the result for a particular ordinal in jiffies.
> > - *
> > - * Return: A maximal duration time for an ordinal in jiffies.
> > + * Returns the maximum amount of time the chip is expected by kernel
> > + to
> > + * take in jiffies.
> > */
> > -unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32
> > ordinal)
> > +unsigned long tpm2_calc_ordinal_duration(u32 ordinal)
> > {
> > - unsigned int index;
> > + int i;
> >
> > - index = tpm2_ordinal_duration_index(ordinal);
> > + for (i = 0; i < ARRAY_SIZE(tpm2_ordinal_duration_map); i++)
> > + if (ordinal == tpm2_ordinal_duration_map[i].ordinal)
> > + return msecs_to_jiffies(tpm2_ordinal_duration_map[i].duration);
> >
> > - if (index != TPM_UNDEFINED)
> > - return chip->duration[index];
> > - else
> > - return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> > + return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> > }
> >
> > -
> > struct tpm2_pcr_read_out {
> > __be32 update_cnt;
> > __be32 pcr_selects_cnt;
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index
> > b0e9eb5ef022..dc0338a783f3 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -228,10 +228,11 @@ enum tpm2_timeouts {
> > TPM2_TIMEOUT_B = 4000,
> > TPM2_TIMEOUT_C = 200,
> > TPM2_TIMEOUT_D = 30,
> > +};
> > +
> > +enum tpm2_durations {
> > TPM2_DURATION_SHORT = 20,
> > - TPM2_DURATION_MEDIUM = 750,
> > TPM2_DURATION_LONG = 2000,
> > - TPM2_DURATION_LONG_LONG = 300000,
> > TPM2_DURATION_DEFAULT = 120000,
> > };
> >
> > --
> > 2.39.5
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 2/4] security: ima: introduce IMA_INIT_LATE_SYNC option
From: Yeoreum Yun @ 2026-06-09 11:13 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-integrity, paul,
roberto.sassu, noodles, jarkko, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <f135eab04611432040e4ba66cef2786424af8cc8.camel@linux.ibm.com>
On Mon, Jun 08, 2026 at 01:40:00PM -0400, Mimi Zohar wrote:
> On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> > To generate the boot_aggregate log in the IMA subsystem with
> > TPM PCR values, the TPM driver must be built as built-in and
> > must be probed before the IMA subsystem is initialized.
> >
> > However, when the TPM device operates over the FF-A protocol using
> > the CRB interface, probing fails and returns -EPROBE_DEFER if
> > the tpm_crb_ffa device — an FF-A device that provides the communication
> > interface to the tpm_crb driver — has not yet been probed.
> >
> > To ensure the TPM device operating over the FF-A protocol with
> > the CRB interface is probed before IMA initialization,
> > the following conditions must be met:
> >
> > 1. The corresponding ffa_device must be registered,
> > which is done via ffa_init().
> >
> > 2. The tpm_crb_driver must successfully probe this device via
> > tpm_crb_ffa_init().
> >
> > 3. The tpm_crb driver using CRB over FF-A can then
> > be probed successfully. (See crb_acpi_add() and
> > tpm_crb_ffa_init() for reference.)
> >
> > Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init()
> > are all registered with device_initcall, which means
> > crb_acpi_driver_init() may be invoked before ffa_init() and
> > tpm_crb_ffa_init() are completed.
> >
> > When this occurs, probing the TPM device is deferred.
> > However, the deferred probe can happen after the IMA subsystem
> > has already been initialized, since IMA initialization is performed
> > during late_initcall, and deferred_probe_initcall() is performed
> > at the same level.
> >
> > And the similar situation is reported on TPM devices attached on SPI
> > bus[0].
> >
> > To resolve this, introduce IMA_INIT_LATE_SYNC option to initialise
> > IMA at late_inicall_sync so that IMA is initialized with the TPM
> > device probed deferred.
> >
> > When this option is enabled, modules that access files in the
> > initramfs through usermode helper calls such as request_module()
> > during initcall must not be built-in. Otherwise, IMA may miss
> > measuring those files [1].
> >
> > Link: https://lore.kernel.org/all/aYXEepLhUouN5f99@earth.li/ [0]
> > Link: https://lore.kernel.org/all/2b3782398cc17ce9d355490a0c42ebce9120a9ae.camel@linux.ibm.com/ [1]
> > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> > security/integrity/ima/Kconfig | 10 ++++++++++
> > security/integrity/ima/ima_main.c | 4 ++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 862fbee2b174..75f71401fba3 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -332,4 +332,14 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
> > If set to the default value of 0, an extra half page of memory for those
> > additional measurements will be allocated.
> >
> > +config IMA_INIT_LATE_SYNC
> > + bool "Initialise IMA at late_initcall_sync"
> > + default n
> > + help
> > + This option initialises IMA at late_initcall_sync for platforms
> > + where TPM device probing is deferred.
> > + When this option is enabled, modules that access files in the
> > + initramfs through usermode helper calls such as request_module()
> > + during initcall must not be built-in. Otherwise, IMA may miss
> > + file measurements for them.
> > endif
>
> I fixed the merge conflict with the "ima: Exporting and deleting IMA measurement
> records from kernel memory" patch set. These patches are now queued in next-
> integrity-testing awaiting Paul's Ack.
Thanks!
[...]
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH] tpm: eventlog: tpm2: allow event log entries ending at the log boundary
From: ZongYao.Chen @ 2026-06-09 9:05 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Jason Gunthorpe, Nayna Jain, Tianjia Zhang,
linux-integrity, linux-kernel
In-Reply-To: <aiY_1doMftc_2WRp@kernel.org>
On Mon, Jun 08, 2026 at 07:06:45AM +0300, Jarkko Sakkinen wrote:
> This looks unnecessary turnover. Please rethink. We should be minizing
> the diff for bug fixes, not the other way around.
Hi Jarkko,
You're right, please drop this patch.
After closer investigation, the issue turned out to be caused by
non-compliant firmware rather than a kernel bug. I will not pursue this
change.
Sorry for the noise.
Zongyao
^ permalink raw reply
* Re: [PATCH v6 2/4] security: ima: introduce IMA_INIT_LATE_SYNC option
From: Mimi Zohar @ 2026-06-08 17:40 UTC (permalink / raw)
To: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity
Cc: paul, roberto.sassu, noodles, jarkko, sudeep.holla, jmorris,
serge, dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-3-yeoreum.yun@arm.com>
On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> To generate the boot_aggregate log in the IMA subsystem with
> TPM PCR values, the TPM driver must be built as built-in and
> must be probed before the IMA subsystem is initialized.
>
> However, when the TPM device operates over the FF-A protocol using
> the CRB interface, probing fails and returns -EPROBE_DEFER if
> the tpm_crb_ffa device — an FF-A device that provides the communication
> interface to the tpm_crb driver — has not yet been probed.
>
> To ensure the TPM device operating over the FF-A protocol with
> the CRB interface is probed before IMA initialization,
> the following conditions must be met:
>
> 1. The corresponding ffa_device must be registered,
> which is done via ffa_init().
>
> 2. The tpm_crb_driver must successfully probe this device via
> tpm_crb_ffa_init().
>
> 3. The tpm_crb driver using CRB over FF-A can then
> be probed successfully. (See crb_acpi_add() and
> tpm_crb_ffa_init() for reference.)
>
> Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init()
> are all registered with device_initcall, which means
> crb_acpi_driver_init() may be invoked before ffa_init() and
> tpm_crb_ffa_init() are completed.
>
> When this occurs, probing the TPM device is deferred.
> However, the deferred probe can happen after the IMA subsystem
> has already been initialized, since IMA initialization is performed
> during late_initcall, and deferred_probe_initcall() is performed
> at the same level.
>
> And the similar situation is reported on TPM devices attached on SPI
> bus[0].
>
> To resolve this, introduce IMA_INIT_LATE_SYNC option to initialise
> IMA at late_inicall_sync so that IMA is initialized with the TPM
> device probed deferred.
>
> When this option is enabled, modules that access files in the
> initramfs through usermode helper calls such as request_module()
> during initcall must not be built-in. Otherwise, IMA may miss
> measuring those files [1].
>
> Link: https://lore.kernel.org/all/aYXEepLhUouN5f99@earth.li/ [0]
> Link: https://lore.kernel.org/all/2b3782398cc17ce9d355490a0c42ebce9120a9ae.camel@linux.ibm.com/ [1]
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> security/integrity/ima/Kconfig | 10 ++++++++++
> security/integrity/ima/ima_main.c | 4 ++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 862fbee2b174..75f71401fba3 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -332,4 +332,14 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
> If set to the default value of 0, an extra half page of memory for those
> additional measurements will be allocated.
>
> +config IMA_INIT_LATE_SYNC
> + bool "Initialise IMA at late_initcall_sync"
> + default n
> + help
> + This option initialises IMA at late_initcall_sync for platforms
> + where TPM device probing is deferred.
> + When this option is enabled, modules that access files in the
> + initramfs through usermode helper calls such as request_module()
> + during initcall must not be built-in. Otherwise, IMA may miss
> + file measurements for them.
> endif
I fixed the merge conflict with the "ima: Exporting and deleting IMA measurement
records from kernel memory" patch set. These patches are now queued in next-
integrity-testing awaiting Paul's Ack.
Mimi
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5cea53fc36df..1cfae4b83dc5 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1337,5 +1337,9 @@ DEFINE_LSM(ima) = {
> .order = LSM_ORDER_LAST,
> .blobs = &ima_blob_sizes,
> /* Start IMA after the TPM is available */
> +#ifndef CONFIG_IMA_INIT_LATE_SYNC
> .initcall_late = init_ima,
> +#else
> + .initcall_late_sync = init_ima,
> +#endif
> };
^ permalink raw reply
* Re: [PATCH v1] tpm_crb: Check ACPI_COMPANION() against NULL during probe
From: Rafael J. Wysocki @ 2026-06-08 17:35 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, Andy Shevchenko,
LKML, Linux ACPI
In-Reply-To: <CAJZ5v0jQUQ85MpyPZNbLmxqaGGvsTBKsdf8gdPNmFFSpZkj4eQ@mail.gmail.com>
On Tue, May 19, 2026 at 11:01 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, May 16, 2026 at 3:15 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Tue, May 12, 2026 at 06:16:23PM +0200, Rafael J. Wysocki wrote:
> > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > >
> > > Every platform driver can be forced to match a device that doesn't match
> > > its list of device IDs because of device_match_driver_override(), so
> > > platform drivers that rely on the existence of a device's ACPI companion
> > > object need to verify its presence.
> > >
> > > Accordingly, add a requisite ACPI_COMPANION() check against NULL to the
> > > tpm_crb driver.
> > >
> > > Fixes: 48fe2cddc85c ("tpm_crb: Convert ACPI driver to a platform one")
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > > drivers/char/tpm/tpm_crb.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > --- a/drivers/char/tpm/tpm_crb.c
> > > +++ b/drivers/char/tpm/tpm_crb.c
> > > @@ -786,8 +786,8 @@ static int crb_map_pluton(struct device
> > > static int crb_acpi_probe(struct platform_device *pdev)
> > > {
> > > struct device *dev = &pdev->dev;
> > > - struct acpi_device *device = ACPI_COMPANION(dev);
> > > struct acpi_table_tpm2 *buf;
> > > + struct acpi_device *device;
> > > struct crb_priv *priv;
> > > struct tpm_chip *chip;
> > > struct tpm2_crb_smc *crb_smc;
> > > @@ -797,6 +797,10 @@ static int crb_acpi_probe(struct platfor
> > > u32 sm;
> > > int rc;
> > >
> > > + device = ACPI_COMPANION(dev);
> > > + if (!device)
> > > + return -ENODEV;
> > > +
> > > status = acpi_get_table(ACPI_SIG_TPM2, 1,
> > > (struct acpi_table_header **) &buf);
> > > if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
> > >
> > >
> > >
> >
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Thanks!
>
> So do you want me to pick up this one?
I took the silence as consent and picked it up. If you'd rather route
it differently, please let me know.
Thanks!
^ permalink raw reply
* Re: [PATCH v6 1/4] security: lsm: allow LSMs to register for late_initcall_sync init
From: Paul Moore @ 2026-06-08 17:24 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
roberto.sassu, noodles, jarkko, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <9b499a2a3101cadcfdcd6fc32289f54df10fea80.camel@linux.ibm.com>
On Mon, Jun 8, 2026 at 12:52 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> > There are situations where LSMs have dependencies that might mean they
> > want to be initialised later in the boot process, to ensure those
> > dependencies are available. In particular there are some TPM setups (Arm
> > FF-A devices, SPI attached TPMs) required by IMA which are not
> > guaranteed to be initialised for regular initcall_late.
> >
> > Add an initcall_late_sync option that can be used in these situations.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>
> Cc: Paul Moore <paul@paul-moore.com>
>
> Jarkko has already queued 4/4. Paul, can we get an Ack from you?
It's in my queue, but with the merge window opening next week, this is
definitely not suitable for the upcoming merge window, so it's not
near the top of my review list.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v6 1/4] security: lsm: allow LSMs to register for late_initcall_sync init
From: Mimi Zohar @ 2026-06-08 16:52 UTC (permalink / raw)
To: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity
Cc: paul, roberto.sassu, noodles, jarkko, sudeep.holla, jmorris,
serge, dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-2-yeoreum.yun@arm.com>
On Fri, 2026-06-05 at 15:43 +0100, Yeoreum Yun wrote:
> There are situations where LSMs have dependencies that might mean they
> want to be initialised later in the boot process, to ensure those
> dependencies are available. In particular there are some TPM setups (Arm
> FF-A devices, SPI attached TPMs) required by IMA which are not
> guaranteed to be initialised for regular initcall_late.
>
> Add an initcall_late_sync option that can be used in these situations.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Cc: Paul Moore <paul@paul-moore.com>
Jarkko has already queued 4/4. Paul, can we get an Ack from you?
Mimi
^ permalink raw reply
* Re: [PATCH v7 00/12] ima: Exporting and deleting IMA measurement records from kernel memory
From: Roberto Sassu @ 2026-06-08 16:22 UTC (permalink / raw)
To: Mimi Zohar, corbet, skhan, 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: <22debae414a07a3cbdb62e723dfb737d6d4bd693.camel@linux.ibm.com>
On Mon, 2026-06-08 at 12:21 -0400, Mimi Zohar wrote:
> On Fri, 2026-06-05 at 19:22 +0200, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > Introduction
> > ============
> >
> > The IMA measurements list is currently stored in the kernel memory.
> > Memory occupation grows linearly with the number of records, and can
> > become a problem especially in environments with reduced resources.
> >
> > While there is an advantage in keeping the IMA measurements list in
> > kernel memory, so that it is always available for reading from the
> > securityfs interfaces, storing it elsewhere would make it possible to
> > free precious memory for other kernel usage.
> >
> > The IMA measurements list needs to be retained and safely stored for new
> > attestation servers to validate it. Assuming the IMA measurements list
> > is properly saved, storing it outside the kernel does not introduce
> > security issues, since its integrity is anyway protected by the TPM.
> >
> > Hence, the new IMA staging mechanism is introduced to export IMA
> > measurements to user space and delete them from kernel space.
> >
> > Staging consists in atomically moving the current measurements list to a
> > temporary list, so that measurements can be deleted afterwards. The
> > staging operation locks the hot path (racing with addition of new
> > measurements) for a very short time, only for swapping the list
> > pointers. Deletion of the measurements instead is done locklessly, away
> > from the hot path.
> >
> > There are two flavors of the staging mechanism. In the staging with
> > prompt, all current measurements are staged, read and deleted upon
> > confirmation. In the staging and deleting flavor, N measurements are
> > staged from the beginning of the current measurements list and
> > immediately deleted without confirmation.
> >
> >
> > Usage
> > =====
> >
> > The IMA staging mechanism can be enabled from the kernel configuration
> > with the CONFIG_IMA_STAGING option. This option prevents inadvertently
> > removing the IMA measurement list on systems which do not properly save
> > it.
> >
> > If the option is enabled, IMA duplicates the current securityfs
> > measurements interfaces (both binary and ASCII), by adding the _staged
> > file suffix. Both the original and the staging interfaces gain the write
> > permission for the root user and group, but require the process to have
> > CAP_SYS_ADMIN set.
> >
> > The staging mechanism supports two flavors.
> >
> > Staging with prompt
> > ~~~~~~~~~~~~~~~~~~~
> >
> > The current measurement list is moved to a temporary staging area,
> > allowing it to be saved to external storage, before being deleted upon
> > confirmation.
> >
> > This staging process is achieved with the following steps.
> >
> > 1. echo A > <_staged interface>: the user requests IMA to stage the
> > entire measurements list;
> > 2. cat <_staged interface>: the user reads the staged measurements;
> > 3. echo D > <_staged interface>: the user requests IMA to delete
> > staged measurements.
> >
> > Staging and deleting
> > ~~~~~~~~~~~~~~~~~~~~
> >
> > N measurements are staged to a temporary staging area, and immediately
> > deleted without further confirmation.
> >
> > This staging process is achieved with the following steps.
> >
> > 1. cat <original interface>: the user reads the current measurements
> > list and determines what the value N for staging should be;
> > 2. echo N > <original interface>: the user requests IMA to delete N
> > measurements from the current measurements list.
> >
> >
> > Management of Staged Measurements
> > =================================
> >
> > Since with the staging mechanism measurement records are removed from
> > the kernel, the staged measurements need to be saved in a storage and
> > concatenated together, so that they can be presented during remote
> > attestation as if staging was never done. This task can be accomplished
> > by a remote attestation agent modified to support staging, or a system
> > service.
> >
> >
> > Patch set content
> > =================
> >
> > Patches 1-8 are preparatory patches to quickly replace the hash table,
> > maintain separate counters for the different measurements list types,
> > mediate access to the measurements list interface, and simplify the staging
> > patches.
> >
> > Patch 9 introduces the staging with prompt flavor. Patch 10 makes it
> > possible to flush the hash table when deleting all the staged measurements.
> > Patch 11 introduces the staging and deleting flavor. Patch 12 adds the
> > documentation of the staging mechanism.
> >
> >
> > Changelog
> > =========
> >
> > v6:
> > - Make ima_extend_list_mutex as static since it is not needed anymore by
> > ima_dump_measurement_list() (suggested by Mimi)
> > - Export ima_flush_htable in patch 11 instead of 10 (suggested by Mimi)
> > - Add clarification in the documentation regarding a proactive remote
> > attestation agent, and storing all the measurements in the storage
> > (suggested by Mimi)
>
> Roberto, thank you for making these and all the other changes. The patch set is
> now queued in next-integrity.
Perfect, thank you!
Roberto
^ permalink raw reply
* Re: [PATCH v7 00/12] ima: Exporting and deleting IMA measurement records from kernel memory
From: Mimi Zohar @ 2026-06-08 16:21 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, 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: <20260605172236.2042045-1-roberto.sassu@huaweicloud.com>
On Fri, 2026-06-05 at 19:22 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduction
> ============
>
> The IMA measurements list is currently stored in the kernel memory.
> Memory occupation grows linearly with the number of records, and can
> become a problem especially in environments with reduced resources.
>
> While there is an advantage in keeping the IMA measurements list in
> kernel memory, so that it is always available for reading from the
> securityfs interfaces, storing it elsewhere would make it possible to
> free precious memory for other kernel usage.
>
> The IMA measurements list needs to be retained and safely stored for new
> attestation servers to validate it. Assuming the IMA measurements list
> is properly saved, storing it outside the kernel does not introduce
> security issues, since its integrity is anyway protected by the TPM.
>
> Hence, the new IMA staging mechanism is introduced to export IMA
> measurements to user space and delete them from kernel space.
>
> Staging consists in atomically moving the current measurements list to a
> temporary list, so that measurements can be deleted afterwards. The
> staging operation locks the hot path (racing with addition of new
> measurements) for a very short time, only for swapping the list
> pointers. Deletion of the measurements instead is done locklessly, away
> from the hot path.
>
> There are two flavors of the staging mechanism. In the staging with
> prompt, all current measurements are staged, read and deleted upon
> confirmation. In the staging and deleting flavor, N measurements are
> staged from the beginning of the current measurements list and
> immediately deleted without confirmation.
>
>
> Usage
> =====
>
> The IMA staging mechanism can be enabled from the kernel configuration
> with the CONFIG_IMA_STAGING option. This option prevents inadvertently
> removing the IMA measurement list on systems which do not properly save
> it.
>
> If the option is enabled, IMA duplicates the current securityfs
> measurements interfaces (both binary and ASCII), by adding the _staged
> file suffix. Both the original and the staging interfaces gain the write
> permission for the root user and group, but require the process to have
> CAP_SYS_ADMIN set.
>
> The staging mechanism supports two flavors.
>
> Staging with prompt
> ~~~~~~~~~~~~~~~~~~~
>
> The current measurement list is moved to a temporary staging area,
> allowing it to be saved to external storage, before being deleted upon
> confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. echo A > <_staged interface>: the user requests IMA to stage the
> entire measurements list;
> 2. cat <_staged interface>: the user reads the staged measurements;
> 3. echo D > <_staged interface>: the user requests IMA to delete
> staged measurements.
>
> Staging and deleting
> ~~~~~~~~~~~~~~~~~~~~
>
> N measurements are staged to a temporary staging area, and immediately
> deleted without further confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. cat <original interface>: the user reads the current measurements
> list and determines what the value N for staging should be;
> 2. echo N > <original interface>: the user requests IMA to delete N
> measurements from the current measurements list.
>
>
> Management of Staged Measurements
> =================================
>
> Since with the staging mechanism measurement records are removed from
> the kernel, the staged measurements need to be saved in a storage and
> concatenated together, so that they can be presented during remote
> attestation as if staging was never done. This task can be accomplished
> by a remote attestation agent modified to support staging, or a system
> service.
>
>
> Patch set content
> =================
>
> Patches 1-8 are preparatory patches to quickly replace the hash table,
> maintain separate counters for the different measurements list types,
> mediate access to the measurements list interface, and simplify the staging
> patches.
>
> Patch 9 introduces the staging with prompt flavor. Patch 10 makes it
> possible to flush the hash table when deleting all the staged measurements.
> Patch 11 introduces the staging and deleting flavor. Patch 12 adds the
> documentation of the staging mechanism.
>
>
> Changelog
> =========
>
> v6:
> - Make ima_extend_list_mutex as static since it is not needed anymore by
> ima_dump_measurement_list() (suggested by Mimi)
> - Export ima_flush_htable in patch 11 instead of 10 (suggested by Mimi)
> - Add clarification in the documentation regarding a proactive remote
> attestation agent, and storing all the measurements in the storage
> (suggested by Mimi)
Roberto, thank you for making these and all the other changes. The patch set is
now queued in next-integrity.
Mimi
^ permalink raw reply
* RE: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
From: Benoit HOUYERE @ 2026-06-08 14:46 UTC (permalink / raw)
To: Serge E. Hallyn, Jarkko Sakkinen
Cc: linux-integrity@vger.kernel.org, Frédéric Jouen,
Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM,
Laurent CHARPENTIER
In-Reply-To: <aMzSyCQks3NlMhPI@mail.hallyn.com>
Hello Serge and Jarkko,
We have detected a regression with this fix. Indeed, it miss one zero on TPM_LONG_LONG. Initial value was 300000 and not 30000.
> + {TPM2_CC_CREATE_PRIMARY, 30000},
> + {TPM2_CC_CREATE, 30000},
> + {TPM2_CC_CREATE_LOADED, 30000},
> +enum tpm2_durations {
> TPM2_DURATION_SHORT = 20,
> - TPM2_DURATION_MEDIUM = 750,
> TPM2_DURATION_LONG = 2000,
> - TPM2_DURATION_LONG_LONG = 300000,
> TPM2_DURATION_DEFAULT = 120000,
> };
Best Regards, Cordialement, Cordialmente, Hälsningar, 最好的问候, Mit besten Grüßen, 真心を込めて, 진심으로
Benoit HOUYERE | Tel: +33 6 14 22 81 30
TPM specialist
-----Original Message-----
From: Serge E. Hallyn <serge@hallyn.com>
Sent: Friday, September 19, 2025 5:49 AM
To: Jarkko Sakkinen <jarkko@kernel.org>
Cc: linux-integrity@vger.kernel.org; Frédéric Jouen <fjouen@sealsq.com>; Peter Huewe <peterhuewe@gmx.de>; Jason Gunthorpe <jgg@ziepe.ca>; James Bottomley <James.Bottomley@hansenpartnership.com>; Mimi Zohar <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn <serge@hallyn.com>; open list <linux-kernel@vger.kernel.org>; open list:KEYS-TRUSTED <keyrings@vger.kernel.org>; open list:SECURITY SUBSYSTEM <linux-security-module@vger.kernel.org>
Subject: Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
On Thu, Sep 18, 2025 at 10:30:18PM +0300, Jarkko Sakkinen wrote:
> The current shenanigans for duration calculation introduce too much
> complexity for a trivial problem, and further the code is hard to
> patch and maintain.
>
> Address these issues with a flat look-up table, which is easy to
> understand and patch. If leaf driver specific patching is required in
> future, it is easy enough to make a copy of this table during driver
> initialization and add the chip parameter back.
>
> 'chip->duration' is retained for TPM 1.x.
>
> As the first entry for this new behavior address TCG spec update
> mentioned in this issue:
>
> https://github.com/raspberrypi/linux/issues/7054
>
> Therefore, for TPM_SelfTest the duration is set to 3000 ms.
>
> This does not categorize a as bug, given that this is introduced to
> the spec after the feature was originally made.
>
> Cc: Frédéric Jouen <fjouen@sealsq.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
fwiw (which shouldn't be much) looks good to me, but two questions, one here and one below.
First, it looks like in the existing code it is possible for a tpm2 chip to set its own timeouts and then set the TPM_CHIP_FLAG_HAVE_TIMEOUTS flag to avoid using the defaults, but I don't see anything using that in-tree. Is it possible that there are out of tree drivers that will be sabotaged here? Or am I misunderstanding that completely?
> ---
> v2:
> - Add the missing msec_to_jiffies() calls.
> - Drop redundant stuff.
> ---
> drivers/char/tpm/tpm-interface.c | 2 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/char/tpm/tpm2-cmd.c | 127 ++++++++-----------------------
> include/linux/tpm.h | 5 +-
> 4 files changed, 37 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c
> b/drivers/char/tpm/tpm-interface.c
> index b71725827743..c9f173001d0e 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -52,7 +52,7 @@ MODULE_PARM_DESC(suspend_pcr, unsigned long
> tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal) {
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - return tpm2_calc_ordinal_duration(chip, ordinal);
> + return tpm2_calc_ordinal_duration(ordinal);
> else
> return tpm1_calc_ordinal_duration(chip, ordinal); } diff --git
> a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> 7bb87fa5f7a1..2726bd38e5ac 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -299,7 +299,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32
> property_id, ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
> int tpm2_auto_startup(struct tpm_chip *chip); void
> tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type); -unsigned
> long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> +unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
> int tpm2_probe(struct tpm_chip *chip); int
> tpm2_get_cc_attrs_tbl(struct tpm_chip *chip); int tpm2_find_cc(struct
> tpm_chip *chip, u32 cc); diff --git a/drivers/char/tpm/tpm2-cmd.c
> b/drivers/char/tpm/tpm2-cmd.c index 524d802ede26..7d77f6fbc152 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -28,120 +28,57 @@ static struct tpm2_hash tpm2_hash_map[] = {
>
> int tpm2_get_timeouts(struct tpm_chip *chip) {
> - /* Fixed timeouts for TPM2 */
> chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
> chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
> chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
> chip->timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D);
> -
> - /* PTP spec timeouts */
> - chip->duration[TPM_SHORT] = msecs_to_jiffies(TPM2_DURATION_SHORT);
> - chip->duration[TPM_MEDIUM] = msecs_to_jiffies(TPM2_DURATION_MEDIUM);
> - chip->duration[TPM_LONG] = msecs_to_jiffies(TPM2_DURATION_LONG);
> -
> - /* Key creation commands long timeouts */
> - chip->duration[TPM_LONG_LONG] =
> - msecs_to_jiffies(TPM2_DURATION_LONG_LONG);
> -
> chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
> -
> return 0;
> }
>
> -/**
> - * tpm2_ordinal_duration_index() - returns an index to the chip
> duration table
> - * @ordinal: TPM command ordinal.
> - *
> - * The function returns an index to the chip duration table
> - * (enum tpm_duration), that describes the maximum amount of
> - * time the chip could take to return the result for a particular ordinal.
> - *
> - * The values of the MEDIUM, and LONG durations are taken
> - * from the PC Client Profile (PTP) specification (750, 2000 msec)
> - *
> - * LONG_LONG is for commands that generates keys which empirically
> takes
> - * a longer time on some systems.
> - *
> - * Return:
> - * * TPM_MEDIUM
> - * * TPM_LONG
> - * * TPM_LONG_LONG
> - * * TPM_UNDEFINED
> +/*
> + * Contains the maximum durations in milliseconds for TPM2 commands.
> */
> -static u8 tpm2_ordinal_duration_index(u32 ordinal) -{
> - switch (ordinal) {
> - /* Startup */
> - case TPM2_CC_STARTUP: /* 144 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_SELF_TEST: /* 143 */
> - return TPM_LONG;
> -
> - case TPM2_CC_GET_RANDOM: /* 17B */
> - return TPM_LONG;
> -
> - case TPM2_CC_SEQUENCE_UPDATE: /* 15C */
> - return TPM_MEDIUM;
> - case TPM2_CC_SEQUENCE_COMPLETE: /* 13E */
> - return TPM_MEDIUM;
> - case TPM2_CC_EVENT_SEQUENCE_COMPLETE: /* 185 */
> - return TPM_MEDIUM;
> - case TPM2_CC_HASH_SEQUENCE_START: /* 186 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_VERIFY_SIGNATURE: /* 177 */
> - return TPM_LONG_LONG;
> -
> - case TPM2_CC_PCR_EXTEND: /* 182 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_HIERARCHY_CONTROL: /* 121 */
> - return TPM_LONG;
> - case TPM2_CC_HIERARCHY_CHANGE_AUTH: /* 129 */
> - return TPM_LONG;
> -
> - case TPM2_CC_GET_CAPABILITY: /* 17A */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_NV_READ: /* 14E */
> - return TPM_LONG;
> -
> - case TPM2_CC_CREATE_PRIMARY: /* 131 */
> - return TPM_LONG_LONG;
> - case TPM2_CC_CREATE: /* 153 */
> - return TPM_LONG_LONG;
> - case TPM2_CC_CREATE_LOADED: /* 191 */
> - return TPM_LONG_LONG;
> -
> - default:
> - return TPM_UNDEFINED;
> - }
> -}
> +static const struct {
> + unsigned long ordinal;
> + unsigned long duration;
> +} tpm2_ordinal_duration_map[] = {
> + {TPM2_CC_STARTUP, 750},
> + {TPM2_CC_SELF_TEST, 3000},
I assume you intended to increase TPM2_CC_SELF_TEST from 2000 to 3000 here? But it's not mentioned in the commit, so making sure...
> + {TPM2_CC_GET_RANDOM, 2000},
> + {TPM2_CC_SEQUENCE_UPDATE, 750},
> + {TPM2_CC_SEQUENCE_COMPLETE, 750},
> + {TPM2_CC_EVENT_SEQUENCE_COMPLETE, 750},
> + {TPM2_CC_HASH_SEQUENCE_START, 750},
> + {TPM2_CC_VERIFY_SIGNATURE, 30000},
> + {TPM2_CC_PCR_EXTEND, 750},
> + {TPM2_CC_HIERARCHY_CONTROL, 2000},
> + {TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
> + {TPM2_CC_GET_CAPABILITY, 750},
> + {TPM2_CC_NV_READ, 2000},
> + {TPM2_CC_CREATE_PRIMARY, 30000},
> + {TPM2_CC_CREATE, 30000},
> + {TPM2_CC_CREATE_LOADED, 30000},
> +};
>
> /**
> - * tpm2_calc_ordinal_duration() - calculate the maximum command duration
> - * @chip: TPM chip to use.
> + * tpm2_calc_ordinal_duration() - Calculate the maximum command
> + duration
> * @ordinal: TPM command ordinal.
> *
> - * The function returns the maximum amount of time the chip could
> take
> - * to return the result for a particular ordinal in jiffies.
> - *
> - * Return: A maximal duration time for an ordinal in jiffies.
> + * Returns the maximum amount of time the chip is expected by kernel
> + to
> + * take in jiffies.
> */
> -unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32
> ordinal)
> +unsigned long tpm2_calc_ordinal_duration(u32 ordinal)
> {
> - unsigned int index;
> + int i;
>
> - index = tpm2_ordinal_duration_index(ordinal);
> + for (i = 0; i < ARRAY_SIZE(tpm2_ordinal_duration_map); i++)
> + if (ordinal == tpm2_ordinal_duration_map[i].ordinal)
> + return msecs_to_jiffies(tpm2_ordinal_duration_map[i].duration);
>
> - if (index != TPM_UNDEFINED)
> - return chip->duration[index];
> - else
> - return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> + return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> }
>
> -
> struct tpm2_pcr_read_out {
> __be32 update_cnt;
> __be32 pcr_selects_cnt;
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h index
> b0e9eb5ef022..dc0338a783f3 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -228,10 +228,11 @@ enum tpm2_timeouts {
> TPM2_TIMEOUT_B = 4000,
> TPM2_TIMEOUT_C = 200,
> TPM2_TIMEOUT_D = 30,
> +};
> +
> +enum tpm2_durations {
> TPM2_DURATION_SHORT = 20,
> - TPM2_DURATION_MEDIUM = 750,
> TPM2_DURATION_LONG = 2000,
> - TPM2_DURATION_LONG_LONG = 300000,
> TPM2_DURATION_DEFAULT = 120000,
> };
>
> --
> 2.39.5
^ permalink raw reply
* Re: [PATCH v6 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Yeoreum Yun @ 2026-06-08 7:15 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <aiZJAR3-JACaDcwR@kernel.org>
On Mon, Jun 08, 2026 at 07:45:53AM +0300, Jarkko Sakkinen wrote:
> On Fri, Jun 05, 2026 at 03:43:25PM +0100, Yeoreum Yun wrote:
> > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in")
> > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> >
> > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > initialises IMA at the late_initcall_sync level, so this change is no
> > longer required.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> > 1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> > index 99f1c1e5644b..025c4d4b17ca 100644
> > --- a/drivers/char/tpm/tpm_crb_ffa.c
> > +++ b/drivers/char/tpm/tpm_crb_ffa.c
> > @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> > */
> > int tpm_crb_ffa_init(void)
> > {
> > - int ret = 0;
> > -
> > - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> > - ret = ffa_register(&tpm_crb_ffa_driver);
> > - if (ret) {
> > - tpm_crb_ffa = ERR_PTR(-ENODEV);
> > - return ret;
> > - }
> > - }
> > -
> > if (!tpm_crb_ffa)
> > - ret = -ENOENT;
> > + return -ENOENT;
> >
> > if (IS_ERR_VALUE(tpm_crb_ffa))
> > - ret = -ENODEV;
> > + return -ENODEV;
> >
> > - return ret;
> > + return 0;
> > }
> > EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
> >
> > @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> > .id_table = tpm_crb_ffa_device_id,
> > };
> >
> > -#ifdef MODULE
> > module_ffa_driver(tpm_crb_ffa_driver);
> > -#endif
> >
> > MODULE_AUTHOR("Arm");
> > MODULE_DESCRIPTION("TPM CRB FFA driver");
> > --
> > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> >
>
> Is this different I applied?
>
> If yes, I'll swap (if mandatory).
No difference. Thanks! ;)
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [PATCH next] keys: Replace strcpy(derived_buf, "AUTH_KEY") with strscpy(..., HASH_SIZE)
From: Jarkko Sakkinen @ 2026-06-08 4:46 UTC (permalink / raw)
To: david.laight.linux
Cc: Kees Cook, linux-hardening, Arnd Bergmann, keyrings,
linux-integrity, linux-kernel, linux-security-module,
David Howells, James Morris, Mimi Zohar, Paul Moore,
Serge E. Hallyn
In-Reply-To: <20260606202633.5018-9-david.laight.linux@gmail.com>
On Sat, Jun 06, 2026 at 09:26:03PM +0100, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> derived_buf is guaranteed to be HASH_SIZE - and it is more than enough.
> The strscpy() degenerates into an memcpy() (as did the strcpy()).
> Do the same for the associated "ENC_KEY" copy.
>
> Removes a possibly unbounded strcpy().
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> This is one of a group of patches that remove potentially unbounded
> strcpy() calls.
>
> They are mostly replaced by strscpy() or, when strlen() has just been
> called, with memcpy() (usually including the '\0').
>
> Calls with copy string literals into arrays are left unchanged.
> They are safe and easily detected as such.
>
> The changes were made by getting the compiler to detect the calls and
> then fixing the code by hand.
>
> Note that all the changes are only compile tested.
>
> Some Makefiles were changed to allow files to contain strcpy().
> As well as 'difficult to fix' files, this included 'show' functions
> as they really need to use sysfs_emit() or seq_printf().
>
> All the patches are being sent individually to avoid very long cc lists.
> Apologies for the terse commit messages and likely unexpected tags.
> (There are about 100 patches in total.)
>
> security/keys/encrypted-keys/encrypted.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 56b531587a1e..59cb77b237b3 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -343,9 +343,9 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
> return -ENOMEM;
>
> if (key_type)
> - strcpy(derived_buf, "AUTH_KEY");
> + strscpy(derived_buf, "AUTH_KEY", HASH_SIZE);
> else
> - strcpy(derived_buf, "ENC_KEY");
> + strscpy(derived_buf, "ENC_KEY", HASH_SIZE);
>
> memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
> master_keylen);
> --
> 2.39.5
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 4:45 UTC (permalink / raw)
To: Yeoreum Yun
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-5-yeoreum.yun@arm.com>
On Fri, Jun 05, 2026 at 03:43:25PM +0100, Yeoreum Yun wrote:
> commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in")
> probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
>
> However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> initialises IMA at the late_initcall_sync level, so this change is no
> longer required.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 99f1c1e5644b..025c4d4b17ca 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> */
> int tpm_crb_ffa_init(void)
> {
> - int ret = 0;
> -
> - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> - ret = ffa_register(&tpm_crb_ffa_driver);
> - if (ret) {
> - tpm_crb_ffa = ERR_PTR(-ENODEV);
> - return ret;
> - }
> - }
> -
> if (!tpm_crb_ffa)
> - ret = -ENOENT;
> + return -ENOENT;
>
> if (IS_ERR_VALUE(tpm_crb_ffa))
> - ret = -ENODEV;
> + return -ENODEV;
>
> - return ret;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>
> @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> .id_table = tpm_crb_ffa_device_id,
> };
>
> -#ifdef MODULE
> module_ffa_driver(tpm_crb_ffa_driver);
> -#endif
>
> MODULE_AUTHOR("Arm");
> MODULE_DESCRIPTION("TPM CRB FFA driver");
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
Is this different I applied?
If yes, I'll swap (if mandatory).
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] tpm: eventlog: tpm2: allow event log entries ending at the log boundary
From: Jarkko Sakkinen @ 2026-06-08 4:06 UTC (permalink / raw)
To: ZongYao.Chen
Cc: Peter Huewe, Jason Gunthorpe, Nayna Jain, Tianjia Zhang,
linux-integrity, linux-kernel
In-Reply-To: <20260604025356.3436943-1-ZongYao.Chen@linux.alibaba.com>
On Thu, Jun 04, 2026 at 10:53:47AM +0800, ZongYao.Chen@linux.alibaba.com wrote:
> From: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
>
> The TPM2 firmware event log buffer is a half-open range:
> [bios_event_log, bios_event_log_end). An entry ending exactly at
> bios_event_log_end is still inside the buffer; only an entry extending
> past that address is malformed.
>
> The TPM2 seq_file iterator did not handle this boundary consistently.
> The TCG_EfiSpecIdEvent header had to satisfy "addr + size < limit".
> Later events were rejected when "addr + size >= limit". Firmware that
> packs the final measurement tightly at the end of the log can therefore
> lose that measurement. If it is the first measurement after the spec ID
> header, binary_bios_measurements shows only the header.
>
> This has been observed on bare-metal systems whose UEFI enables the SM3
> PCR bank, but the bug is not SM3-specific. Any tightly packed TPM2 log
> whose final event ends at bios_event_log_end can hit it.
>
> Accept entries that end exactly at the log boundary by rejecting only
> "addr + size > limit". An accepted boundary entry has its last byte at
> limit - 1, so this does not allow reading past the buffer. Keep
> zero-length entries rejected.
>
> Also treat addr >= limit as EOF in tpm2_bios_measurements_start().
> After seq_file restarts from a later position, start() can scan past a
> valid final entry and leave addr equal to bios_event_log_end. That
> address is the end marker, not another event header.
>
> Leave the "marker >= limit" check in tpm2_bios_measurements_next()
> unchanged. There, marker is already the start of the next event, so
> "marker == limit" means EOF.
This is the most unclear bug description I've read for a long
time. Please explain what's the problem in simple teerms and
how this solves this. Mixing up pseudo-code and text does not
help.
>
> Fixes: 4d23cc323cdb ("tpm: add securityfs support for TPM 2.0 firmware event log")
> Signed-off-by: Zongyao Chen <ZongYao.Chen@linux.alibaba.com>
> ---
> drivers/char/tpm/eventlog/tpm2.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
> index 37a05800980c..6b65d872e43a 100644
> --- a/drivers/char/tpm/eventlog/tpm2.c
> +++ b/drivers/char/tpm/eventlog/tpm2.c
> @@ -54,31 +54,38 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
> size = struct_size(event_header, event, event_header->event_size);
>
> if (*pos == 0) {
> - if (addr + size < limit) {
> - if ((event_header->event_type == 0) &&
> - (event_header->event_size == 0))
> - return NULL;
> - return SEQ_START_TOKEN;
> - }
> + if (addr + size > limit)
> + return NULL;
> + if (event_header->event_type == 0 &&
> + event_header->event_size == 0)
> + return NULL;
> + return SEQ_START_TOKEN;
This looks unnecessary turnover. Please rethink. We should be minizing
the diff for bug fixes, not the other way around.
> }
>
> if (*pos > 0) {
> addr += size;
> + if (addr >= limit)
> + return NULL;
> event = addr;
> size = calc_tpm2_event_size(event, event_header);
> - if ((addr + size >= limit) || (size == 0))
> + if ((addr + size > limit) || size == 0)
> return NULL;
> }
>
> for (i = 0; i < (*pos - 1); i++) {
> + if (addr >= limit)
> + return NULL;
> event = addr;
> size = calc_tpm2_event_size(event, event_header);
>
> - if ((addr + size >= limit) || (size == 0))
> + if ((addr + size > limit) || size == 0)
> return NULL;
> addr += size;
> }
>
> + if (addr >= limit)
> + return NULL;
> +
> return addr;
> }
>
> @@ -115,7 +122,7 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
> event = v;
>
> event_size = calc_tpm2_event_size(event, event_header);
> - if (((v + event_size) >= limit) || (event_size == 0))
> + if (((v + event_size) > limit) || event_size == 0)
> return NULL;
>
> return v;
> --
> 2.47.3
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v5 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 4:01 UTC (permalink / raw)
To: Yeoreum Yun
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <ah7TAk3iItltddzT@e129823.arm.com>
On Tue, Jun 02, 2026 at 01:56:34PM +0100, Yeoreum Yun wrote:
> > On Mon, Jun 01, 2026 at 03:27:49PM +0100, Yeoreum Yun wrote:
> > > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's build_in")
> > > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> > >
> > > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > > initialises IMA at the late_initcall_sync level, so this change is no
> > > longer required.
> > >
> > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Might be rb tag?. Thanks!
>
> --
> Sincerely,
> Yeoreum Yun
SOB is enough information as it is a small change. I pushed it to
for-next-tpm.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v4 3/3] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 3:54 UTC (permalink / raw)
To: Sudeep Holla
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
paul, zohar, roberto.sassu, noodles, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260602-vague-proficient-gibbon-b005c0@sudeepholla>
On Tue, Jun 02, 2026 at 10:57:25AM +0100, Sudeep Holla wrote:
> On Tue, Jun 02, 2026 at 04:50:42AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jun 01, 2026 at 09:54:08AM +0100, Sudeep Holla wrote:
> > > On Mon, Jun 01, 2026 at 08:17:13AM +0100, Yeoreum Yun wrote:
> > > > Hi Jarkko,
> > > >
> > > > Sorry for late answer.
> > > >
> > > > > On Mon, May 25, 2026 at 08:54:04AM +0100, Yeoreum Yun wrote:
> > > > > > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's build_in")
> > > > > > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> > > > > >
> > > > > > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > > > > > initialises IMA at the late_initcall_sync level, so this change is no
> > > > > > longer required.
> > > > > >
> > > > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > > > > > ---
> > > > > > drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> > > > > > 1 file changed, 3 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > index 99f1c1e5644b..025c4d4b17ca 100644
> > > > > > --- a/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> > > > > > */
> > > > > > int tpm_crb_ffa_init(void)
> > > > > > {
> > > > > > - int ret = 0;
> > > > > > -
> > > > > > - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> > > > > > - ret = ffa_register(&tpm_crb_ffa_driver);
> > > > > > - if (ret) {
> > > > > > - tpm_crb_ffa = ERR_PTR(-ENODEV);
> > > > > > - return ret;
> > > > > > - }
> > > > > > - }
> > > > > > -
> > > > > > if (!tpm_crb_ffa)
> > > > > > - ret = -ENOENT;
> > > > > > + return -ENOENT;
> > > > > >
> > > > > > if (IS_ERR_VALUE(tpm_crb_ffa))
> > > > > > - ret = -ENODEV;
> > > > > > + return -ENODEV;
> > > > > >
> > > > > > - return ret;
> > > > > > + return 0;
> > > > > > }
> > > > > > EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
> > > > > >
> > > > > > @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> > > > > > .id_table = tpm_crb_ffa_device_id,
> > > > > > };
> > > > > >
> > > > > > -#ifdef MODULE
> > > > > > module_ffa_driver(tpm_crb_ffa_driver);
> > > > > > -#endif
> > > > > >
> > > > > > MODULE_AUTHOR("Arm");
> > > > > > MODULE_DESCRIPTION("TPM CRB FFA driver");
> > > > > > --
> > > > > > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> > > > > >
> > > > >
> > > > > How we would sync up this patch? Through which tree etc.
> > > >
> > > > IMHO, the IMA relevant thing would be into IMA tree,
> > > > However I think this patch would be much easier to sync into Sudeep's
> > > > FF-A tree where ff-a initilisation is reverted to device_initcall
> > > > unless you're uncomfortable.
> > > >
> > > > For this, It might be better to split this patch from this series
> > > > since by above and defer probe of ff-a would make a register failure
> > > > of registering tpm_crb_ffa driver which is built-in.
> > > >
> > > > @Sudeep what do you think?
> > > >
> > >
> > > IIRC, there is/was no dependency between these and FF-A patches that are
> > > queued in terms of build. I agree there may be dependency to get all the
> > > functionality but we can resort to linux-next for that. FF-A is not enabled
> > > in the defconfig, so anyone working on FF-A + TPM must enable then and can
> > > rely on -next IMHO.
> > >
> > > That said, I have already sent PR for FF-A to SoC team and it is already
> > > queued for v7.2. I don't have any other plans unless they are fixes.
> >
> > Works for me.
> >
>
> Sorry if I was not clear. I haven't included any TPM patches in this series
> as part of my FF-A pull request queued for v7.2. So I was asking to route this
> via your tree.
It's now in 'for-next-tpm'. Just pushed.
>
> --
> Regards,
> Sudeep
BR, Jarkko
^ permalink raw reply
* [PATCH next] keys: Replace strcpy(derived_buf, "AUTH_KEY") with strscpy(..., HASH_SIZE)
From: david.laight.linux @ 2026-06-06 20:26 UTC (permalink / raw)
To: Kees Cook, linux-hardening, Arnd Bergmann, keyrings,
linux-integrity, linux-kernel, linux-security-module
Cc: David Howells, James Morris, Jarkko Sakkinen, Mimi Zohar,
Paul Moore, Serge E. Hallyn, David Laight
From: David Laight <david.laight.linux@gmail.com>
derived_buf is guaranteed to be HASH_SIZE - and it is more than enough.
The strscpy() degenerates into an memcpy() (as did the strcpy()).
Do the same for the associated "ENC_KEY" copy.
Removes a possibly unbounded strcpy().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
This is one of a group of patches that remove potentially unbounded
strcpy() calls.
They are mostly replaced by strscpy() or, when strlen() has just been
called, with memcpy() (usually including the '\0').
Calls with copy string literals into arrays are left unchanged.
They are safe and easily detected as such.
The changes were made by getting the compiler to detect the calls and
then fixing the code by hand.
Note that all the changes are only compile tested.
Some Makefiles were changed to allow files to contain strcpy().
As well as 'difficult to fix' files, this included 'show' functions
as they really need to use sysfs_emit() or seq_printf().
All the patches are being sent individually to avoid very long cc lists.
Apologies for the terse commit messages and likely unexpected tags.
(There are about 100 patches in total.)
security/keys/encrypted-keys/encrypted.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 56b531587a1e..59cb77b237b3 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -343,9 +343,9 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
return -ENOMEM;
if (key_type)
- strcpy(derived_buf, "AUTH_KEY");
+ strscpy(derived_buf, "AUTH_KEY", HASH_SIZE);
else
- strcpy(derived_buf, "ENC_KEY");
+ strscpy(derived_buf, "ENC_KEY", HASH_SIZE);
memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
master_keylen);
--
2.39.5
^ permalink raw reply related
* [PATCH v7 12/12] doc: security: Add documentation of exporting and deleting IMA measurements
From: Roberto Sassu @ 2026-06-05 17:22 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: <20260605172236.2042045-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Add the documentation of exporting and deleting IMA measurements in
Documentation/security/IMA-export-delete.rst.
Also add the missing Documentation/security/IMA-templates.rst file in
MAINTAINERS.
Link: https://github.com/linux-integrity/linux/issues/1
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Documentation/security/IMA-export-delete.rst | 203 +++++++++++++++++++
Documentation/security/index.rst | 1 +
MAINTAINERS | 2 +
3 files changed, 206 insertions(+)
create mode 100644 Documentation/security/IMA-export-delete.rst
diff --git a/Documentation/security/IMA-export-delete.rst b/Documentation/security/IMA-export-delete.rst
new file mode 100644
index 000000000000..1600ead03b03
--- /dev/null
+++ b/Documentation/security/IMA-export-delete.rst
@@ -0,0 +1,203 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==================================
+IMA Measurements Export and Delete
+==================================
+
+
+Introduction
+============
+
+The IMA measurements list is currently stored in the kernel memory. Memory
+occupation grows linearly with the number of records, and can become a
+problem especially in environments with reduced resources.
+
+While there is an advantage in keeping the IMA measurements list in kernel
+memory, so that it is always available for reading from the securityfs
+interfaces, storing it elsewhere would make it possible to free precious
+memory for other kernel usage.
+
+The IMA measurements list needs to be retained and safely stored for new
+attestation servers to validate it. Assuming the IMA measurements list is
+properly saved, storing it outside the kernel does not introduce security
+issues, since its integrity is anyway protected by the TPM.
+
+Hence, the new IMA staging mechanism is introduced to export IMA
+measurements to user space and delete them from kernel space.
+
+Staging consists in atomically moving the current measurements list to a
+temporary list, so that measurements can be deleted afterwards. The staging
+operation locks the hot path (racing with addition of new measurements) for
+a very short time, only for swapping the list pointers. Deletion of the
+measurements instead is done locklessly, away from the hot path.
+
+There are two flavors of the staging mechanism. In the staging with prompt,
+all current measurements are staged, read and deleted upon confirmation. In
+the staging and deleting flavor, N measurements are staged from the
+beginning of the current measurements list and immediately deleted without
+confirmation.
+
+
+Management of Staged Measurements
+=================================
+
+Since with the staging mechanism measurement records are removed from the
+kernel, the staged measurements need to be saved in a storage and
+concatenated together, so that they can be presented during remote
+attestation as if staging was never done. This task can be accomplished by
+a remote attestation agent modified to support staging, or a system
+service.
+
+Coordination is necessary in the case where there are multiple actors
+requesting measurements to be staged.
+
+In the staging with prompt case, the measurement interfaces can be accessed
+only by one actor (writer) at a time, so the others will get an error until
+the former closes it. Since the actors don't care about N, when they gain
+access to the interface, they will get all the staged measurements at the
+time of their request.
+
+In the case of staging and deleting, coordination is more important, since
+there is the risk that two actors unaware of each other compute the value N
+on the current measurements list and request IMA to stage N twice.
+
+
+Remote Attestation Agent Workflow
+=================================
+
+Remote attestation agents can be configured to always present all the
+measurements to the remote verifiers or, alternatively, to only provide the
+measurements that have not been verified yet by the remote verifiers.
+
+In the latter case, determining which measurements need to be sent and
+verified must solely depend on the remote verifier. The remote attestation
+agent can proactively send partial measurements, at the condition that they
+are the ones that the remote verifier needs.
+
+An agent can rely on one of the supported staging methods to proactively
+send to a remote verifier the measurements since the previous request up
+to the ones that verify the TPM quote obtained in the current request.
+The workflow with each staging method is the following.
+
+With staging with prompt, the agent stages the current measurements list,
+reads and stores the measurements in a storage and immediately requests
+IMA to delete the staged measurements from kernel memory. Afterwards, it
+calculates N by replaying the PCR extend on the stored measurements until
+the calculated PCRs match the quoted PCRs. It then keeps the measurements
+in excess for the next attestation request.
+
+At the next attestation request, the agent performs the same steps above,
+and concatenates the new measurements to the ones in excess from the
+previous request. Also in this case, the agent replays the PCR extend until
+it matches the currently quoted PCRs, keeps the measurements in excess and
+presents the new N measurement records to the remote attestation server.
+
+With the staging and deleting method, the agent reads the current
+measurements list, calculates N and requests IMA to delete only those. The
+measurements in excess are kept in the IMA measurements list and can be
+retrieved at the next remote attestation request.
+
+While keeping only the excess measurements in the storage could be
+sufficient to serve the requests of a remote verifier, it is advised to
+keep all the obtained measurements locally, as they might be needed for the
+attestation with a different remote verifier.
+
+
+Usage
+=====
+
+The IMA staging mechanism can be enabled from the kernel configuration with
+the CONFIG_IMA_STAGING option. This option prevents inadvertently removing
+the IMA measurement list on systems which do not properly save it.
+
+If the option is enabled, IMA duplicates the current securityfs
+measurements interfaces (both binary and ASCII), by adding the ``_staged``
+file suffix. Both the original and the staging interfaces gain the write
+permission for the root user and group, but require the process to have
+CAP_SYS_ADMIN set.
+
+The staging mechanism supports two flavors.
+
+
+Staging with prompt
+~~~~~~~~~~~~~~~~~~~
+
+The current measurements list is moved to a temporary staging area,
+allowing it to be saved to external storage, before being deleted upon
+confirmation.
+
+This staging process is achieved with the following steps.
+
+ 1. ``echo A > <_staged interface>``: the user requests IMA to stage the
+ entire measurements list;
+ 2. ``cat <_staged interface>``: the user reads the staged measurements;
+ 3. ``echo D > <_staged interface>``: the user requests IMA to delete
+ staged measurements.
+
+
+Staging and deleting
+~~~~~~~~~~~~~~~~~~~~
+
+N measurements are staged to a temporary staging area, and immediately
+deleted without further confirmation.
+
+This staging process is achieved with the following steps.
+
+ 1. ``cat <original interface>``: the user reads the current measurements
+ list and determines what the value N for staging should be;
+ 2. ``echo N > <original interface>``: the user requests IMA to delete N
+ measurements from the current measurements list.
+
+
+Interface Access
+================
+
+In order to avoid the IMA measurements list being suddenly truncated by the
+staging mechanism during a read, or having multiple concurrent staging, a
+semaphore-like locking scheme has been implemented on all the measurements
+list interfaces.
+
+Multiple readers can access concurrently the original and staged
+interfaces, and they can be in mutual exclusion with one writer. In order
+to see the same state across all the measurement interfaces, the same
+writer is allowed to open multiple interfaces for write or read/write.
+
+If an illegal access occurs, the open to the measurements list interface is
+denied.
+
+
+Kexec
+=====
+
+In the event a kexec() system call occurs between staging and deleting, the
+staged measurement records are marshalled before the current measurements
+list, so that they are both available when the secondary kernel starts.
+
+If measurement is suspended before requesting to delete staged or current
+measurements, IMA returns an error to user space to let it know that
+marshalling is already in progress, so that it does not save the
+measurements twice.
+
+IMA also disallows staging when suspending measurement, to avoid the
+situation where neither measurements are carried over to the secondary
+kernel, nor they are saved by user space to the storage.
+
+
+Hash table
+==========
+
+By default, the template digest of staged measurement records are kept in
+kernel memory (only template data are freed), to be able to detect
+duplicate records independently of staging.
+
+The new kernel option ``ima_flush_htable`` has been introduced to
+explicitly request a complete deletion of the staged measurements, for
+maximum kernel memory saving. If the option has been specified, duplicate
+records are still avoided on records of the current measurements list,
+but there can be duplicates between different groups of staged
+measurements.
+
+Flushing the hash table is supported only for the staging with prompt
+flavor. For the staging and deleting flavor, it would have been necessary
+to lock the hot path adding new measurements for the time needed to remove
+each selected measurement individually.
diff --git a/Documentation/security/index.rst b/Documentation/security/index.rst
index 3e0a7114a862..00650dcf38cb 100644
--- a/Documentation/security/index.rst
+++ b/Documentation/security/index.rst
@@ -8,6 +8,7 @@ Security Documentation
credentials
snp-tdx-threat-model
IMA-templates
+ IMA-export-delete
keys/index
lsm
lsm-development
diff --git a/MAINTAINERS b/MAINTAINERS
index 461a3eed6129..70ff6bae3493 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12752,6 +12752,8 @@ R: Eric Snowberg <eric.snowberg@oracle.com>
L: linux-integrity@vger.kernel.org
S: Supported
T: git git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
+F: Documentation/security/IMA-export-delete.rst
+F: Documentation/security/IMA-templates.rst
F: include/linux/secure_boot.h
F: security/integrity/
F: security/integrity/ima/
--
2.43.0
^ permalink raw reply related
* [PATCH v7 11/12] ima: Support staging and deleting N measurements records
From: Roberto Sassu @ 2026-06-05 17:22 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: <20260605172236.2042045-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Add support for sending a value N between 1 and ULONG_MAX to the IMA
original measurement interface. This value represents the number of
measurements that should be deleted from the current measurements list. In
this case, measurements are staged in an internal non-user visible list,
and immediately deleted.
This staging method allows the remote attestation agents to easily separate
the measurements that were verified (staged and deleted) from those that
weren't due to the race between taking a TPM quote and reading the
measurements list.
In order to minimize the locking time of ima_extend_list_mutex, deleting
N records is realized by doing a lockless walk in the current measurements
list to determine the N-th entry to cut, to cut the current measurements
list under the lock, and by deleting the excess records after releasing the
lock.
Flushing the hash table is not supported for N records, since it would
require removing the N records one by one from the hash table under the
ima_extend_list_mutex lock, which would increase the locking time.
Link: https://github.com/linux-integrity/linux/issues/1
Co-developed-by: Steven Chen <chenste@linux.microsoft.com>
Signed-off-by: Steven Chen <chenste@linux.microsoft.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/Kconfig | 3 ++
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 32 +++++++++++++--
security/integrity/ima/ima_queue.c | 65 +++++++++++++++++++++++++++++-
4 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 02436670f746..f4d25e045808 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -341,6 +341,9 @@ config IMA_STAGING
It allows user space to stage the measurements list for deletion and
to delete the staged measurements after confirmation.
+ Or, alternatively, it allows user space to specify N measurements
+ records to stage internally, so that they can be immediately deleted.
+
On kexec, staging is aborted and any staged measurement records are
copied to the secondary kernel.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3892d2a6c2e2..caaedd4b58fd 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -320,6 +320,7 @@ 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_staged_delete_all(void);
+int ima_queue_delete_partial(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);
@@ -342,6 +343,7 @@ extern atomic_long_t ima_num_records[BINARY__LAST];
/* Total number of violations since hard boot. */
extern atomic_long_t ima_num_violations;
extern struct hlist_head __rcu *ima_htable;
+extern bool ima_flush_htable;
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 96d7503a605b..174a94740da1 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -28,6 +28,7 @@
* Requests:
* 'A\n': stage the entire measurements list
* 'D\n': delete all staged measurements
+ * '[1, ULONG_MAX]\n' delete N measurements records
*/
#define STAGED_REQ_LENGTH 21
@@ -343,6 +344,7 @@ static ssize_t _ima_measurements_write(struct file *file,
loff_t *ppos, bool staged_interface)
{
char req[STAGED_REQ_LENGTH];
+ unsigned long req_value;
int ret;
if (datalen < 2 || datalen > STAGED_REQ_LENGTH)
@@ -370,7 +372,24 @@ static ssize_t _ima_measurements_write(struct file *file,
ret = ima_queue_staged_delete_all();
break;
default:
- ret = -EINVAL;
+ if (staged_interface)
+ return -EINVAL;
+
+ if (ima_flush_htable) {
+ pr_debug("Deleting staged N measurements not supported when flushing the hash table is requested\n");
+ return -EINVAL;
+ }
+
+ ret = kstrtoul(req, 10, &req_value);
+ if (ret < 0)
+ return ret;
+
+ if (req_value == 0) {
+ pr_debug("Must delete at least one entry\n");
+ return -EINVAL;
+ }
+
+ ret = ima_queue_delete_partial(req_value);
}
if (ret < 0)
@@ -379,6 +398,12 @@ static ssize_t _ima_measurements_write(struct file *file,
return datalen;
}
+static ssize_t ima_measurements_write(struct file *file, const char __user *buf,
+ size_t datalen, loff_t *ppos)
+{
+ return _ima_measurements_write(file, buf, datalen, ppos, false);
+}
+
static ssize_t ima_measurements_staged_write(struct file *file,
const char __user *buf,
size_t datalen, loff_t *ppos)
@@ -389,6 +414,7 @@ static ssize_t ima_measurements_staged_write(struct file *file,
static const struct file_operations ima_measurements_ops = {
.open = ima_measurements_open,
.read = seq_read,
+ .write = ima_measurements_write,
.llseek = seq_lseek,
.release = ima_measurements_release,
};
@@ -470,6 +496,7 @@ static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
static const struct file_operations ima_ascii_measurements_ops = {
.open = ima_ascii_measurements_open,
.read = seq_read,
+ .write = ima_measurements_write,
.llseek = seq_lseek,
.release = ima_measurements_release,
};
@@ -603,14 +630,13 @@ 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;
- umode_t permissions = (S_IRUSR | S_IRGRP);
+ umode_t permissions = (S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP);
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;
- permissions |= (S_IWUSR | S_IWGRP);
file_suffix = "_staged";
}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index df1e81ea7a36..f89f0ca3d4ed 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -22,7 +22,7 @@
#define AUDIT_CAUSE_LEN_MAX 32
-static bool ima_flush_htable;
+bool ima_flush_htable;
static int __init ima_flush_htable_setup(char *str)
{
@@ -405,6 +405,69 @@ int ima_queue_staged_delete_all(void)
return 0;
}
+/**
+ * ima_queue_delete_partial - Delete current measurements
+ * @req_value: Number of measurements to delete
+ *
+ * Delete the requested number of measurements from the current measurements
+ * list, and update the number of records and the binary run-time size
+ * accordingly.
+ *
+ * Refuse to delete current measurements if measurement is suspended, so that
+ * dump can be done in a lockless way and user space is notified about current
+ * measurements being carried over to the secondary kernel, so that it does not
+ * save them twice.
+ *
+ * Return: Zero on success, a negative value otherwise.
+ */
+int ima_queue_delete_partial(unsigned long req_value)
+{
+ unsigned long req_value_copy = req_value;
+ unsigned long size_to_remove = 0, num_to_remove = 0;
+ LIST_HEAD(ima_measurements_trim);
+ struct ima_queue_entry *qe;
+ int ret = 0;
+
+ /*
+ * list_for_each_entry_rcu() without rcu_read_lock() is fine because
+ * only list append can happen concurrently. No list replace due to the
+ * staging/delete writers mutual exclusion.
+ */
+ list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
+ size_to_remove += get_binary_runtime_size(qe->entry);
+ num_to_remove++;
+
+ if (--req_value_copy == 0)
+ break;
+ }
+
+ /* Not enough records to delete. */
+ if (req_value_copy > 0)
+ return -ENOENT;
+
+ mutex_lock(&ima_extend_list_mutex);
+ if (ima_measurements_suspended) {
+ mutex_unlock(&ima_extend_list_mutex);
+ return -ESTALE;
+ }
+
+ /*
+ * qe remains valid because ima_fs.c enforces single-writer exclusion.
+ */
+ __list_cut_position(&ima_measurements_trim, &ima_measurements,
+ &qe->later);
+
+ atomic_long_sub(num_to_remove, &ima_num_records[BINARY]);
+
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ binary_runtime_size[BINARY] -= size_to_remove;
+
+ mutex_unlock(&ima_extend_list_mutex);
+
+ ima_queue_delete(&ima_measurements_trim, false);
+ return ret;
+}
+
/**
* ima_queue_delete - Delete measurements
* @head: List head measurements are deleted from
--
2.43.0
^ permalink raw reply related
* [PATCH v7 10/12] ima: Add support for flushing the hash table when staging measurements
From: Roberto Sassu @ 2026-06-05 17:22 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: <20260605172236.2042045-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
During staging and delete, measurements are not completely deallocated.
Their entry digest portion is kept and is still reachable with the hash
table to detect duplicate records. If the number of records is significant,
this reduces the memory saving benefit of staging.
Some users might be interested in achieving the best memory saving (the
measurements are completely deallocated) at the cost of having duplicate
records across the staged measurement lists. Duplicate records are still
avoided within the current measurement list.
Introduce the new kernel option ima_flush_htable to decide whether or not
the digests of staged measurement records are flushed from the hash table,
when they are deleted, to achieve the maximum memory saving.
When the option is enabled, replace the old hash table with a new one,
by calling ima_alloc_replace_htable(), and completely delete the
measurements 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>
---
.../admin-guide/kernel-parameters.txt | 6 +++
security/integrity/ima/ima_queue.c | 41 ++++++++++++++++---
2 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4d0f545fb3ec..aad318803f82 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2343,6 +2343,12 @@ 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 records, to achieve maximum
+ memory saving at the cost of having duplicate
+ records across the staged measurement lists.
+
ima_hash= [IMA]
Format: { md5 | sha1 | rmd160 | sha256 | sha384
| sha512 | ... }
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index cdc21e1b929b..df1e81ea7a36 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -22,6 +22,20 @@
#define AUDIT_CAUSE_LEN_MAX 32
+static 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;
@@ -332,7 +346,7 @@ int ima_queue_stage(void)
return ret;
}
-static void ima_queue_delete(struct list_head *head);
+static void ima_queue_delete(struct list_head *head, bool flush_htable);
/**
* ima_queue_staged_delete_all - Delete staged measurements
@@ -350,6 +364,7 @@ static void ima_queue_delete(struct list_head *head);
*/
int ima_queue_staged_delete_all(void)
{
+ struct hlist_head *old_queue = NULL;
LIST_HEAD(ima_measurements_trim);
mutex_lock(&ima_extend_list_mutex);
@@ -371,21 +386,35 @@ int ima_queue_staged_delete_all(void)
if (IS_ENABLED(CONFIG_IMA_KEXEC))
binary_runtime_size[BINARY_STAGED] = 0;
+ 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);
+ }
+ }
+
mutex_unlock(&ima_extend_list_mutex);
- ima_queue_delete(&ima_measurements_trim);
+ if (ima_flush_htable) {
+ synchronize_rcu();
+ kfree(old_queue);
+ }
+
+ ima_queue_delete(&ima_measurements_trim, ima_flush_htable);
return 0;
}
/**
* ima_queue_delete - Delete measurements
* @head: List head measurements are deleted from
+ * @flush_htable: Whether or not the hash table is being flushed
*
* Delete the measurements from the passed list head completely if the
- * hash table is not enabled, or partially (only the template data), if the
- * hash table is used.
+ * hash table is not enabled or is being flushed, or partially (only the
+ * template data), if the hash table is used.
*/
-static void ima_queue_delete(struct list_head *head)
+static void ima_queue_delete(struct list_head *head, bool flush_htable)
{
struct ima_queue_entry *qe, *qe_tmp;
unsigned int i;
@@ -407,7 +436,7 @@ static void ima_queue_delete(struct list_head *head)
list_del(&qe->later);
/* No leak if condition is false, referenced by ima_htable. */
- if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
+ if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) || flush_htable) {
kfree(qe->entry->digests);
kfree(qe->entry);
kfree(qe);
--
2.43.0
^ permalink raw reply related
* [PATCH v7 09/12] ima: Add support for staging measurements with prompt
From: Roberto Sassu @ 2026-06-05 17:22 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, Stefan Berger
In-Reply-To: <20260605172236.2042045-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Introduce the ability of staging the IMA measurement list and deleting them
with a prompt.
Staging means moving the current measurement list records to a separate
location, and allowing users to read and delete it. This causes the current
measurement list to be emptied (since records were moved) and new
measurements to be added on the empty list. Staging can be done only once
at a time. In the event of kexec(), staging is aborted and staged records
will be carried over to the new kernel.
Introduce ascii_runtime_measurements_<algo>_staged and
binary_runtime_measurements_<algo>_staged interfaces to access and delete
the measurements.
Use 'echo A > <IMA _staged interface>' and
'echo D > <IMA _staged interface>' to respectively stage and delete the
entire measurements list. Locking of these interfaces is also mediated with
a call to _ima_measurements_open() and with ima_measurements_release().
Implement the staging functionality by introducing the new global
measurements list ima_measurements_staged, and ima_queue_stage() and
ima_queue_staged_delete_all() to respectively move measurements from the
current measurements list to the staged one, and to move staged
measurements to the ima_measurements_trim list for deletion. Introduce
ima_queue_delete() to delete the measurements.
Staging is forbidden after measurement is suspended, and between staging
and deleting, so that walking the staged and current measurements list can
be done locklessly in ima_dump_measurement_list(). Strict ordering of
suspending and dumping is enforced by two reboot notifiers with different
priority. Refusing to delete staged measurements also signals to user space
that those measurements are already carried over to the secondary kernel,
so that it does not save them twice.
Finally, introduce the BINARY_STAGED and BINARY_FULL binary measurements
list types, to maintain the counters and the binary size of staged
measurements and the full measurements list (including records that were
staged). BINARY still represents the current binary measurements list.
Use the binary size for the BINARY + BINARY_STAGED types in
ima_add_kexec_buffer(), since both measurements list types are copied to
the secondary kernel during kexec. Use BINARY_FULL in
ima_measure_kexec_event(), to generate a critical data record.
It should be noted that the BINARY_FULL counter is not passed through
kexec. Thus, the number of records included in the kexec critical data
records refers to the records since the critical data records generated
from the previous kexec event.
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
Suggested-by: Gregory Lumen <gregorylumen@linux.microsoft.com> (staging revert)
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
---
security/integrity/ima/Kconfig | 12 ++
security/integrity/ima/ima.h | 7 +-
security/integrity/ima/ima_fs.c | 174 ++++++++++++++++++++++++++---
security/integrity/ima/ima_kexec.c | 20 +++-
security/integrity/ima/ima_queue.c | 140 ++++++++++++++++++++++-
5 files changed, 333 insertions(+), 20 deletions(-)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 862fbee2b174..02436670f746 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -332,4 +332,16 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
If set to the default value of 0, an extra half page of memory for those
additional measurements will be allocated.
+config IMA_STAGING
+ bool "Support for staging the measurements list"
+ default n
+ help
+ Add support for staging the measurements list.
+
+ It allows user space to stage the measurements list for deletion and
+ to delete the staged measurements after confirmation.
+
+ On kexec, staging is aborted and any staged measurement records are
+ copied to the secondary kernel.
+
endif
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c00c133a140f..3892d2a6c2e2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -30,9 +30,11 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
/*
* BINARY: current binary measurements list
+ * BINARY_STAGED: staged binary measurements list
+ * BINARY_FULL: binary measurements list since IMA init (lost after kexec)
*/
enum binary_lists {
- BINARY, BINARY__LAST
+ BINARY, BINARY_STAGED, BINARY_FULL, BINARY__LAST
};
/* digest size for IMA, fits SHA1 or MD5 */
@@ -125,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 {
@@ -315,6 +318,8 @@ 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_staged_delete_all(void);
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);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index f6ecee2d7699..96d7503a605b 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -24,6 +24,13 @@
#include "ima.h"
+/*
+ * Requests:
+ * 'A\n': stage the entire measurements list
+ * 'D\n': delete all staged measurements
+ */
+#define STAGED_REQ_LENGTH 21
+
static DEFINE_MUTEX(ima_write_mutex);
static DEFINE_MUTEX(ima_measure_mutex);
static long ima_measure_users;
@@ -99,6 +106,11 @@ 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)
{
@@ -120,6 +132,12 @@ 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)
{
}
@@ -213,6 +231,13 @@ static const struct seq_operations ima_measurments_seqops = {
.show = ima_measurements_show
};
+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_measure_lock(bool write)
{
mutex_lock(&ima_measure_mutex);
@@ -307,6 +332,60 @@ static int ima_measurements_release(struct inode *inode, struct file *file)
return ret;
}
+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_write(struct file *file,
+ const char __user *buf, size_t datalen,
+ loff_t *ppos, bool staged_interface)
+{
+ char req[STAGED_REQ_LENGTH];
+ int ret;
+
+ if (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 || !staged_interface)
+ return -EINVAL;
+
+ ret = ima_queue_stage();
+ break;
+ case 'D':
+ if (datalen != 2 || !staged_interface)
+ return -EINVAL;
+
+ ret = ima_queue_staged_delete_all();
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ if (ret < 0)
+ return ret;
+
+ return datalen;
+}
+
+static ssize_t ima_measurements_staged_write(struct file *file,
+ const char __user *buf,
+ size_t datalen, loff_t *ppos)
+{
+ return _ima_measurements_write(file, buf, datalen, ppos, true);
+}
+
static const struct file_operations ima_measurements_ops = {
.open = ima_measurements_open,
.read = seq_read,
@@ -314,6 +393,14 @@ static const struct file_operations ima_measurements_ops = {
.release = ima_measurements_release,
};
+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)
{
u32 i;
@@ -387,6 +474,28 @@ static const struct file_operations ima_ascii_measurements_ops = {
.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)
{
void *data = NULL;
@@ -490,10 +599,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;
+ umode_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;
+ permissions |= (S_IWUSR | S_IWGRP);
+ file_suffix = "_staged";
+ }
+
if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
count++;
@@ -504,29 +624,32 @@ static int __init create_securityfs_measurement_lists(void)
if (algo == HASH_ALGO__LAST)
snprintf(file_name, sizeof(file_name),
- "ascii_runtime_measurements_tpm_alg_%x",
- ima_tpm_chip->allocated_banks[i].alg_id);
+ "ascii_runtime_measurements_tpm_alg_%x%s",
+ ima_tpm_chip->allocated_banks[i].alg_id,
+ file_suffix);
else
snprintf(file_name, sizeof(file_name),
- "ascii_runtime_measurements_%s",
- hash_algo_name[algo]);
- dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
+ "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)
snprintf(file_name, sizeof(file_name),
- "binary_runtime_measurements_tpm_alg_%x",
- ima_tpm_chip->allocated_banks[i].alg_id);
+ "binary_runtime_measurements_tpm_alg_%x%s",
+ ima_tpm_chip->allocated_banks[i].alg_id,
+ file_suffix);
else
snprintf(file_name, sizeof(file_name),
- "binary_runtime_measurements_%s",
- hash_algo_name[algo]);
- dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
+ "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);
}
@@ -534,6 +657,23 @@ static int __init create_securityfs_measurement_lists(void)
return 0;
}
+static int __init create_securityfs_staging_links(void)
+{
+ struct dentry *dentry;
+
+ dentry = securityfs_create_symlink("binary_runtime_measurements_staged",
+ ima_dir, "binary_runtime_measurements_sha1_staged", NULL);
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ dentry = securityfs_create_symlink("ascii_runtime_measurements_staged",
+ ima_dir, "ascii_runtime_measurements_sha1_staged", NULL);
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ return 0;
+}
+
/*
* ima_open_policy: sequentialize access to the policy file
*/
@@ -626,7 +766,13 @@ int __init ima_fs_init(void)
goto out;
}
- ret = create_securityfs_measurement_lists();
+ ret = create_securityfs_measurement_lists(false);
+ if (ret == 0 && IS_ENABLED(CONFIG_IMA_STAGING)) {
+ ret = create_securityfs_measurement_lists(true);
+ if (ret == 0)
+ ret = create_securityfs_staging_links();
+ }
+
if (ret != 0)
goto out;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 26d41974429e..0d845693a1f7 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(BINARY);
- len = atomic_long_read(&ima_num_records[BINARY]);
+ buf_size = ima_get_binary_runtime_size(BINARY_FULL);
+ len = atomic_long_read(&ima_num_records[BINARY_FULL]);
n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
"kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
@@ -106,13 +106,24 @@ 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) {
+ /*
+ * Lockless walks possible due to strict ordering of the reboot
+ * notifiers, suspending measurement before dump, and forbidding
+ * staging/deleting (list mutations) after suspend.
+ */
+ list_for_each_entry(qe, &ima_measurements_staged, later) {
ret = ima_dump_measurement(&khdr, qe);
if (ret < 0)
break;
}
+ list_for_each_entry(qe, &ima_measurements, later) {
+ if (!ret)
+ ret = ima_dump_measurement(&khdr, qe);
+ if (ret < 0)
+ break;
+ }
+
/*
* fill in reserved space with some buffer details
* (eg. version, buffer size, number of measurements)
@@ -167,6 +178,7 @@ void ima_add_kexec_buffer(struct kimage *image)
extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
binary_runtime_size = ima_get_binary_runtime_size(BINARY) +
+ ima_get_binary_runtime_size(BINARY_STAGED) +
extra_memory;
if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 618694d5c082..cdc21e1b929b 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -26,6 +26,7 @@
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[BINARY__LAST];
#else
@@ -42,7 +43,7 @@ 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.
*/
@@ -171,12 +172,16 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
lockdep_is_held(&ima_extend_list_mutex));
atomic_long_inc(&ima_num_records[BINARY]);
+ atomic_long_inc(&ima_num_records[BINARY_FULL]);
+
if (update_htable) {
key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
hlist_add_head_rcu(&qe->hnext, &htable[key]);
}
ima_update_binary_runtime_size(entry, BINARY);
+ ima_update_binary_runtime_size(entry, BINARY_FULL);
+
return 0;
}
@@ -277,6 +282,139 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
return result;
}
+/**
+ * ima_queue_stage - Stage all measurements
+ *
+ * If the staged measurements list is empty, the current measurements list is
+ * not empty, and measurement is not suspended, move the measurements from the
+ * current list to the staged one, and update the number of records and binary
+ * run-time size accordingly.
+ *
+ * Do not allow staging after measurement is suspended, so that dumping
+ * measurements can be done in a lockless way.
+ *
+ * Return: Zero on success, a negative value otherwise.
+ */
+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;
+ }
+
+ if (ima_measurements_suspended) {
+ ret = -EACCES;
+ goto out_unlock;
+ }
+
+ list_replace(&ima_measurements, &ima_measurements_staged);
+ INIT_LIST_HEAD(&ima_measurements);
+
+ atomic_long_set(&ima_num_records[BINARY_STAGED],
+ atomic_long_read(&ima_num_records[BINARY]));
+ atomic_long_set(&ima_num_records[BINARY], 0);
+
+ if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
+ binary_runtime_size[BINARY_STAGED] =
+ binary_runtime_size[BINARY];
+ binary_runtime_size[BINARY] = 0;
+ }
+out_unlock:
+ mutex_unlock(&ima_extend_list_mutex);
+ return ret;
+}
+
+static void ima_queue_delete(struct list_head *head);
+
+/**
+ * ima_queue_staged_delete_all - Delete staged measurements
+ *
+ * Move staged measurements to a temporary list, ima_measurements_trim, update
+ * the number of records and the binary run-time size accordingly. Finally,
+ * delete measurements in the temporary list.
+ *
+ * Refuse to delete staged measurements if measurement is suspended, so that
+ * dump can be done in a lockless way and user space is notified about staged
+ * measurements being carried over to the secondary kernel, so that it does not
+ * save them twice.
+ *
+ * Return: Zero on success, a negative value otherwise.
+ */
+int ima_queue_staged_delete_all(void)
+{
+ LIST_HEAD(ima_measurements_trim);
+
+ mutex_lock(&ima_extend_list_mutex);
+ if (list_empty(&ima_measurements_staged)) {
+ mutex_unlock(&ima_extend_list_mutex);
+ return -ENOENT;
+ }
+
+ if (ima_measurements_suspended) {
+ mutex_unlock(&ima_extend_list_mutex);
+ return -ESTALE;
+ }
+
+ list_replace(&ima_measurements_staged, &ima_measurements_trim);
+ INIT_LIST_HEAD(&ima_measurements_staged);
+
+ atomic_long_set(&ima_num_records[BINARY_STAGED], 0);
+
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ binary_runtime_size[BINARY_STAGED] = 0;
+
+ mutex_unlock(&ima_extend_list_mutex);
+
+ ima_queue_delete(&ima_measurements_trim);
+ return 0;
+}
+
+/**
+ * ima_queue_delete - Delete measurements
+ * @head: List head measurements are deleted from
+ *
+ * Delete the measurements from the passed list head completely if the
+ * hash table is not enabled, or partially (only the template data), if the
+ * hash table is used.
+ */
+static void ima_queue_delete(struct list_head *head)
+{
+ struct ima_queue_entry *qe, *qe_tmp;
+ unsigned int i;
+
+ list_for_each_entry_safe(qe, qe_tmp, head, 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 condition is false, referenced by ima_htable. */
+ if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
+ kfree(qe->entry->digests);
+ kfree(qe->entry);
+ kfree(qe);
+ }
+ }
+}
+
int ima_restore_measurement_entry(struct ima_template_entry *entry)
{
int result = 0;
--
2.43.0
^ permalink raw reply related
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