Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM

Yeoreum Yun
2026-04-22T18:46:08Z

Hi Mimi,

> On Wed, 2026-04-22 at 17:24 +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.
> >
> > To resolve this, call ima_init() again at late_inicall_sync level
> > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > log though TPM device presents in the system.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>
> A lot of change for just detecting whether ima_init() is being called on
> late_initcall or late_initcall_sync(), without any explanation for all the other
> changes (e.g. ima_init_core).
>
> Please just limit the change to just calling ima_init() twice.

My concern is that ima_update_policy_flags() will be called
when ima_init() is deferred -- not initialised anything.
though functionally, it might be okay however,
I think ima_update_policy_flags() and notifier should work after ima_init()
works logically.

This change I think not much quite a lot. just wrapper ima_init() with
ima_init_core() with some error handling.

Am I missing something?

>
>
> > ---
> > include/linux/lsm_hooks.h | 2 +
> > security/integrity/ima/ima.h | 4 +-
> > security/integrity/ima/ima_init.c | 10 +++-
> > security/integrity/ima/ima_main.c | 76 +++++++++++++++++++++++------
> > security/integrity/ima/ima_policy.c | 3 ++
> > security/lsm_init.c | 13 ++++-
> > 6 files changed, 87 insertions(+), 21 deletions(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index d48bf0ad26f4..88fe105b7f00 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -166,6 +166,7 @@ enum lsm_order {
> > * @initcall_fs: LSM callback for fs_initcall setup, optional
> > * @initcall_device: LSM callback for device_initcall() setup, optional
> > * @initcall_late: LSM callback for late_initcall() setup, optional
> > + * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
> > */
> > struct lsm_info {
> > const struct lsm_id *id;
> > @@ -181,6 +182,7 @@ struct lsm_info {
> > int (*initcall_fs)(void);
> > int (*initcall_device)(void);
> > int (*initcall_late)(void);
> > + int (*initcall_late_sync)(void);
> > };
> >
> > #define DEFINE_LSM(lsm) \
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index 89ebe98ffc5e..75ee7ad184d0 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -62,6 +62,8 @@ extern int ima_hash_algo_idx __ro_after_init;
> > extern int ima_extra_slots __ro_after_init;
> > extern struct ima_algo_desc *ima_algo_array __ro_after_init;
> >
> > +extern bool ima_initialised __ro_after_init;
> > +
> > extern int ima_appraise;
> > extern struct tpm_chip *ima_tpm_chip;
> > extern const char boot_aggregate_name[];
> > @@ -257,7 +259,7 @@ static inline void ima_measure_kexec_event(const char *event_name) {}
> > extern bool ima_canonical_fmt;
> >
> > /* Internal IMA function definitions */
> > -int ima_init(void);
> > +int ima_init(bool late);
> > int ima_fs_init(void);
> > int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> > const char *op, struct inode *inode,
> > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> > index a2f34f2d8ad7..c28c71090ad2 100644
> > --- a/security/integrity/ima/ima_init.c
> > +++ b/security/integrity/ima/ima_init.c
> > @@ -115,13 +115,19 @@ void __init ima_load_x509(void)
> > }
> > #endif
> >
> > -int __init ima_init(void)
> > +int __init ima_init(bool late)
> > {
> > int rc;
> >
> > ima_tpm_chip = tpm_default_chip();
> > - if (!ima_tpm_chip)
> > + if (!ima_tpm_chip) {
> > + if (!late) {
> > + pr_info("Defer initialisation to the late_initcall_sync stage.\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > pr_info("No TPM chip found, activating TPM-bypass!\n");
> > + }
> >
> > rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
> > if (rc)
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 1d6229b156fb..ac444ee600e2 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -38,6 +38,7 @@ int ima_appraise;
> > #endif
> >
> > int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> > +bool ima_initialised __ro_after_init = false;
> > static int hash_setup_done;
> > static int ima_disabled __ro_after_init;
> >
> > @@ -1237,6 +1238,35 @@ static int ima_kernel_module_request(char *kmod_name)
> >
> > #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> >
> > +static int __init init_ima_core(bool late)
> > +{
> > + int err;
> > +
> > + if (ima_initialised)
> > + return 0;
> > +
> > + err = ima_init(late);
> > + if (err == -EPROBE_DEFER)
> > + return 0;
> > +
> > + if (err && strcmp(hash_algo_name[ima_hash_algo],
> > + CONFIG_IMA_DEFAULT_HASH) != 0) {
> > + pr_info("Allocating %s failed, going to use default hash algorithm %s\n",
> > + hash_algo_name[ima_hash_algo], CONFIG_IMA_DEFAULT_HASH);
> > + hash_setup_done = 0;
> > + hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > + err = ima_init(late);
> > + }
> > +
> > + if (!err) {
> > + ima_update_policy_flags();
> > + ima_initialised = true;
> > + } else
> > + ima_disabled = 1;
> > +
> > + return err;
> > +}
> > +
> > static int __init init_ima(void)
> > {
> > int error;
> > @@ -1250,30 +1280,42 @@ static int __init init_ima(void)
> > ima_appraise_parse_cmdline();
> > ima_init_template_list();
> > hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > - error = ima_init();
> > -
> > - if (error && strcmp(hash_algo_name[ima_hash_algo],
> > - CONFIG_IMA_DEFAULT_HASH) != 0) {
> > - pr_info("Allocating %s failed, going to use default hash algorithm %s\n",
> > - hash_algo_name[ima_hash_algo], CONFIG_IMA_DEFAULT_HASH);
> > - hash_setup_done = 0;
> > - hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > - error = ima_init();
> > - }
> > -
> > - if (error)
> > - return error;
> >
> > error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier);
> > - if (error)
> > + if (error) {
> > pr_warn("Couldn't register LSM notifier, error %d\n", error);
> > + goto disable_ima;
> > + }
> >
> > - if (!error)
> > - ima_update_policy_flags();
> > + error = init_ima_core(false);
> > + if (error) {
> > + unregister_blocking_lsm_notifier(&ima_lsm_policy_notifier);
> > + goto disable_ima;
> > + }
> > +
> > + return 0;
> >
> > +disable_ima:
> > + ima_disabled = 1;
> > return error;
> > }
> >
> > +static int __init late_init_ima(void)
> > +{
> > + int err;
> > +
> > + if (ima_disabled)
> > + return 0;
> > +
> > + err = init_ima_core(true);
> > + if (err) {
> > + unregister_blocking_lsm_notifier(&ima_lsm_policy_notifier);
> > + ima_disabled = 1;
> > + }
> > +
> > + return err;
> > +}
> > +
> > static struct security_hook_list ima_hooks[] __ro_after_init = {
> > LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
> > LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec),
> > @@ -1321,4 +1363,6 @@ DEFINE_LSM(ima) = {
> > .blobs = &ima_blob_sizes,
> > /* Start IMA after the TPM is available */
> > .initcall_late = init_ima,
> > + /* Start IMA late in case of probing TPM is deferred. */
> > + .initcall_late_sync = late_init_ima,
> > };
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index bf2d7ba4c14a..c3bcc3521c81 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -501,6 +501,9 @@ static void ima_lsm_update_rules(void)
> > int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> > void *lsm_data)
> > {
> > + if (!ima_initialised)
> > + return NOTIFY_DONE;
> > +
> > if (event != LSM_POLICY_CHANGE)
> > return NOTIFY_DONE;
> >
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index 573e2a7250c4..4e5c59beb82a 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
> > * security_initcall_late - Run the LSM late initcalls
> > */
> > static int __init security_initcall_late(void)
> > +{
> > + return lsm_initcall(late);
> > +}
> > +late_initcall(security_initcall_late);
> > +
> > +/**
> > + * security_initcall_late_sync - Run the LSM late initcalls sync
> > + */
> > +static int __init security_initcall_late_sync(void)
> > {
> > int rc;
> >
> > - rc = lsm_initcall(late);
> > + rc = lsm_initcall(late_sync);
> > lsm_pr_dbg("all enabled LSMs fully activated\n");
> > call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
> >
> > return rc;
> > }
> > -late_initcall(security_initcall_late);
> > +late_initcall_sync(security_initcall_late_sync);
> > --
> > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

--
Sincerely,
Yeoreum Yun