Linux Integrity Measurement development
 help / color / mirror / Atom feed
* Re: [RFC 0/4] tpm_crb: Add command and response buffer chunking support
From: Arun Menon @ 2026-04-23 10:43 UTC (permalink / raw)
  To: Jarkko Sakkinen, Stefan Berger
  Cc: linux-kernel, linux-integrity, Jason Gunthorpe, Peter Huewe
In-Reply-To: <adYTLq0qgWpA1kIS@kernel.org>

On Wed, Apr 08, 2026 at 11:34:54AM +0300, Jarkko Sakkinen wrote:
> On Tue, Mar 24, 2026 at 06:11:11PM +0530, Arun Menon wrote:
> > Hi Jarkko,
> > 
> > On Tue, Mar 24, 2026 at 12:41:26PM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Mar 24, 2026 at 12:47:59PM +0530, Arun Menon wrote:
> > > > The new version of TCG TPM v185 (currently under review [1]) supports
> > > > sending data/commands in chunks for the CRB (Command Response Buffer)
> > > > interface. This is in line with the initiative to support PQC algorithms.
> > > > 
> > > > This series implements the logic to send and receive larger TPM
> > > > cmd/rsp between the linux guest and the TPM backend in chunks.
> > > > Currently, the TPM CRB driver is limited by the physical size of the
> > > > MMIO window. When userspace attempts to send a payload that exceeds this
> > > > size, the driver rejects it.
> > > > 
> > > > This series introduces chunking support. The driver now checks the CRB
> > > > interface capability for CRB_INTF_CAP_CRB_CHUNK. If supported by the
> > > > backend, the driver will slice oversized commands into MMIO-sized
> > > > chunks, signalling the backend via CRB_START_NEXT_CHUNK, and finalizing
> > > > with CRB_START_INVOKE. Responses are also read back in a similar chunked
> > > > manner.
> > > > 
> > > > If the backend does not support chunking, the driver retains its legacy
> > > > behaviour and enforces the standard size limits.
> > > > 
> > > > This feature also requires the QEMU to interpret the data in chunks and
> > > > forward it to the TPM backend and subsequently dispatch the TPM response
> > > > in chunks back to the linux guest. This is implemented in [2]
> > > > 
> > > > [1] https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p07_rc1_121225.pdf
> > > > [2] https://lore.kernel.org/qemu-devel/20260319135316.37412-1-armenon@redhat.com/
> > > > 
> > > > Arun Menon (4):
> > > >   tpm_crb: Add definition of TPM CRB chunking fields
> > > >   tpm_crb: Add new wrapper function to invoke start method
> > > >   tpm_crb: Implement command and response chunking logic
> > > >   tpm: Increase TPM_BUFSIZE to 64kB for chunking support
> > > > 
> > > >  drivers/char/tpm/tpm.h     |   2 +-
> > > >  drivers/char/tpm/tpm_crb.c | 194 ++++++++++++++++++++++++++-----------
> > > >  2 files changed, 137 insertions(+), 59 deletions(-)
> > > > 
> > > > -- 
> > > > 2.53.0
> > > > 
> > > 
> > > When QEMU has the feature available?
> > 
> > The QEMU patches are in review at the moment,
> > here is the link: https://lore.kernel.org/qemu-devel/20260319135316.37412-1-armenon@redhat.com/
> > Hoping to have them merged soon.
> 
> Right, and additional question: what about swtpm?

Hi Jarkko,

Apologies for the delay in following up.
Regarding your question about swtpm, I’ve looped in Stefan Berger who 
should be able to provide more technical context on that front.

I also wanted to gently point you toward the latest revision of this
series v2, as I’ve addressed some feedback since our last exchange.
latest : https://lore.kernel.org/lkml/20260324181244.17741-1-armenon@redhat.com/

I’ve been working on the QEMU integration side in the meantime to ensure
the end-to-end flow is solid.
qemu link : https://lore.kernel.org/qemu-devel/20260422103018.123608-1-armenon@redhat.com/

I look forward to your feedback when time permits.

> 
> For both, to give detailed review, good enough is their main branch
> (i.e. as long as upstream accepts them I can use them).
> 
> > 
> > > 
> > > BR, Jarkko
> > > 
> > 
> > Regards,
> > Arun Menon
> > 
> 
> BR, Jarkko

Regards,
Arun Menon


^ permalink raw reply

* Re: [RFC 2/4] tpm_crb: Add new wrapper function to invoke start method
From: Arun Menon @ 2026-04-23 10:48 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel, linux-integrity, Jason Gunthorpe, Peter Huewe
In-Reply-To: <adYTdmCKferDsu4_@kernel.org>

On Wed, Apr 08, 2026 at 11:36:06AM +0300, Jarkko Sakkinen wrote:
> On Tue, Mar 24, 2026 at 06:13:20PM +0530, Arun Menon wrote:
> > 
> > On Tue, Mar 24, 2026 at 12:42:19PM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Mar 24, 2026 at 12:48:01PM +0530, Arun Menon wrote:
> > > > - Extract start invocation code into a separate function called
> > > >   crb_trigger_tpm. This will be helpful in repeatedly calling the
> > > >   start method with different bits set to toggle between start,
> > > >   nextChunk or crbRspRetry behaviours.
> > > > - Based on the bit set, we signal the TPM to consume
> > > >   contents of the command/response buffer.
> > > > - No functional change intended.
> > > 
> > > Please write a proper commit message, which is not a task list.
> > 
> > Yes, I shall do that. Thank you for taking a look.
> 
> Yeah, and now is good time to improve cosmetic details like this.
> 
> I rather take closer look at code when I can run it as I'm not LLM
> coding agent, I need to run to understand cannot hallucinate it :-)

Makes total sense. I have updated the commit messages in v2.
https://lore.kernel.org/lkml/20260324181244.17741-2-armenon@redhat.com/

> 
> BR, Jarkko

Regards,
Arun Menon


^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Yeoreum Yun @ 2026-04-23 10:56 UTC (permalink / raw)
  To: Will Deacon
  Cc: Sudeep Holla, Marc Zyngier, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, zohar,
	roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, oupton,
	joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	sebastianene
In-Reply-To: <aene4KFD5kbSbFRm@willie-the-truck>

Hi Will,

> On Wed, Apr 22, 2026 at 02:32:23PM +0100, Yeoreum Yun wrote:
> > Hi All,
> >
> > > > On Tue, Apr 21, 2026 at 07:57:43AM +0100, Yeoreum Yun wrote:
> > > >
> > > > [...]
> > > >
> > > > >
> > > > > Also, the FF-A initialization is not driven by a device probe, but rather
> > > > > happens as part of the bus registration itself,
> > > > > so it does not fit well with a device_link or probe deferral based approach.
> > > > >
> > > > > Instead, perhaps we could go with the idea I mentioned previously:
> > > > > either introduce a notifier, or create a pseudo ffa_device
> > > > > once pKVM initialization has completed, and
> > > > > then let the ffa driver perform the additional initialization from there.
> > > > >
> > > > > Am I missing something?
> > > > >
> > > >
> > > > In order to handle/cleanup some ugliness in interrupt management in the
> > > > FF-A driver, we may introduce DT node eventually. But it will take sometime.
> > >
> > > Unfortunately, I think this DT node wouldn't be helpful to solve
> > > this situation for dependency with the kvm misc device...
> > >
> > > IMHO, current situation, the notifier seems to good option. unless
> > > we make the initcall to recongise this dependency.
> > >
> >
> > I think the best approach for now is to introduce a notifier to handle this situation.
> > If there are no further suggestions, I’ll send a v2 based on:
> >   - https://lore.kernel.org/all/aeS4rAeVQ0yJIPYw@e129823.arm.com/
>
> I can't say that I'm a huge fan of that :/
>
> The notifier will literally fire once, for a single listener. That's
> called a function call.


I revisited Marc’s suggestion about using device links
(https://lore.kernel.org/all/87pl3vb5bm.wl-maz@kernel.org/)

but unless I’m misunderstanding something, I don’t think it would be a viable solution:
 - https://lore.kernel.org/all/aen0j3qM2k06OdXC@e129823.arm.com/#t

Also, calling functions defined by FF-A from KVM would introduce
an unnecessary module dependency between the KVM and FF-A drivers.

I’ve been trying to find an alternative approach,
but I’m not confident about what would be appropriate.

If you don’t mind, could you share your thoughts on this?

Thanks!

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 11:01 UTC (permalink / raw)
  To: Yeoreum Yun
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aem0SSQuE1e3pGOS@e129823.arm.com>

On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > 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?
> > > 
> > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > 
> > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > it by caller of ima_init().
> > 
> > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > to anything else.  Just call ima_init() a second time.
> 
> I’m not fully convinced this is sufficient.
> 
> What I meant is the case where ima_init() fails due to other
> initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).

The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
available at late_initcall.  This would be classified as a bug fix and would be
backported.  No other changes should be included in this patch.

> 
> If it fails at the late_initcall stage for such reasons, then we
> should not call ima_init() again at late_initcall_sync.
> 
> For this reason, instead of adding a static variable inside
> ima_init(), I think it would be better to manage the state in the
> caller and introduce something like an ima_initialised flag. Also, if
> initialisation fails for other reasons, the notifier block should be
> unregistered.

Defining a global file static variable, in lieu of a local static variable, is
fine. Defining two functions, one for late_initcall and another for
late_initcall_sync, that do nothing other than call ima_init() is also fine.
Please keep this patch as simple as possible.

