Hello Paul, Le 01/07/2024 à 15:53, Paul Menzel a écrit : > Dear Romain, > > > Thank you for your patch. > > Am 01.07.24 um 15:38 schrieb Romain Naour: >> From: Romain Naour <romain.naour@xxxxxxx> >> >> It has been reported that on some plateforms the ima and evm > > platforms > >> initialization were performed too early during initcall initialization >> process and misses TPM chip detection [1] [2]. >> >> Indeed, ima may uses a TPM chip but requires to wait for bus >> interface (spi or i2c) and TPM driver initialization. >> >> [ 0.166261] ima: No TPM chip found, activating TPM-bypass! >> ... >> [ 0.166322] evm: Initialising EVM extended attributes: >> ... >> [ 0.182571] ti-sci 44083000.system-controller: ABI: 3.1 (firmware rev >> 0x0009 '9.2.4--v09.02.04 (Kool Koala)') >> [ 0.281540] omap_i2c 42120000.i2c: bus 0 rev0.12 at 400 kHz >> [ 0.282314] omap_i2c 2000000.i2c: bus 4 rev0.12 at 400 kHz >> [ 0.282972] omap_i2c 2010000.i2c: bus 5 rev0.12 at 400 kHz >> [ 0.335177] tpm_tis_i2c 2-002e: 2.0 TPM (device-id 0x1C, rev-id 22) >> [ 0.471596] omap_i2c 2020000.i2c: bus 2 rev0.12 at 100 kHz >> [ 0.472310] omap_i2c 2030000.i2c: bus 6 rev0.12 at 400 kHz >> [ 0.472951] omap_i2c 2040000.i2c: bus 3 rev0.12 at 100 kHz >> [ 0.473596] omap_i2c 2050000.i2c: bus 7 rev0.12 at 400 kHz >> [ 0.474274] omap_i2c 2060000.i2c: bus 1 rev0.12 at 100 kHz >> >> The ima stack was expecting to start after the TPM device (hence the >> comment) using late_initcall() but fail to do so on such plateforms: > > platforms I'll fix, thanks! > >> >> late_initcall(init_ima); /* Start IMA after the TPM is available */ >> >> Using late_initcall_sync() variant allows to really wait for i2c >> initialization completion. >> >> [ 0.285986] omap_i2c 42120000.i2c: bus 0 rev0.12 at 400 kHz >> [ 0.286706] omap_i2c 2000000.i2c: bus 4 rev0.12 at 400 kHz >> [ 0.287382] omap_i2c 2010000.i2c: bus 5 rev0.12 at 400 kHz >> [ 0.331503] tpm_tis_i2c 2-002e: 2.0 TPM (device-id 0x1C, rev-id 22) >> [ 0.677185] omap_i2c 2020000.i2c: bus 2 rev0.12 at 100 kHz >> [ 0.677904] omap_i2c 2030000.i2c: bus 6 rev0.12 at 400 kHz >> [ 0.678557] omap_i2c 2040000.i2c: bus 3 rev0.12 at 100 kHz >> [ 0.679167] omap_i2c 2050000.i2c: bus 7 rev0.12 at 400 kHz >> [ 0.679792] omap_i2c 2060000.i2c: bus 1 rev0.12 at 100 kHz >> ... >> [ 3.062788] ima: Allocated hash algorithm: sha256 >> ... >> [ 3.318975] ima: No architecture policies found >> [ 3.323536] evm: Initialising EVM extended attributes: >> [ 3.328662] evm: security.selinux (disabled) >> [ 3.332919] evm: security.SMACK64 (disabled) >> [ 3.337177] evm: security.SMACK64EXEC (disabled) >> [ 3.341781] evm: security.SMACK64TRANSMUTE (disabled) >> [ 3.346819] evm: security.SMACK64MMAP (disabled) >> [ 3.351422] evm: security.apparmor (disabled) >> [ 3.355764] evm: security.ima >> [ 3.358721] evm: security.capability >> [ 3.362285] evm: HMAC attrs: 0x1 >> >> [1] >> https://lore.kernel.org/linux-integrity/9b98d912-ba78-402c-a5c8-154bef8794f7@xxxxxxxx/ >> [2] >> https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1375425/tda4vm-ima-vs-tpm-builtin-driver-boot-order >> >> Signed-off-by: Romain Naour <romain.naour@xxxxxxx> > > Should this get a Fixes: tag and be also applied to the stable series? The current behavior can be reproduced on any released kernel (at least since 6.1). But I'm not sure if it should be backported to stable kernels since it delays the ima/evm initialization at runtime. > >> --- >> security/integrity/evm/evm_main.c | 2 +- >> security/integrity/ima/ima_main.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/security/integrity/evm/evm_main.c >> b/security/integrity/evm/evm_main.c >> index 62fe66dd53ce..316f8d140825 100644 >> --- a/security/integrity/evm/evm_main.c >> +++ b/security/integrity/evm/evm_main.c >> @@ -1180,4 +1180,4 @@ DEFINE_LSM(evm) = { >> .blobs = &evm_blob_sizes, >> }; >> -late_initcall(init_evm); >> +late_initcall_sync(init_evm); /* Start EVM after IMA */ >> diff --git a/security/integrity/ima/ima_main.c >> b/security/integrity/ima/ima_main.c >> index f04f43af651c..0aa7cd9aabfa 100644 >> --- a/security/integrity/ima/ima_main.c >> +++ b/security/integrity/ima/ima_main.c >> @@ -1220,4 +1220,4 @@ DEFINE_LSM(ima) = { >> .blobs = &ima_blob_sizes, >> }; >> -late_initcall(init_ima); /* Start IMA after the TPM is available */ >> +late_initcall_sync(init_ima); /* Start IMA after the TPM is available */ > > Looks good to me. Thanks for the review! Best regards, Romain > > > Kind regards, > > Paul
