[PATCH 0/2] fix failure of integration between tpm_crb_ffa with ima

[PATCH 0/2] fix failure of integration between tpm_crb_ffa with ima

[Yeoreum Yun]

To integrate tpm_event_log with ima subsystem,
tpm_crb and tpm_crb_ffa driver should be built as built-in
(ARM_FFA_TRANSPORT=y && CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).

However, this could make failure for ima_init() gets tpm chip when
each initcall function deployed like:

0000000000000888 l       .initcall6.init>-------0000000000000000 crb_acpi_driver_init
000000000000088c l       .initcall6.init>-------0000000000000000 tpm_crb_ffa_driver_init
0000000000000a9c l       .initcall6.init>-------0000000000000000 ffa_init

If crb_api_driver_init() is called first, probing tpm device using CRB over
FFA method is deferred since tpm_crb_ffa_driver_init() or
ffa_init() doesn't called yet -- this means the secure partition isn't
probed yet.

and deferred probe of the tpm device will be probed by system workqueue
in proper time after deferred_probe_initcall() registers the work.
However, ima_init() could be called first if they're deployed like:

000000000000012c l       .initcall7.init>-------0000000000000000 init_ima
000000000000016c l       .initcall7.init>-------0000000000000000 deferred_probe_initcall7

In this situation, ima_init() failed to find tpm device and it failed to
generate boot_aggregate with PCR values.

That's why kernel prints log this situation though tpm device exists:
   [    3.080786] ima: No TPM chip found, activating TPM-bypass!

To resolve this,
  Patch #1, change ffa_init()'s init level to rootfs_initcall so that
  ffa_device generate before any ffa_driver is loaded.

  Patch #2, call ffa_register() at tpm_crb_ffa_init() when
  it's built as built-in. so that when tpm device is probed,
  tpm_crb_ffa secure partition in probed state with related ffa_driver.

---

Yeoreum Yun (2):
  driver/firmware/arm_ffa: change ffa_init's initlevel
  driver/tpm/tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's
    built-in

 drivers/char/tpm/tpm_crb_ffa.c    | 22 +++++++++++++++++-----
 drivers/firmware/arm_ffa/driver.c |  2 +-
 2 files changed, 18 insertions(+), 6 deletions(-)

--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

[PATCH 1/2] firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall

[Yeoreum Yun]

The Linux IMA (Integrity Measurement Architecture) subsystem used for secure
boot, file integrity, or remote attestation cannot be a loadable module
for few reasons listed below:

 o Boot-Time Integrity: IMA’s main role is to measure and appraise files
   before they are used. This includes measuring critical system files during
   early boot (e.g., init, init scripts, login binaries). If IMA were a module,
   it would be loaded too late to cover those.

 o TPM Dependency: IMA integrates tightly with the TPM to record measurements
   into PCRs. The TPM must be initialized early (ideally before init_ima()),
   which aligns with IMA being built-in.

 o Security Model: IMA is part of a Trusted Computing Base (TCB). Making it a
   module would weaken the security model, as a potentially compromised system
   could delay or tamper with its initialization.

IMA must be built-in to ensure it starts measuring from the earliest possible
point in boot which inturn implies TPM must be initialised and ready to use
before IMA.

To enable integration of tpm_event_log with the IMA subsystem, the TPM drivers
(tpm_crb and tpm_crb_ffa) also needs to be built-in. However with FF-A driver
also being initialised at device initcall level, it can lead to an
initialization order issue where:
 - crb_acpi_driver_init() may run before tpm_crb_ffa_driver()_init and ffa_init()
 - As a result, probing the TPM device via CRB over FFA is deferred
 - ima_init() (called as a late initcall) runs before deferred probe completes,
   IMA fails to find the TPM and logs the below error:

   |  ima: No TPM chip found, activating TPM-bypass!

Eventually it fails to generate boot_aggregate with PCR values.

Because of the above stated dependency, the ffa driver needs to initialised
before tpm_crb_ffa module to ensure IMA finds the TPM successfully when
present.

Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index fe55613a8ea9..1a690b8186df 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -2058,7 +2058,7 @@ static int __init ffa_init(void)
 	kfree(drv_info);
 	return ret;
 }