> 
> I’d also like to ask again whether it is fine to call
> ima_update_policy_flags() and keep the notifier registered in the
> deferred TPM case. While this may be functionally acceptable, it seems
> logically questionable to do so when ima_init() has not completed.

Other than extending the TPM, IMA should behave exactly the same whether there
is a TPM or goes into TPM-bypass mode.

> 
> There is also a possibility that a deferred case ultimately fails (e.g.
> deferred at late_initcall, but then failing at late_initcall_sync
> for another reason, even while entering TPM bypass mode). In that case,
> it seems more appropriate to handle this state in the caller of
> ima_init(), rather than inside ima_init() itself.

If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
bypass mode.  Please don't make any other changes to the existing IMA behavior
and hide it here behind the late_initcall_sync change.

> 
> Am I still missing something?

When your original patch moved the initialization from late_initcall to
late_initcall_sync, you didn't question anything.  There's absolutely no
difference between that and calling ima_init twice, as long as on late_initcall
ima_init() returns immediately if the TPM chip isn't defined.

Any other changes are superfluous.  Keep the patch simple!

Mimi

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 11:20 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <56a8aab50a3b5ce0a345fc2079fb2abc7d0f1b23.camel@linux.ibm.com>

Hi Mimi,

> On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > 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?
> > > >
> > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > >
> > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > it by caller of ima_init().
> > >
> > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > to anything else.  Just call ima_init() a second time.
> >
> > I’m not fully convinced this is sufficient.
> >
> > What I meant is the case where ima_init() fails due to other
> > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
>
> The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> available at late_initcall.  This would be classified as a bug fix and would be
> backported.  No other changes should be included in this patch.

Okay.

> >
> > I’d also like to ask again whether it is fine to call
> > ima_update_policy_flags() and keep the notifier registered in the
> > deferred TPM case. While this may be functionally acceptable, it seems
> > logically questionable to do so when ima_init() has not completed.
>
> Other than extending the TPM, IMA should behave exactly the same whether there
> is a TPM or goes into TPM-bypass mode.
>
> >
> > There is also a possibility that a deferred case ultimately fails (e.g.
> > deferred at late_initcall, but then failing at late_initcall_sync
> > for another reason, even while entering TPM bypass mode). In that case,
> > it seems more appropriate to handle this state in the caller of
> > ima_init(), rather than inside ima_init() itself.
>
> If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> bypass mode.  Please don't make any other changes to the existing IMA behavior
> and hide it here behind the late_initcall_sync change.

Okay. you're talking called ima_update_policy_flags() at late_initcall
wouldn't be not a problem even in case of late_initcall_sync's ima_init()
get failed with "TPM-bypass mode".

I see then, I'll make a patch simpler then.

Thanks.

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 12:34 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeoAlVEwzRUPrlVe@e129823.arm.com>

> > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > 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?
> > > > >
> > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > >
> > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > it by caller of ima_init().
> > > >
> > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > to anything else.  Just call ima_init() a second time.
> > >
> > > I’m not fully convinced this is sufficient.
> > >
> > > What I meant is the case where ima_init() fails due to other
> > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> >
> > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > available at late_initcall.  This would be classified as a bug fix and would be
> > backported.  No other changes should be included in this patch.
>
> Okay.
>
> > >
> > > I’d also like to ask again whether it is fine to call
> > > ima_update_policy_flags() and keep the notifier registered in the
> > > deferred TPM case. While this may be functionally acceptable, it seems
> > > logically questionable to do so when ima_init() has not completed.
> >
> > Other than extending the TPM, IMA should behave exactly the same whether there
> > is a TPM or goes into TPM-bypass mode.
> >
> > >
> > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > deferred at late_initcall, but then failing at late_initcall_sync
> > > for another reason, even while entering TPM bypass mode). In that case,
> > > it seems more appropriate to handle this state in the caller of
> > > ima_init(), rather than inside ima_init() itself.
> >
> > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > and hide it here behind the late_initcall_sync change.
>
> Okay. you're talking called ima_update_policy_flags() at late_initcall
> wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> get failed with "TPM-bypass mode".
>
> I see then, I'll make a patch simpler then.

But I think in case of below situation:
  - late_initcall's first ima_init() is deferred.
  - late_initcall_sync try again but failed and try again with
    CONFIG_IMA_DEFAULT_HASH.

I would like to sustain init_ima_core to reduce the same code repeat
in late_initcall_sync.

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC 0/4] tpm_crb: Add command and response buffer chunking support
From: Jarkko Sakkinen @ 2026-04-23 12:50 UTC (permalink / raw)
  To: Arun Menon
  Cc: Stefan Berger, linux-kernel, linux-integrity, Jason Gunthorpe,
	Peter Huewe
In-Reply-To: <aen3zxxedDXlekRb@fedora>

On Thu, Apr 23, 2026 at 04:13:27PM +0530, Arun Menon wrote:
> On Wed, Apr 08, 2026 at 11:34:54AM +0300, Jarkko Sakkinen wrote:
> > On Tue, Mar 24, 2026 at 06:11:11PM +0530, Arun Menon wrote:
> > > Hi Jarkko,
> > > 
> > > On Tue, Mar 24, 2026 at 12:41:26PM +0200, Jarkko Sakkinen wrote:
> > > > On Tue, Mar 24, 2026 at 12:47:59PM +0530, Arun Menon wrote:
> > > > > The new version of TCG TPM v185 (currently under review [1]) supports
> > > > > sending data/commands in chunks for the CRB (Command Response Buffer)
> > > > > interface. This is in line with the initiative to support PQC algorithms.
> > > > > 
> > > > > This series implements the logic to send and receive larger TPM
> > > > > cmd/rsp between the linux guest and the TPM backend in chunks.
> > > > > Currently, the TPM CRB driver is limited by the physical size of the
> > > > > MMIO window. When userspace attempts to send a payload that exceeds this
> > > > > size, the driver rejects it.
> > > > > 
> > > > > This series introduces chunking support. The driver now checks the CRB
> > > > > interface capability for CRB_INTF_CAP_CRB_CHUNK. If supported by the
> > > > > backend, the driver will slice oversized commands into MMIO-sized
> > > > > chunks, signalling the backend via CRB_START_NEXT_CHUNK, and finalizing
> > > > > with CRB_START_INVOKE. Responses are also read back in a similar chunked
> > > > > manner.
> > > > > 
> > > > > If the backend does not support chunking, the driver retains its legacy
> > > > > behaviour and enforces the standard size limits.
> > > > > 
> > > > > This feature also requires the QEMU to interpret the data in chunks and
> > > > > forward it to the TPM backend and subsequently dispatch the TPM response
> > > > > in chunks back to the linux guest. This is implemented in [2]
> > > > > 
> > > > > [1] https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p07_rc1_121225.pdf
> > > > > [2] https://lore.kernel.org/qemu-devel/20260319135316.37412-1-armenon@redhat.com/
> > > > > 
> > > > > Arun Menon (4):
> > > > >   tpm_crb: Add definition of TPM CRB chunking fields
> > > > >   tpm_crb: Add new wrapper function to invoke start method
> > > > >   tpm_crb: Implement command and response chunking logic
> > > > >   tpm: Increase TPM_BUFSIZE to 64kB for chunking support
> > > > > 
> > > > >  drivers/char/tpm/tpm.h     |   2 +-
> > > > >  drivers/char/tpm/tpm_crb.c | 194 ++++++++++++++++++++++++++-----------
> > > > >  2 files changed, 137 insertions(+), 59 deletions(-)
> > > > > 
> > > > > -- 
> > > > > 2.53.0
> > > > > 
> > > > 
> > > > When QEMU has the feature available?
> > > 
> > > The QEMU patches are in review at the moment,
> > > here is the link: https://lore.kernel.org/qemu-devel/20260319135316.37412-1-armenon@redhat.com/
> > > Hoping to have them merged soon.
> > 
> > Right, and additional question: what about swtpm?
> 
> Hi Jarkko,
> 
> Apologies for the delay in following up.
> Regarding your question about swtpm, I’ve looped in Stefan Berger who 
> should be able to provide more technical context on that front.
> 
> I also wanted to gently point you toward the latest revision of this
> series v2, as I’ve addressed some feedback since our last exchange.
> latest : https://lore.kernel.org/lkml/20260324181244.17741-1-armenon@redhat.com/
> 
> I’ve been working on the QEMU integration side in the meantime to ensure
> the end-to-end flow is solid.
> qemu link : https://lore.kernel.org/qemu-devel/20260422103018.123608-1-armenon@redhat.com/
> 
> I look forward to your feedback when time permits.

Thanks I'll move this to my TODO folder for detailed look. I'm
right now travelling in Hungary but will look in detail next
week.

BR, Jarkko

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Jonathan McDowell @ 2026-04-23 12:53 UTC (permalink / raw)
  To: Yeoreum Yun
  Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeoRxWPyOHGJd+Jh@e129823.arm.com>

On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
>> > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
>> > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
>> > > > > > 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?
>> > > > >
>> > > > > Also, if we handle in ima_init() only, but it failed with other reason,
>> > > > > we shouldn't call again ima_init() in the late_initcall_sync.
>> > > > >
>> > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
>> > > > > it by caller of ima_init().
>> > > >
>> > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
>> > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
>> > > > to anything else.  Just call ima_init() a second time.
>> > >
>> > > I’m not fully convinced this is sufficient.
>> > >
>> > > What I meant is the case where ima_init() fails due to other
>> > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
>> >
>> > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
>> > available at late_initcall.  This would be classified as a bug fix and would be
>> > backported.  No other changes should be included in this patch.
>>
>> Okay.
>>
>> > >
>> > > I’d also like to ask again whether it is fine to call
>> > > ima_update_policy_flags() and keep the notifier registered in the
>> > > deferred TPM case. While this may be functionally acceptable, it seems
>> > > logically questionable to do so when ima_init() has not completed.
>> >
>> > Other than extending the TPM, IMA should behave exactly the same whether there
>> > is a TPM or goes into TPM-bypass mode.
>> >
>> > >
>> > > There is also a possibility that a deferred case ultimately fails (e.g.
>> > > deferred at late_initcall, but then failing at late_initcall_sync
>> > > for another reason, even while entering TPM bypass mode). In that case,
>> > > it seems more appropriate to handle this state in the caller of
>> > > ima_init(), rather than inside ima_init() itself.
>> >
>> > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
>> > bypass mode.  Please don't make any other changes to the existing IMA behavior
>> > and hide it here behind the late_initcall_sync change.
>>
>> Okay. you're talking called ima_update_policy_flags() at late_initcall
>> wouldn't be not a problem even in case of late_initcall_sync's ima_init()
>> get failed with "TPM-bypass mode".
>>
>> I see then, I'll make a patch simpler then.
>
>But I think in case of below situation:
>  - late_initcall's first ima_init() is deferred.
>  - late_initcall_sync try again but failed and try again with
>    CONFIG_IMA_DEFAULT_HASH.
>
>I would like to sustain init_ima_core to reduce the same code repeat
>in late_initcall_sync.

I think what Mimi's proposing is:

If we're in late_initcall, and the TPM isn't available, return 
immediately with an error (the EPROBE_DEFER?), don't do any init.

If we're in late_initcall_sync, either we're already initialised, so do 
return and nothing, or run through the entire flow, even if the TPM 
isn't unavailable.

So ima_init() just needs to know a) if it's in the sync or non-sync mode 
and b) for the sync mode, if we've already done the init at
non-sync.

J.

-- 
... I'm not popular enough to be different.

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 13:07 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeoWO2Cwo04YYu2l@earth.li>

Hi,

> > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > 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?
> > > > > > >
> > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > >
> > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > it by caller of ima_init().
> > > > > >
> > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > to anything else.  Just call ima_init() a second time.
> > > > >
> > > > > I’m not fully convinced this is sufficient.
> > > > >
> > > > > What I meant is the case where ima_init() fails due to other
> > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > >
> > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > backported.  No other changes should be included in this patch.
> > >
> > > Okay.
> > >
> > > > >
> > > > > I’d also like to ask again whether it is fine to call
> > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > logically questionable to do so when ima_init() has not completed.
> > > >
> > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > is a TPM or goes into TPM-bypass mode.
> > > >
> > > > >
> > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > it seems more appropriate to handle this state in the caller of
> > > > > ima_init(), rather than inside ima_init() itself.
> > > >
> > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > and hide it here behind the late_initcall_sync change.
> > >
> > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > get failed with "TPM-bypass mode".
> > >
> > > I see then, I'll make a patch simpler then.
> >
> > But I think in case of below situation:
> >  - late_initcall's first ima_init() is deferred.
> >  - late_initcall_sync try again but failed and try again with
> >    CONFIG_IMA_DEFAULT_HASH.
> >
> > I would like to sustain init_ima_core to reduce the same code repeat
> > in late_initcall_sync.
>
> I think what Mimi's proposing is:
>
> If we're in late_initcall, and the TPM isn't available, return immediately
> with an error (the EPROBE_DEFER?), don't do any init.
>
> If we're in late_initcall_sync, either we're already initialised, so do
> return and nothing, or run through the entire flow, even if the TPM isn't
> unavailable.
>
> So ima_init() just needs to know a) if it's in the sync or non-sync mode and
> b) for the sync mode, if we've already done the init at
> non-sync.

But think think about when "late_initcall_sync" happens.
In case of it, whether TPM present or by-pass mode, if it failed,
it try again with the DEFAULT_HASH if hash isn't use DEFAULT one
(e.x. user set boot arguments hash_setup=md5).

IOW, late_initcall_sync should call twice just like former code do this.
I mean to wrap this duplication of code with init_core_ima().
so that int late_initcall_sync in case of deferred case to try agina
ima_init() with the DEFAULT HASH.

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [PATCH] ima: return error early if file xattr cannot be changed
From: Mimi Zohar @ 2026-04-23 13:11 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-integrity
In-Reply-To: <aposxvqsrlbe7gtyvtsdh5nyg5sgofimerqpt6ez4fbxhtqyjj@4u3othdcgipp>

On Wed, 2026-04-22 at 07:34 -0400, Goldwyn Rodrigues wrote:
> During early boot, the filesystem is read-only and any changes
> to xattrs are not allowed. This fails in case of ext4 because
> changing xattr starts an ext4 transaction which fails with the
> following warning.
> 
> WARNING: fs/ext4/ext4_jbd2.c:75 at ext4_journal_check_start+0x63/0xa0 [ext4], CPU#1: systemd-sysroot/561
> CPU: 1 UID: 0 PID: 561 Comm: systemd-sysroot Not tainted 6.19.12-1-default #1 PREEMPT(voluntary) openSUSE Tumbleweed  c2dfc3c9d9f6f1233251c5d4410574fe82a348ee
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
> RIP: 0010:ext4_journal_check_start+0x63/0xa0 [ext4]
> Call Trace:
>   __ext4_journal_start_sb+0x3e/0x180 [ext4 6d025f3bc52c89a957b89a89d211fadf5e9434e1]
>   ext4_xattr_set+0x104/0x150 [ext4 6d025f3bc52c89a957b89a89d211fadf5e9434e1]
>   __vfs_setxattr+0x9a/0xd0
>   __vfs_setxattr_noperm+0x76/0x1f0
>   ima_appraise_measurement+0x23e/0xe40
>   ima_d_path+0x5a/0xd0
>   process_measurement+0xb29/0xc40
>   ? copy_from_kernel_nofault+0x21/0xe0
>   ? fscrypt_file_open+0xc0/0xe0
>   ? ext4_file_open+0x60/0x490 [ext4 6d025f3bc52c89a957b89a89d211fadf5e9434e1]
>   ? bpf_prog_31efb7c56239148b_restrict_filesystems+0xab/0x126
>   ? __bpf_prog_exit+0x23/0xd0
>   ? __bpf_tramp_exit+0xd/0x50
>   ? bpf_trampoline_6442530367+0x9f/0xea
>   ima_file_check+0x57/0x80
>   security_file_post_open+0x50/0xf0
>   path_openat+0x493/0x1650
>   do_filp_open+0xc7/0x170
> 
> Detect the state of the file early and return the error.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

Thank you!  The patch makes a lot of sense.

Mimi

> ---
>  security/integrity/ima/ima_appraise.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 870dde67707b..6a4dda46114c 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -90,6 +90,11 @@ static int ima_fix_xattr(struct dentry *dentry,
>  	int rc, offset;
>  	u8 algo = iint->ima_hash->algo;
>  
> +	if (IS_RDONLY(d_inode(dentry)))
> +		return -EROFS;
> +	if (IS_IMMUTABLE(d_inode(dentry)))
> +		return -EPERM;
> +
>  	if (algo <= HASH_ALGO_SHA1) {
>  		offset = 1;
>  		iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST;

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 13:43 UTC (permalink / raw)
  To: Jonathan McDowell, Yeoreum Yun
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeoWO2Cwo04YYu2l@earth.li>

On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > 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?
> > > > > > > 
> > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > 
> > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > it by caller of ima_init().
> > > > > > 
> > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > to anything else.  Just call ima_init() a second time.
> > > > > 
> > > > > I’m not fully convinced this is sufficient.
> > > > > 
> > > > > What I meant is the case where ima_init() fails due to other
> > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > 
> > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > backported.  No other changes should be included in this patch.
> > > 
> > > Okay.
> > > 
> > > > > 
> > > > > I’d also like to ask again whether it is fine to call
> > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > logically questionable to do so when ima_init() has not completed.
> > > > 
> > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > is a TPM or goes into TPM-bypass mode.
> > > > 
> > > > > 
> > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > it seems more appropriate to handle this state in the caller of
> > > > > ima_init(), rather than inside ima_init() itself.
> > > > 
> > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > and hide it here behind the late_initcall_sync change.
> > > 
> > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > get failed with "TPM-bypass mode".
> > > 
> > > I see then, I'll make a patch simpler then.
> > 
> > But I think in case of below situation:
> >  - late_initcall's first ima_init() is deferred.
> >  - late_initcall_sync try again but failed and try again with
> >    CONFIG_IMA_DEFAULT_HASH.
> > 
> > I would like to sustain init_ima_core to reduce the same code repeat
> > in late_initcall_sync.
> 
> I think what Mimi's proposing is:
> 
> If we're in late_initcall, and the TPM isn't available, return 
> immediately with an error (the EPROBE_DEFER?), don't do any init.
> 
> If we're in late_initcall_sync, either we're already initialised, so do 
> return and nothing, or run through the entire flow, even if the TPM 
> isn't unavailable.
> 
> So ima_init() just needs to know a) if it's in the sync or non-sync mode 
> and b) for the sync mode, if we've already done the init at
> non-sync.

Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
should not be included in this patch.  Since Yeoreum is not hearing me, feel
free to post a patch.

Mimi

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 13:55 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Jonathan McDowell, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
	roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, jgg,
	sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will, noodles, sebastianene
In-Reply-To: <bd908e28298d968740d03c97bc7e441de188b7b4.camel@linux.ibm.com>