-module_init(ffa_init);
+rootfs_initcall(ffa_init);
 
 static void __exit ffa_exit(void)
 {
-- 
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

[PATCH 2/2] tpm: tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's built-in

[Yeoreum Yun]

To integrate tpm_event_log with IMA subsystem,
tpm_crb and tpm_crb_ffa driver should be built as built-in
(CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).

However, this could make failure for ima_init() gets tpm chip when
each initcall function deployed like:

0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init

If crb_api_driver_init() is called first, probing tpm device using CRB over
FFA method is deferred since tpm_crb_ffa_driver_init() isn't called yet.

Deferred probe of the tpm device will be probed by system workqueue
in proper time after deferred_probe_initcall() registers the work.
However, ima_init() could be called first if they're deployed like:

000000000000012c l       .initcall7.init	0000000000000000 init_ima
000000000000016c l       .initcall7.init	0000000000000000 deferred_probe_initcall7

In this situation, ima_init() failed to find tpm device and it failed to
generate boot_aggregate with PCR values.

That's why kernel prints log this situation though tpm device exists:
   [    3.080786] ima: No TPM chip found, activating TPM-bypass!

To resolve this, call ffa_register() in tpm_crb_ffa_init() when it was
built with built-in so that ima can generate boot_aggregate log with
PCR values properly.

Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 drivers/char/tpm/tpm_crb_ffa.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 4ead61f01299..2ef29b66fa5d 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -115,6 +115,7 @@ struct tpm_crb_ffa {
 };
 
 static struct tpm_crb_ffa *tpm_crb_ffa;
+static struct ffa_driver tpm_crb_ffa_driver;
 
 static int tpm_crb_ffa_to_linux_errno(int errno)
 {
@@ -168,13 +169,22 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
  */
 int tpm_crb_ffa_init(void)
 {
-	if (!tpm_crb_ffa)
-		return -ENOENT;
+	int ret = 0;
 
-	if (IS_ERR_VALUE(tpm_crb_ffa))
-		return -ENODEV;
+	if (IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
+		if (!tpm_crb_ffa)
+			ret = -ENOENT;
 
-	return 0;
+		if (IS_ERR_VALUE(tpm_crb_ffa))
+			ret = -ENODEV;
+
+		return ret;
+	}
+
+	ret = ffa_register(&tpm_crb_ffa_driver);
+	BUG_ON(!ret && !tpm_crb_ffa);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
 
@@ -369,7 +379,9 @@ 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}

Re: [PATCH 2/2] tpm: tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's built-in

[Jarkko Sakkinen]

On Fri, Jun 06, 2025 at 11:57:54AM +0100, Yeoreum Yun wrote:
> To integrate tpm_event_log with IMA subsystem,
> tpm_crb and tpm_crb_ffa driver should be built as built-in
> (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> 
> However, this could make failure for ima_init() gets tpm chip when
> each initcall function deployed like:
> 
> 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init

The only failure I see is the patch 1/2 which changes init call level,
and leaves kernel Git to a broken state.

It breaks the famous "zero regressions policy".

BR, Jarkko
 

Re: [PATCH 2/2] tpm: tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's built-in

[Yeoreum Yun]

Hi Jarkko,

> > To integrate tpm_event_log with IMA subsystem,
> > tpm_crb and tpm_crb_ffa driver should be built as built-in
> > (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> >
> > However, this could make failure for ima_init() gets tpm chip when
> > each initcall function deployed like:
> >
> > 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> > 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init
>
> The only failure I see is the patch 1/2 which changes init call level,
> and leaves kernel Git to a broken state.
>
> It breaks the famous "zero regressions policy".
>
> BR, Jarkko

Sorry, would you let me know what is broken more detail?
IMHO, by changing the init call level for ffa_init()
it's called early than before device_initcall() and it seems not to
break anything.

What breaks do you mean?

Thanks.

--
Sincerely,
Yeoreum Yun

Re: [PATCH 2/2] tpm: tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's built-in

[Jarkko Sakkinen]

On Fri, Jun 06, 2025 at 07:12:43PM +0100, Yeoreum Yun wrote:
> Hi Jarkko,
> 
> > > To integrate tpm_event_log with IMA subsystem,
> > > tpm_crb and tpm_crb_ffa driver should be built as built-in
> > > (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> > >
> > > However, this could make failure for ima_init() gets tpm chip when
> > > each initcall function deployed like:
> > >
> > > 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> > > 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init
> >
> > The only failure I see is the patch 1/2 which changes init call level,
> > and leaves kernel Git to a broken state.
> >
> > It breaks the famous "zero regressions policy".
> >
> > BR, Jarkko
> 
> Sorry, would you let me know what is broken more detail?
> IMHO, by changing the init call level for ffa_init()
> it's called early than before device_initcall() and it seems not to
> break anything.
> 
> What breaks do you mean?

Your description in the cover letter and commit messages in unclear
and convoluted. Please describe exact causalities instead of something
not defined could cause "failure" (which is also abstract concept).

I'll check the next round.

> 
> Thanks.
> 
> --
> Sincerely,
> Yeoreum Yun

BR, Jarkko

Re: [PATCH 2/2] tpm: tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's built-in

[Yeoreum Yun]

Hi Jarkko,

[...]
> Your description in the cover letter and commit messages in unclear
> and convoluted. Please describe exact causalities instead of something
> not defined could cause "failure" (which is also abstract concept).
>
> I'll check the next round.

Hmm, I'll do my best to rewrite them
But I’m not sure if that would work well for you…

Thanks.

--
Sincerely,
Yeoreum Yun

Re: [PATCH 2/2] tpm: tpm_crb_ffa: maunally register tpm_crb_ffa driver when it's built-in

[Jarkko Sakkinen]

On Fri, Jun 06, 2025 at 07:12:43PM +0100, Yeoreum Yun wrote:
> Hi Jarkko,
> 
> > > To integrate tpm_event_log with IMA subsystem,
> > > tpm_crb and tpm_crb_ffa driver should be built as built-in
> > > (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> > >
> > > However, this could make failure for ima_init() gets tpm chip when
> > > each initcall function deployed like:
> > >
> > > 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> > > 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init
> >
> > The only failure I see is the patch 1/2 which changes init call level,
> > and leaves kernel Git to a broken state.
> >
> > It breaks the famous "zero regressions policy".
> >
> > BR, Jarkko
> 
> Sorry, would you let me know what is broken more detail?
> IMHO, by changing the init call level for ffa_init()
> it's called early than before device_initcall() and it seems not to
> break anything.
> 
> What breaks do you mean?

Let's start from very beginning. Why this change is needed and not just
1/2?

IMA intializes as a late initcall, which after TPM has initialized.

> 
> Thanks.
> 
> --
> Sincerely,
> Yeoreum Yun

BR, Jarkko