> On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > 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?
> > > > > > > >
> > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > >
> > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > it by caller of ima_init().
> > > > > > >
> > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > > to anything else.  Just call ima_init() a second time.
> > > > > >
> > > > > > I’m not fully convinced this is sufficient.
> > > > > >
> > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > >
> > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > > backported.  No other changes should be included in this patch.
> > > >
> > > > Okay.
> > > >
> > > > > >
> > > > > > I’d also like to ask again whether it is fine to call
> > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > logically questionable to do so when ima_init() has not completed.
> > > > >
> > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > is a TPM or goes into TPM-bypass mode.
> > > > >
> > > > > >
> > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > ima_init(), rather than inside ima_init() itself.
> > > > >
> > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > > and hide it here behind the late_initcall_sync change.
> > > >
> > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > get failed with "TPM-bypass mode".
> > > >
> > > > I see then, I'll make a patch simpler then.
> > >
> > > But I think in case of below situation:
> > >  - late_initcall's first ima_init() is deferred.
> > >  - late_initcall_sync try again but failed and try again with
> > >    CONFIG_IMA_DEFAULT_HASH.
> > >
> > > I would like to sustain init_ima_core to reduce the same code repeat
> > > in late_initcall_sync.
> >
> > I think what Mimi's proposing is:
> >
> > If we're in late_initcall, and the TPM isn't available, return
> > immediately with an error (the EPROBE_DEFER?), don't do any init.
> >
> > If we're in late_initcall_sync, either we're already initialised, so do
> > return and nothing, or run through the entire flow, even if the TPM
> > isn't unavailable.
> >
> > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > and b) for the sync mode, if we've already done the init at
> > non-sync.
>
> Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
> should not be included in this patch.  Since Yeoreum is not hearing me, feel
> free to post a patch.

I see. so what you need to is this only
If it looks good to you. I'll send it at v3.

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_init.c b/security/integrity/ima/ima_init.c
index a2f34f2d8ad7..334fa8927c45 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -118,10 +118,22 @@ void __init ima_load_x509(void)
 int __init ima_init(void)
 {
        int rc;
+       static bool deferred = false;
+       static bool initialised = false;
+
+       if (initialised)
+               return 0;

        ima_tpm_chip = tpm_default_chip();
-       if (!ima_tpm_chip)
+       if (!ima_tpm_chip) {
+               if (!deferred) {
+                       pr_info("Defer initialisation to the late_initcall_sync stage.\n");
+                       deferred = true;
+                       return 0;
+               }
+
                pr_info("No TPM chip found, activating TPM-bypass!\n");
+       }

        rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
        if (rc)
@@ -158,5 +170,7 @@ int __init ima_init(void)
                                  UTS_RELEASE, strlen(UTS_RELEASE), false,
                                  NULL, 0);

+       initialised = true;
+
        return rc;
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1d6229b156fb..847ec74a183d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1274,6 +1274,11 @@ static int __init init_ima(void)
        return error;
 }

+static int __init late_init_ima(void)
+{
+       return ima_init();
+}
+
 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 +1326,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/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);
...skipping...
+
                pr_info("No TPM chip found, activating TPM-bypass!\n");
+       }

        rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
        if (rc)
@@ -158,5 +170,7 @@ int __init ima_init(void)
                                  UTS_RELEASE, strlen(UTS_RELEASE), false,
                                  NULL, 0);

+       initialised = true;
+
        return rc;
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1d6229b156fb..847ec74a183d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1274,6 +1274,11 @@ static int __init init_ima(void)
        return error;
 }

+static int __init late_init_ima(void)
+{
+       return ima_init();
+}
+
 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 +1326,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/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);


--
Sincerely,
Yeoreum Yun

^ permalink raw reply related

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Jonathan McDowell @ 2026-04-23 14:03 UTC (permalink / raw)
  To: Yeoreum Yun
  Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeokwrC86WI7uT+K@e129823.arm.com>

On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
>> On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
>> > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
>> > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
>> > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
>> > > > > > > > > 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?
>> > > > > > > >
>> > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
>> > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
>> > > > > > > >
>> > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
>> > > > > > > > it by caller of ima_init().
>> > > > > > >
>> > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
>> > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
>> > > > > > > to anything else.  Just call ima_init() a second time.
>> > > > > >
>> > > > > > I’m not fully convinced this is sufficient.
>> > > > > >
>> > > > > > What I meant is the case where ima_init() fails due to other
>> > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
>> > > > >
>> > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
>> > > > > available at late_initcall.  This would be classified as a bug fix and would be
>> > > > > backported.  No other changes should be included in this patch.
>> > > >
>> > > > Okay.
>> > > >
>> > > > > >
>> > > > > > I’d also like to ask again whether it is fine to call
>> > > > > > ima_update_policy_flags() and keep the notifier registered in the
>> > > > > > deferred TPM case. While this may be functionally acceptable, it seems
>> > > > > > logically questionable to do so when ima_init() has not completed.
>> > > > >
>> > > > > Other than extending the TPM, IMA should behave exactly the same whether there
>> > > > > is a TPM or goes into TPM-bypass mode.
>> > > > >
>> > > > > >
>> > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
>> > > > > > deferred at late_initcall, but then failing at late_initcall_sync
>> > > > > > for another reason, even while entering TPM bypass mode). In that case,
>> > > > > > it seems more appropriate to handle this state in the caller of
>> > > > > > ima_init(), rather than inside ima_init() itself.
>> > > > >
>> > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
>> > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
>> > > > > and hide it here behind the late_initcall_sync change.
>> > > >
>> > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
>> > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
>> > > > get failed with "TPM-bypass mode".
>> > > >
>> > > > I see then, I'll make a patch simpler then.
>> > >
>> > > But I think in case of below situation:
>> > >  - late_initcall's first ima_init() is deferred.
>> > >  - late_initcall_sync try again but failed and try again with
>> > >    CONFIG_IMA_DEFAULT_HASH.
>> > >
>> > > I would like to sustain init_ima_core to reduce the same code repeat
>> > > in late_initcall_sync.
>> >
>> > I think what Mimi's proposing is:
>> >
>> > If we're in late_initcall, and the TPM isn't available, return
>> > immediately with an error (the EPROBE_DEFER?), don't do any init.
>> >
>> > If we're in late_initcall_sync, either we're already initialised, so do
>> > return and nothing, or run through the entire flow, even if the TPM
>> > isn't unavailable.
>> >
>> > So ima_init() just needs to know a) if it's in the sync or non-sync mode
>> > and b) for the sync mode, if we've already done the init at
>> > non-sync.
>>
>> Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
>> should not be included in this patch.  Since Yeoreum is not hearing me, feel
>> free to post a patch.
>
>I see. so what you need to is this only
>If it looks good to you. I'll send it at v3.

FWIW, I pulled the tpm_default_chip check out a level to account for the 
extra init you mentioned, and have the following (completely untested or 
compiled, but gives the approach):

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_init.c b/security/integrity/ima/ima_init.c
index a2f34f2d8ad7..a60dfb8316d8 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -119,10 +119,6 @@ int __init ima_init(void)
  {
  	int rc;
  
-	ima_tpm_chip = tpm_default_chip();
-	if (!ima_tpm_chip)
-		pr_info("No TPM chip found, activating TPM-bypass!\n");
-
  	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
  	if (rc)
  		return rc;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1d6229b156fb..b60a85fa803a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
  
  #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
  
-static int __init init_ima(void)
+static int __init init_ima(bool sync)
  {
  	int error;
  
@@ -1247,6 +1247,19 @@ static int __init init_ima(void)
  		return 0;
  	}
  
+	/* If we found the TPM during our first attempt, nothing further to do */
+	if (sync && ima_tpm_chip)
+		return 0;
+
+	ima_tpm_chip = tpm_default_chip();
+	if (!ima_tpm_chip && !sync) {
+		pr_debug("TPM not available, will try later\n");
+		return -EPROBE_DEFER;
+	}
+
+	if (!ima_tpm_chip)
+		pr_info("No TPM chip found, activating TPM-bypass!\n");
+
  	ima_appraise_parse_cmdline();
  	ima_init_template_list();
  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
@@ -1274,6 +1287,16 @@ static int __init init_ima(void)
  	return error;
  }
  
+static int __init init_ima_late(void)
+{
+	return init_ima(false);
+}
+
+static int __init init_ima_late_sync(void)
+{
+	return init_ima(true);
+}
+
  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),
@@ -1319,6 +1342,7 @@ DEFINE_LSM(ima) = {
  	.init = init_ima_lsm,
  	.order = LSM_ORDER_LAST,
  	.blobs = &ima_blob_sizes,
-	/* Start IMA after the TPM is available */
-	.initcall_late = init_ima,
+	/* Ensure we start IMA after the TPM is available */
+	.initcall_late = init_ima_late,
+	.initcall_late_sync = init_ima_late_sync,
  };
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);


J.

-- 
Rock and roll stops the traffic.
This .sig brought to you by the letter A and the number 40
Product of the Republic of HuggieTag

^ permalink raw reply related

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 14:33 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeomlp3I0eVE5mce@earth.li>

Hi Jonathan,

> * # Be careful, this email looks suspicious; * Out of Character: The sender is exhibiting a significant deviation from their usual behavior, this may indicate that their account has been compromised. Be extra cautious before opening links or attachments. *
> On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
> > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > 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?
> > > > > > > > > >
> > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > > > >
> > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > > > it by caller of ima_init().
> > > > > > > > >
> > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > > > > to anything else.  Just call ima_init() a second time.
> > > > > > > >
> > > > > > > > I’m not fully convinced this is sufficient.
> > > > > > > >
> > > > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > > > >
> > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > > > > backported.  No other changes should be included in this patch.
> > > > > >
> > > > > > Okay.
> > > > > >
> > > > > > > >
> > > > > > > > I’d also like to ask again whether it is fine to call
> > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > > > logically questionable to do so when ima_init() has not completed.
> > > > > > >
> > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > > > is a TPM or goes into TPM-bypass mode.
> > > > > > >
> > > > > > > >
> > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > > > ima_init(), rather than inside ima_init() itself.
> > > > > > >
> > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > > > > and hide it here behind the late_initcall_sync change.
> > > > > >
> > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > > > get failed with "TPM-bypass mode".
> > > > > >
> > > > > > I see then, I'll make a patch simpler then.
> > > > >
> > > > > But I think in case of below situation:
> > > > >  - late_initcall's first ima_init() is deferred.
> > > > >  - late_initcall_sync try again but failed and try again with
> > > > >    CONFIG_IMA_DEFAULT_HASH.
> > > > >
> > > > > I would like to sustain init_ima_core to reduce the same code repeat
> > > > > in late_initcall_sync.
> > > >
> > > > I think what Mimi's proposing is:
> > > >
> > > > If we're in late_initcall, and the TPM isn't available, return
> > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > >
> > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > return and nothing, or run through the entire flow, even if the TPM
> > > > isn't unavailable.
> > > >
> > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > and b) for the sync mode, if we've already done the init at
> > > > non-sync.
> > >
> > > Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
> > > should not be included in this patch.  Since Yeoreum is not hearing me, feel
> > > free to post a patch.
> >
> > I see. so what you need to is this only
> > If it looks good to you. I'll send it at v3.
>
> FWIW, I pulled the tpm_default_chip check out a level to account for the
> extra init you mentioned, and have the following (completely untested or
> compiled, but gives the approach):
>
> 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_init.c b/security/integrity/ima/ima_init.c
> index a2f34f2d8ad7..a60dfb8316d8 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -119,10 +119,6 @@ int __init ima_init(void)
>  {
>  	int rc;
> -	ima_tpm_chip = tpm_default_chip();
> -	if (!ima_tpm_chip)
> -		pr_info("No TPM chip found, activating TPM-bypass!\n");
> -
>  	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
>  	if (rc)
>  		return rc;
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 1d6229b156fb..b60a85fa803a 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
>  #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> -static int __init init_ima(void)
> +static int __init init_ima(bool sync)
>  {
>  	int error;
> @@ -1247,6 +1247,19 @@ static int __init init_ima(void)
>  		return 0;
>  	}
> +	/* If we found the TPM during our first attempt, nothing further to do */
> +	if (sync && ima_tpm_chip)
> +		return 0;
> +
> +	ima_tpm_chip = tpm_default_chip();
> +	if (!ima_tpm_chip && !sync) {
> +		pr_debug("TPM not available, will try later\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	if (!ima_tpm_chip)
> +		pr_info("No TPM chip found, activating TPM-bypass!\n");
> +
>  	ima_appraise_parse_cmdline();
>  	ima_init_template_list();
>  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> @@ -1274,6 +1287,16 @@ static int __init init_ima(void)
>  	return error;
>  }
> +static int __init init_ima_late(void)
> +{
> +	return init_ima(false);
> +}
> +
> +static int __init init_ima_late_sync(void)
> +{
> +	return init_ima(true);
> +}
> +
>  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),
> @@ -1319,6 +1342,7 @@ DEFINE_LSM(ima) = {
>  	.init = init_ima_lsm,
>  	.order = LSM_ORDER_LAST,
>  	.blobs = &ima_blob_sizes,
> -	/* Start IMA after the TPM is available */
> -	.initcall_late = init_ima,
> +	/* Ensure we start IMA after the TPM is available */
> +	.initcall_late = init_ima_late,
> +	.initcall_late_sync = init_ima_late_sync,
>  };
> 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);

I'm fine this. but are we talking about "ima_init()" not "init_ima()"?
Because of this, I've fixuated and make a long stupid speaking myself.

If this seems good to Mimi, I don't care who send it.
But If you're going to send this, could you includes 2 and 3 too?

Thanks.

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 14:48 UTC (permalink / raw)
  To: Jonathan McDowell, Yeoreum Yun
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeomlp3I0eVE5mce@earth.li>

On Thu, 2026-04-23 at 15:03 +0100, Jonathan McDowell wrote:
> On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
> > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > 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?
> > > > > > > > > > 
> > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > > > > 
> > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > > > it by caller of ima_init().
> > > > > > > > > 
> > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > > > > to anything else.  Just call ima_init() a second time.
> > > > > > > > 
> > > > > > > > I’m not fully convinced this is sufficient.
> > > > > > > > 
> > > > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > > > > 
> > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > > > > backported.  No other changes should be included in this patch.
> > > > > > 
> > > > > > Okay.
> > > > > > 
> > > > > > > > 
> > > > > > > > I’d also like to ask again whether it is fine to call
> > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > > > logically questionable to do so when ima_init() has not completed.
> > > > > > > 
> > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > > > is a TPM or goes into TPM-bypass mode.
> > > > > > > 
> > > > > > > > 
> > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > > > ima_init(), rather than inside ima_init() itself.
> > > > > > > 
> > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > > > > and hide it here behind the late_initcall_sync change.
> > > > > > 
> > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > > > get failed with "TPM-bypass mode".
> > > > > > 
> > > > > > I see then, I'll make a patch simpler then.
> > > > > 
> > > > > But I think in case of below situation:
> > > > >  - late_initcall's first ima_init() is deferred.
> > > > >  - late_initcall_sync try again but failed and try again with
> > > > >    CONFIG_IMA_DEFAULT_HASH.
> > > > > 
> > > > > I would like to sustain init_ima_core to reduce the same code repeat
> > > > > in late_initcall_sync.
> > > > 
> > > > I think what Mimi's proposing is:
> > > > 
> > > > If we're in late_initcall, and the TPM isn't available, return
> > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > 
> > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > return and nothing, or run through the entire flow, even if the TPM
> > > > isn't unavailable.
> > > > 
> > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > and b) for the sync mode, if we've already done the init at
> > > > non-sync.
> > > 
> > > Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
> > > should not be included in this patch.  Since Yeoreum is not hearing me, feel
> > > free to post a patch.
> > 
> > I see. so what you need to is this only
> > If it looks good to you. I'll send it at v3.
> 
> FWIW, I pulled the tpm_default_chip check out a level to account for the 
> extra init you mentioned, and have the following (completely untested or 
> compiled, but gives the approach):

Thanks, Jonathan!  It looks good.  Similarly untested/compiled.

Emitting a message on failure to initialize IMA at late_initcall is good, but
the attestation service won't know.  Could you somehow differentiate between the
late_initcall and late_initcall_sync boot_aggregate records?

Mimi

^ permalink raw reply

* Re: [PATCH v3 0/2] tpm_tis: fix retry exhaustion and add logging
From: Jarkko Sakkinen @ 2026-04-23 16:43 UTC (permalink / raw)
  To: Jacqueline Wong
  Cc: Axel Rasmussen, linux-integrity, peterhuewe, jgg, jhand, LKML
In-Reply-To: <CAK8FdkqPzOiZZHsZPbMQg0s_=+rU6ENtBx-6YtuekyiqG8cvPw@mail.gmail.com>

OK cool, thanks for heads up.

I'll most likely address this mess I'm having ATM by doing rc2
PR with the bug fixes.

BR, Jarkko

On Thu, Apr 23, 2026 at 08:29:47AM -0700, Jacqueline Wong wrote:
> Thanks Jarkko, the patches look correct to me.
> 
> Thanks,
> Jacqueline
> 
> On Thu, Apr 23, 2026 at 3:21 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
>     On Wed, Apr 22, 2026 at 04:00:17PM -0700, Axel Rasmussen wrote:
>     > On Wed, Apr 15, 2026 at 9:00 AM Jacqueline Wong <jacqwong@google.com>
>     wrote:
>     > >
>     > > The Fix:
>     > > - Patch 1: Adds error logs to identify the specific hardware status
>     mismatch.
>     > > - Patch 2: Stops execution immediately when retries are exhausted.
>     > >
>     > > v3 changes:
>     > > - Improved code alignment to pass checkpatch --strict.
>     >
>     > Thanks for sending the v3 Jacqueline!
>     > I suspect this may have slipped under folks' radar by not CC'ing
>     +linux-kernel@.
> 
>     Please check that I carry the correct version of patches:
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log
>     /?h=for-next-tpm
> 
>     This is related to:
> 
>     1. https://lore.kernel.org/linux-integrity/aeVSbVIFaCDRXf7C@kernel.org/
>     2. https://lore.kernel.org/linux-integrity/CAHk-=
>     wiPWCUHsNvzKep7z4VGaL-Brx6Zmh7Efn28WWTPbwn5dA@mail.gmail.com/
>     3. https://lore.kernel.org/all/aee_mCW8p2J6IbIO@kernel.org/
> 
>     I will try to send PRs after my split next branches are sync to -next.
> 
>     BR, Jarkko
> 

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Jonathan McDowell @ 2026-04-23 17:02 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <2866f7679fe6933de667ce74ae68bd4ea9198e2a.camel@linux.ibm.com>

On Thu, Apr 23, 2026 at 10:48:49AM -0400, Mimi Zohar wrote:
>On Thu, 2026-04-23 at 15:03 +0100, Jonathan McDowell wrote:
>> On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
>> > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
>> > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
>> > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
>> > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
>> > > > > > > > > > > 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?
>> > > > > > > > > >
>> > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
>> > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
>> > > > > > > > > >
>> > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
>> > > > > > > > > > it by caller of ima_init().
>> > > > > > > > >
>> > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
>> > > > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
>> > > > > > > > > to anything else.  Just call ima_init() a second time.
>> > > > > > > >
>> > > > > > > > I’m not fully convinced this is sufficient.
>> > > > > > > >
>> > > > > > > > What I meant is the case where ima_init() fails due to other
>> > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
>> > > > > > >
>> > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
>> > > > > > > available at late_initcall.  This would be classified as a bug fix and would be
>> > > > > > > backported.  No other changes should be included in this patch.
>> > > > > >
>> > > > > > Okay.
>> > > > > >
>> > > > > > > >
>> > > > > > > > I’d also like to ask again whether it is fine to call
>> > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
>> > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
>> > > > > > > > logically questionable to do so when ima_init() has not completed.
>> > > > > > >
>> > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
>> > > > > > > is a TPM or goes into TPM-bypass mode.
>> > > > > > >
>> > > > > > > >
>> > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
>> > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
>> > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
>> > > > > > > > it seems more appropriate to handle this state in the caller of
>> > > > > > > > ima_init(), rather than inside ima_init() itself.
>> > > > > > >
>> > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
>> > > > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
>> > > > > > > and hide it here behind the late_initcall_sync change.
>> > > > > >
>> > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
>> > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
>> > > > > > get failed with "TPM-bypass mode".
>> > > > > >
>> > > > > > I see then, I'll make a patch simpler then.
>> > > > >
>> > > > > But I think in case of below situation:
>> > > > >  - late_initcall's first ima_init() is deferred.
>> > > > >  - late_initcall_sync try again but failed and try again with
>> > > > >    CONFIG_IMA_DEFAULT_HASH.
>> > > > >
>> > > > > I would like to sustain init_ima_core to reduce the same code repeat
>> > > > > in late_initcall_sync.
>> > > >
>> > > > I think what Mimi's proposing is:
>> > > >
>> > > > If we're in late_initcall, and the TPM isn't available, return
>> > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
>> > > >
>> > > > If we're in late_initcall_sync, either we're already initialised, so do
>> > > > return and nothing, or run through the entire flow, even if the TPM
>> > > > isn't unavailable.
>> > > >
>> > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
>> > > > and b) for the sync mode, if we've already done the init at
>> > > > non-sync.
>> > >
>> > > Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
>> > > should not be included in this patch.  Since Yeoreum is not hearing me, feel
>> > > free to post a patch.
>> >
>> > I see. so what you need to is this only
>> > If it looks good to you. I'll send it at v3.
>>
>> FWIW, I pulled the tpm_default_chip check out a level to account for the
>> extra init you mentioned, and have the following (completely untested or
>> compiled, but gives the approach):
>
>Thanks, Jonathan!  It looks good.  Similarly untested/compiled.

FWIW, it does compile.

>Emitting a message on failure to initialize IMA at late_initcall is good, but
>the attestation service won't know.  Could you somehow differentiate between the
>late_initcall and late_initcall_sync boot_aggregate records?

Are you thinking "boot_aggregate" and "boot_aggregate_late" or similar 
as the "filename" on the entries, just so it's clear when we did the 
init in the log, or something else?

J.

-- 
/-\                             | 101 things you can't have too much
|@/  Debian GNU/Linux Developer |      of : 39 - silver bullets.
\-                              |

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 17:13 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aepQwcY523aukAvw@earth.li>

On Thu, 2026-04-23 at 18:02 +0100, Jonathan McDowell wrote:
> > > > > > 
> > > > > > I think what Mimi's proposing is:
> > > > > > 
> > > > > > If we're in late_initcall, and the TPM isn't available, return
> > > > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > > > 
> > > > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > > > return and nothing, or run through the entire flow, even if the TPM
> > > > > > isn't unavailable.
> > > > > > 
> > > > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > > > and b) for the sync mode, if we've already done the init at
> > > > > > non-sync.
> > > > > 
> > > > > Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
> > > > > should not be included in this patch.  Since Yeoreum is not hearing me, feel
> > > > > free to post a patch.
> > > > 
> > > > I see. so what you need to is this only
> > > > If it looks good to you. I'll send it at v3.
> > > 
> > > FWIW, I pulled the tpm_default_chip check out a level to account for the
> > > extra init you mentioned, and have the following (completely untested or
> > > compiled, but gives the approach):
> > 
> > Thanks, Jonathan!  It looks good.  Similarly untested/compiled.
> 
> FWIW, it does compile.
> 
> > Emitting a message on failure to initialize IMA at late_initcall is good, but
> > the attestation service won't know.  Could you somehow differentiate between the
> > late_initcall and late_initcall_sync boot_aggregate records?
> 
> Are you thinking "boot_aggregate" and "boot_aggregate_late" or similar 
> as the "filename" on the entries, just so it's clear when we did the 
> init in the log, or something else?

Perfect!

Mimi

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Mimi Zohar @ 2026-04-23 18:01 UTC (permalink / raw)
  To: Yeoreum Yun, Jonathan McDowell
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
	oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
	will, noodles, sebastianene
In-Reply-To: <aeotq8nPVu4wvEx5@e129823.arm.com>

On Thu, 2026-04-23 at 15:33 +0100, Yeoreum Yun wrote:
> Hi Jonathan,
> 
> > * # Be careful, this email looks suspicious; * Out of Character: The sender is exhibiting a significant deviation from their usual behavior, this may indicate that their account has been compromised. Be extra cautious before opening links or attachments. *
> > On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
> > > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > > 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?
> > > > > > > > > > > 
> > > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > > > > > 
> > > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > > > > it by caller of ima_init().
> > > > > > > > > > 
> > > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > > > > > to anything else.  Just call ima_init() a second time.
> > > > > > > > > 
> > > > > > > > > I’m not fully convinced this is sufficient.
> > > > > > > > > 
> > > > > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > > > > > 
> > > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > > > > > backported.  No other changes should be included in this patch.
> > > > > > > 
> > > > > > > Okay.
> > > > > > > 
> > > > > > > > > 
> > > > > > > > > I’d also like to ask again whether it is fine to call
> > > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > > > > logically questionable to do so when ima_init() has not completed.
> > > > > > > > 
> > > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > > > > is a TPM or goes into TPM-bypass mode.
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > > > > ima_init(), rather than inside ima_init() itself.
> > > > > > > > 
> > > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > > > > > and hide it here behind the late_initcall_sync change.
> > > > > > > 
> > > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > > > > get failed with "TPM-bypass mode".
> > > > > > > 
> > > > > > > I see then, I'll make a patch simpler then.
> > > > > > 
> > > > > > But I think in case of below situation:
> > > > > >  - late_initcall's first ima_init() is deferred.
> > > > > >  - late_initcall_sync try again but failed and try again with
> > > > > >    CONFIG_IMA_DEFAULT_HASH.
> > > > > > 
> > > > > > I would like to sustain init_ima_core to reduce the same code repeat
> > > > > > in late_initcall_sync.
> > > > > 
> > > > > I think what Mimi's proposing is:
> > > > > 
> > > > > If we're in late_initcall, and the TPM isn't available, return
> > > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > > 
> > > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > > return and nothing, or run through the entire flow, even if the TPM
> > > > > isn't unavailable.
> > > > > 
> > > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > > and b) for the sync mode, if we've already done the init at
> > > > > non-sync.
> > > > 
> > > > Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
> > > > should not be included in this patch.  Since Yeoreum is not hearing me, feel
> > > > free to post a patch.
> > > 
> > > I see. so what you need to is this only
> > > If it looks good to you. I'll send it at v3.
> > 
> > FWIW, I pulled the tpm_default_chip check out a level to account for the
> > extra init you mentioned, and have the following (completely untested or
> > compiled, but gives the approach):
> > 
> > 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_init.c b/security/integrity/ima/ima_init.c
> > index a2f34f2d8ad7..a60dfb8316d8 100644
> > --- a/security/integrity/ima/ima_init.c
> > +++ b/security/integrity/ima/ima_init.c
> > @@ -119,10 +119,6 @@ int __init ima_init(void)
> >  {
> >  	int rc;
> > -	ima_tpm_chip = tpm_default_chip();
> > -	if (!ima_tpm_chip)
> > -		pr_info("No TPM chip found, activating TPM-bypass!\n");
> > -
> >  	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
> >  	if (rc)
> >  		return rc;
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 1d6229b156fb..b60a85fa803a 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
> >  #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> > -static int __init init_ima(void)
> > +static int __init init_ima(bool sync)
> >  {
> >  	int error;
> > @@ -1247,6 +1247,19 @@ static int __init init_ima(void)
> >  		return 0;
> >  	}
> > +	/* If we found the TPM during our first attempt, nothing further to do */
> > +	if (sync && ima_tpm_chip)
> > +		return 0;
> > +
> > +	ima_tpm_chip = tpm_default_chip();
> > +	if (!ima_tpm_chip && !sync) {
> > +		pr_debug("TPM not available, will try later\n");
> > +		return -EPROBE_DEFER;
> > +	}
> > +
> > +	if (!ima_tpm_chip)
> > +		pr_info("No TPM chip found, activating TPM-bypass!\n");
> > +
> >  	ima_appraise_parse_cmdline();
> >  	ima_init_template_list();
> >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > @@ -1274,6 +1287,16 @@ static int __init init_ima(void)
> >  	return error;
> >  }
> > +static int __init init_ima_late(void)
> > +{
> > +	return init_ima(false);
> > +}
> > +
> > +static int __init init_ima_late_sync(void)
> > +{
> > +	return init_ima(true);
> > +}
> > +
> >  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),
> > @@ -1319,6 +1342,7 @@ DEFINE_LSM(ima) = {
> >  	.init = init_ima_lsm,
> >  	.order = LSM_ORDER_LAST,
> >  	.blobs = &ima_blob_sizes,
> > -	/* Start IMA after the TPM is available */
> > -	.initcall_late = init_ima,
> > +	/* Ensure we start IMA after the TPM is available */
> > +	.initcall_late = init_ima_late,
> > +	.initcall_late_sync = init_ima_late_sync,
> >  };
> > 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);
> 
> I'm fine this. but are we talking about "ima_init()" not "init_ima()"?

Having two functions named ima_init() and init_ima() is really confusing.  At
least with this patch, init_ima() will be replaced with init_ima_late() and
init_ima_sync().

> Because of this, I've fixuated and make a long stupid speaking myself.

The commit 0e0546eabcd6 ("firmware: arm_ffa: Change initcall level of ffa_init()
to rootfs_initcall") patch description was really well written.  I'm really sad
that it needs to be reverted.

The TPM not being initialized before IMA, has been an issue for a really long
time.  Hopefully this patch will safely fix it, not only for you, but for others
as well.

> 
> If this seems good to Mimi, I don't care who send it.
> But If you're going to send this, could you includes 2 and 3 too?

Once this patch is ready, we can create a topic branch to coordinate upstreaming
the remaining patches.

thanks!

Mimi


^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-23 18:13 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Jonathan McDowell, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
	roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, jgg,
	sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will, noodles, sebastianene
In-Reply-To: <e4e242ae5533d5762a3647186a178764881bf9ff.camel@linux.ibm.com>

> On Thu, 2026-04-23 at 15:33 +0100, Yeoreum Yun wrote:
> > Hi Jonathan,
> >
> > > * # Be careful, this email looks suspicious; * Out of Character: The sender is exhibiting a significant deviation from their usual behavior, this may indicate that their account has been compromised. Be extra cautious before opening links or attachments. *
> > > On Thu, Apr 23, 2026 at 02:55:14PM +0100, Yeoreum Yun wrote:
> > > > > On Thu, 2026-04-23 at 13:53 +0100, Jonathan McDowell wrote:
> > > > > > On Thu, Apr 23, 2026 at 01:34:13PM +0100, Yeoreum Yun wrote:
> > > > > > > > > On Thu, 2026-04-23 at 06:55 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > On Wed, 2026-04-22 at 20:41 +0100, Yeoreum Yun wrote:
> > > > > > > > > > > > > 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?
> > > > > > > > > > > >
> > > > > > > > > > > > Also, if we handle in ima_init() only, but it failed with other reason,
> > > > > > > > > > > > we shouldn't call again ima_init() in the late_initcall_sync.
> > > > > > > > > > > >
> > > > > > > > > > > > To handle this, It wouldn't do in the ima_init() but we need to handle
> > > > > > > > > > > > it by caller of ima_init().
> > > > > > > > > > >
> > > > > > > > > > > Only tpm_default_chip() is being called to set the ima_tpm_chip.  On failure,
> > > > > > > > > > > instead of going into TPM-bypass mode, return immediately.  There are no calls
> > > > > > > > > > > to anything else.  Just call ima_init() a second time.
> > > > > > > > > >
> > > > > > > > > > I’m not fully convinced this is sufficient.
> > > > > > > > > >
> > > > > > > > > > What I meant is the case where ima_init() fails due to other
> > > > > > > > > > initialisation steps, not only tpm_default_chip() (e.g. ima_fs_init()).
> > > > > > > > >
> > > > > > > > > The purpose of THIS patch is to add late_initcall_sync, when the TPM is not
> > > > > > > > > available at late_initcall.  This would be classified as a bug fix and would be
> > > > > > > > > backported.  No other changes should be included in this patch.
> > > > > > > >
> > > > > > > > Okay.
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I’d also like to ask again whether it is fine to call
> > > > > > > > > > ima_update_policy_flags() and keep the notifier registered in the
> > > > > > > > > > deferred TPM case. While this may be functionally acceptable, it seems
> > > > > > > > > > logically questionable to do so when ima_init() has not completed.
> > > > > > > > >
> > > > > > > > > Other than extending the TPM, IMA should behave exactly the same whether there
> > > > > > > > > is a TPM or goes into TPM-bypass mode.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > There is also a possibility that a deferred case ultimately fails (e.g.
> > > > > > > > > > deferred at late_initcall, but then failing at late_initcall_sync
> > > > > > > > > > for another reason, even while entering TPM bypass mode). In that case,
> > > > > > > > > > it seems more appropriate to handle this state in the caller of
> > > > > > > > > > ima_init(), rather than inside ima_init() itself.
> > > > > > > > >
> > > > > > > > > If the TPM isn't found at late_initcall_sync(), then IMA should go into TPM-
> > > > > > > > > bypass mode.  Please don't make any other changes to the existing IMA behavior
> > > > > > > > > and hide it here behind the late_initcall_sync change.
> > > > > > > >
> > > > > > > > Okay. you're talking called ima_update_policy_flags() at late_initcall
> > > > > > > > wouldn't be not a problem even in case of late_initcall_sync's ima_init()
> > > > > > > > get failed with "TPM-bypass mode".
> > > > > > > >
> > > > > > > > I see then, I'll make a patch simpler then.
> > > > > > >
> > > > > > > But I think in case of below situation:
> > > > > > >  - late_initcall's first ima_init() is deferred.
> > > > > > >  - late_initcall_sync try again but failed and try again with
> > > > > > >    CONFIG_IMA_DEFAULT_HASH.
> > > > > > >
> > > > > > > I would like to sustain init_ima_core to reduce the same code repeat
> > > > > > > in late_initcall_sync.
> > > > > >
> > > > > > I think what Mimi's proposing is:
> > > > > >
> > > > > > If we're in late_initcall, and the TPM isn't available, return
> > > > > > immediately with an error (the EPROBE_DEFER?), don't do any init.
> > > > > >
> > > > > > If we're in late_initcall_sync, either we're already initialised, so do
> > > > > > return and nothing, or run through the entire flow, even if the TPM
> > > > > > isn't unavailable.
> > > > > >
> > > > > > So ima_init() just needs to know a) if it's in the sync or non-sync mode
> > > > > > and b) for the sync mode, if we've already done the init at
> > > > > > non-sync.
> > > > >
> > > > > Thanks, Jonathan.  That is exactly what I'm suggesting.  Any other changes
> > > > > should not be included in this patch.  Since Yeoreum is not hearing me, feel
> > > > > free to post a patch.
> > > >
> > > > I see. so what you need to is this only
> > > > If it looks good to you. I'll send it at v3.
> > >
> > > FWIW, I pulled the tpm_default_chip check out a level to account for the
> > > extra init you mentioned, and have the following (completely untested or
> > > compiled, but gives the approach):
> > >
> > > 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_init.c b/security/integrity/ima/ima_init.c
> > > index a2f34f2d8ad7..a60dfb8316d8 100644
> > > --- a/security/integrity/ima/ima_init.c
> > > +++ b/security/integrity/ima/ima_init.c
> > > @@ -119,10 +119,6 @@ int __init ima_init(void)
> > >  {
> > >  	int rc;
> > > -	ima_tpm_chip = tpm_default_chip();
> > > -	if (!ima_tpm_chip)
> > > -		pr_info("No TPM chip found, activating TPM-bypass!\n");
> > > -
> > >  	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
> > >  	if (rc)
> > >  		return rc;
> > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > index 1d6229b156fb..b60a85fa803a 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -1237,7 +1237,7 @@ static int ima_kernel_module_request(char *kmod_name)
> > >  #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> > > -static int __init init_ima(void)
> > > +static int __init init_ima(bool sync)
> > >  {
> > >  	int error;
> > > @@ -1247,6 +1247,19 @@ static int __init init_ima(void)
> > >  		return 0;
> > >  	}
> > > +	/* If we found the TPM during our first attempt, nothing further to do */
> > > +	if (sync && ima_tpm_chip)
> > > +		return 0;
> > > +
> > > +	ima_tpm_chip = tpm_default_chip();
> > > +	if (!ima_tpm_chip && !sync) {
> > > +		pr_debug("TPM not available, will try later\n");
> > > +		return -EPROBE_DEFER;
> > > +	}
> > > +
> > > +	if (!ima_tpm_chip)
> > > +		pr_info("No TPM chip found, activating TPM-bypass!\n");
> > > +
> > >  	ima_appraise_parse_cmdline();
> > >  	ima_init_template_list();
> > >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > > @@ -1274,6 +1287,16 @@ static int __init init_ima(void)
> > >  	return error;
> > >  }
> > > +static int __init init_ima_late(void)
> > > +{
> > > +	return init_ima(false);
> > > +}
> > > +
> > > +static int __init init_ima_late_sync(void)
> > > +{
> > > +	return init_ima(true);
> > > +}
> > > +
> > >  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),
> > > @@ -1319,6 +1342,7 @@ DEFINE_LSM(ima) = {
> > >  	.init = init_ima_lsm,
> > >  	.order = LSM_ORDER_LAST,
> > >  	.blobs = &ima_blob_sizes,
> > > -	/* Start IMA after the TPM is available */
> > > -	.initcall_late = init_ima,
> > > +	/* Ensure we start IMA after the TPM is available */
> > > +	.initcall_late = init_ima_late,
> > > +	.initcall_late_sync = init_ima_late_sync,
> > >  };
> > > 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);
> >
> > I'm fine this. but are we talking about "ima_init()" not "init_ima()"?
>
> Having two functions named ima_init() and init_ima() is really confusing.  At
> least with this patch, init_ima() will be replaced with init_ima_late() and
> init_ima_sync().
>
> > Because of this, I've fixuated and make a long stupid speaking myself.
>
> The commit 0e0546eabcd6 ("firmware: arm_ffa: Change initcall level of ffa_init()
> to rootfs_initcall") patch description was really well written.  I'm really sad
> that it needs to be reverted.
>
> The TPM not being initialized before IMA, has been an issue for a really long
> time.  Hopefully this patch will safely fix it, not only for you, but for others
> as well.
>
> >
> > If this seems good to Mimi, I don't care who send it.
> > But If you're going to send this, could you includes 2 and 3 too?
>
> Once this patch is ready, we can create a topic branch to coordinate upstreaming
> the remaining patches.

Sounds good. Once the patch is posted, I’ll review it as well.
Sorry again for the noise, and thanks for your patience ;)


--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* [PATCH v3] evm: terminate and bound the evm_xattrs read buffer
From: Pengpeng Hou @ 2026-04-23 15:30 UTC (permalink / raw)
  To: Mimi Zohar, Roberto Sassu
  Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge Hallyn, linux-integrity,
	linux-security-module, linux-kernel, pengpeng
In-Reply-To: <20260417223004.1-evm-xattrs-v2-pengpeng@iscas.ac.cn>

evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
enabled xattrs, and then passes strlen(temp) to
simple_read_from_buffer(). When no configured xattrs are enabled, the
fill loop stores nothing and temp[0] remains uninitialized, so strlen()
reads beyond initialized memory.

Explicitly terminate the buffer after allocation, use snprintf() for
each formatted line, and pass the accumulated length, without risk of
truncation, to simple_read_from_buffer().

Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v2:
- adjust the changelog wording to mention why the accumulated length is
  safe
- add the blank line after the allocation error path
- add a comment explaining why snprintf() cannot truncate in the fill loop

diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index acd840461902..4baf5e23bc97 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -127,8 +127,8 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
 			       size_t count, loff_t *ppos)
 {
 	char *temp;
-	int offset = 0;
-	ssize_t rc, size = 0;
+	size_t offset = 0, size = 0;
+	ssize_t rc;
 	struct xattr_list *xattr;
 
 	if (*ppos != 0)
@@ -151,16 +151,22 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
 		return -ENOMEM;
 	}
 
+	temp[size] = '\0';
+
+	/*
+	 * No truncation possible: size is computed over the same enabled
+	 * xattrs under xattr_list_mutex, so offset never exceeds size.
+	 */
 	list_for_each_entry(xattr, &evm_config_xattrnames, list) {
 		if (!xattr->enabled)
 			continue;
 
-		sprintf(temp + offset, "%s\n", xattr->name);
-		offset += strlen(xattr->name) + 1;
+		offset += snprintf(temp + offset, size + 1 - offset, "%s\n",
+				   xattr->name);
 	}
 
 	mutex_unlock(&xattr_list_mutex);
-	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+	rc = simple_read_from_buffer(buf, count, ppos, temp, offset);
 
 	kfree(temp);
 
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-24  1:27 UTC (permalink / raw)
  To: Mimi Zohar, roberto.sassu
  Cc: Yeoreum Yun, Jonathan McDowell, linux-security-module,
	linux-kernel, linux-integrity, linux-arm-kernel, kvmarm, jmorris,
	serge, dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla,
	maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will, noodles, sebastianene
In-Reply-To: <aephL3MzYoqFGaT5@e129823.arm.com>

On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
>
> Sounds good. Once the patch is posted, I’ll review it as well.
> Sorry again for the noise, and thanks for your patience ;)

My apologies for not getting a chance to look at this patchset sooner.

This seems like an obvious, perhaps even stupid, question, but I have
to ask: if IMA can be properly initialized via late_initcall_sync(),
why not simply do the initialization in late_initcall_sync() and drop
the late_initcall() initialization?

Does any IMA functionality suffer if initialization waits until
late_initcall_sync()?  If so, it seems non-critical if waiting until
_sync() is acceptable, as it appears in these patches/comments.

--
paul-moore.com

^ permalink raw reply

* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-24  5:57 UTC (permalink / raw)
  To: Paul Moore
  Cc: Mimi Zohar, roberto.sassu, Jonathan McDowell,
	linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
	eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
	suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
	sebastianene
In-Reply-To: <CAHC9VhRQWHEWQ5NzOPiu8jtYv6UsRm8WVS4fd74AbkOcAd4y_g@mail.gmail.com>

Hi Paul,

> On Thu, Apr 23, 2026 at 2:13 PM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> >
> > Sounds good. Once the patch is posted, I’ll review it as well.
> > Sorry again for the noise, and thanks for your patience ;)
>
> My apologies for not getting a chance to look at this patchset sooner.
>
> This seems like an obvious, perhaps even stupid, question, but I have
> to ask: if IMA can be properly initialized via late_initcall_sync(),
> why not simply do the initialization in late_initcall_sync() and drop
> the late_initcall() initialization?
>
> Does any IMA functionality suffer if initialization waits until
> late_initcall_sync()?  If so, it seems non-critical if waiting until
> _sync() is acceptable, as it appears in these patches/comments.

This is the way first patch did, and here is some discussion for this
(Might you have seen, but in case of you missed):
  - https://lore.kernel.org/all/a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com/

Thanks.

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [PATCH v3] evm: terminate and bound the evm_xattrs read buffer
From: Roberto Sassu @ 2026-04-24  8:13 UTC (permalink / raw)
  To: Pengpeng Hou, Mimi Zohar
  Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge Hallyn, linux-integrity,
	linux-security-module, linux-kernel
In-Reply-To: <20260424070101.1-evm-v3-pengpeng@iscas.ac.cn>

On Thu, 2026-04-23 at 23:30 +0800, Pengpeng Hou wrote:
> evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
> enabled xattrs, and then passes strlen(temp) to
> simple_read_from_buffer(). When no configured xattrs are enabled, the
> fill loop stores nothing and temp[0] remains uninitialized, so strlen()
> reads beyond initialized memory.
> 
> Explicitly terminate the buffer after allocation, use snprintf() for
> each formatted line, and pass the accumulated length, without risk of
> truncation, to simple_read_from_buffer().
> 
> Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

Thanks

Roberto

> ---
> Changes since v2:
> - adjust the changelog wording to mention why the accumulated length is
>   safe
> - add the blank line after the allocation error path
> - add a comment explaining why snprintf() cannot truncate in the fill loop
> 
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index acd840461902..4baf5e23bc97 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -127,8 +127,8 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
>  			       size_t count, loff_t *ppos)
>  {
>  	char *temp;
> -	int offset = 0;
> -	ssize_t rc, size = 0;
> +	size_t offset = 0, size = 0;
> +	ssize_t rc;
>  	struct xattr_list *xattr;
>  
>  	if (*ppos != 0)
> @@ -151,16 +151,22 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
>  		return -ENOMEM;
>  	}
>  
> +	temp[size] = '\0';
> +
> +	/*
> +	 * No truncation possible: size is computed over the same enabled
> +	 * xattrs under xattr_list_mutex, so offset never exceeds size.
> +	 */
>  	list_for_each_entry(xattr, &evm_config_xattrnames, list) {
>  		if (!xattr->enabled)
>  			continue;
>  
> -		sprintf(temp + offset, "%s\n", xattr->name);
> -		offset += strlen(xattr->name) + 1;
> +		offset += snprintf(temp + offset, size + 1 - offset, "%s\n",
> +				   xattr->name);
>  	}
>  
>  	mutex_unlock(&xattr_list_mutex);
> -	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> +	rc = simple_read_from_buffer(buf, count, ppos, temp, offset);
>  
>  	kfree(temp);
>  


^ permalink raw reply

* linux-next changes on my tree
From: Jarkko Sakkinen @ 2026-04-24  9:05 UTC (permalink / raw)
  To: linux-integrity, keyrings; +Cc: Paul Moore, David Howells, Linus Torvalds

Hi,

First I want to apologize for messing up with PRs for 7.1 [1]. I've
taken steps to sanitize my branching and the changes are now reflected
to -next:

commit 3ee56e5cf8afb875beab59e42e0b69cdb31050ab
Merge: e77ce03e10ba 4fb4ce024d06
Author: Mark Brown <broonie@kernel.org>
Date:   Thu Apr 23 13:26:01 2026 +0100

    Merge branch 'for-next-keys' of
    https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

commit e77ce03e10baf7b84d169efcaa497861dc9a07bf
Merge: c6391bbee383 949692da7211
Author: Mark Brown <broonie@kernel.org>
Date:   Thu Apr 23 13:26:00 2026 +0100

    Merge branch 'for-next-tpm' of
    https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

I'm now travelling right now in Budapest but will be back in Finland
tomorrow, and will retry my PRs then.

Paul, I promised to look into [2] this week but unfortunately it is
delayed +1 but is on my TODO list for next week along with couple
of other kernel review/testing backlog items :-) Hope you understand.

[1] https://lore.kernel.org/linux-integrity/aeVSbVIFaCDRXf7C@kernel.org/T/#t
    Yes, and I know late cc-tagging was also additional reason for
    NAK, can't blame branching on that. I did also execute stupid shit
    to add in.
[2] https://lore.kernel.org/all/ad--R8KaGgucLxmH@kernel.org/

BR, Jarkko

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox