| Message ID | 20241118145732.1258631-1-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] ima: Suspend PCR extends and log appends when rebooting | expand |
On Mon, 2024-11-18 at 09:57 -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > To avoid the following types of error messages due to a failure by the TPM > driver to use the TPM, suspend TPM PCR extensions and the appending of > entries to the IMA log once IMA's reboot notifier has been called. This > avoids trying to use the TPM after the TPM subsystem has been shut down. > > [111707.685315][ T1] ima: Error Communicating to TPM chip, result: -19 > [111707.685960][ T1] ima: Error Communicating to TPM chip, result: -19 > > Synchronization with the ima_extend_list_mutex to set > ima_measurements_suspended ensures that the TPM subsystem is not shut down > when IMA holds the mutex while appending to the log and extending the PCR. > The alternative of reading the system_state variable would not provide this > guarantee. > > This error could be observed on a ppc64 machine running SuSE Linux where > processes are still accessing files after devices have been shut down. > > Suspending the IMA log and PCR extensions shortly before reboot does not > seem to open a significant measurement gap since neither TPM quoting would > work for attestation nor that new log entries could be written to anywhere > after devices have been shut down. However, there's a time window between > the invocation of the reboot notifier and the shutdown of devices. This > includes all subsequently invoked reboot notifiers as well as > kernel_restart_prepare() where __usermodehelper_disable() waits for all > running_helpers to exit. During this time window IMA could now miss log > entries even though attestation would still work. The reboot of the system > shortly after may make this small gap insignificant. > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Thank you for updating the patch description and the comment below. The patch is now queued in next-integrity-testing. Mimi > --- [...] > int pcr) > @@ -168,6 +175,18 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > int result = 0, tpmresult = 0; > > mutex_lock(&ima_extend_list_mutex); > + > + /* > + * Avoid appending to the measurement log when the TPM subsystem has > + * been shut down while preparing for system reboot. > + */ > + if (ima_measurements_suspended) { > + audit_cause = "measurements_suspended"; > + audit_info = 0; > + result = -ENODEV; > + goto out; > + } > + > if (!violation && !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) { > if (ima_lookup_digest_entry(digest, entry->pcr)) { > audit_cause = "hash_exists"; > @@ -211,6 +230,31 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry) > return result; > }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 3c323ca213d4..3f1a82b7cd71 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -278,6 +278,7 @@ unsigned long ima_get_binary_runtime_size(void); int ima_init_template(void); void ima_init_template_list(void); int __init ima_init_digests(void); +void __init ima_init_reboot_notifier(void); int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, void *lsm_data); diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 4e208239a40e..a2f34f2d8ad7 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -152,6 +152,8 @@ int __init ima_init(void) ima_init_key_queue(); + ima_init_reboot_notifier(); + ima_measure_critical_data("kernel_info", "kernel_version", UTS_RELEASE, strlen(UTS_RELEASE), false, NULL, 0); diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index 532da87ce519..83d53824aa98 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -16,6 +16,7 @@ */ #include <linux/rculist.h> +#include <linux/reboot.h> #include <linux/slab.h> #include "ima.h" @@ -44,6 +45,12 @@ struct ima_h_table ima_htable = { */ static DEFINE_MUTEX(ima_extend_list_mutex); +/* + * Used internally by the kernel to suspend measurements. + * Protected by ima_extend_list_mutex. + */ +static bool ima_measurements_suspended; + /* lookup up the digest value in the hash table, and return the entry */ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value, int pcr) @@ -168,6 +175,18 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, int result = 0, tpmresult = 0; mutex_lock(&ima_extend_list_mutex); + + /* + * Avoid appending to the measurement log when the TPM subsystem has + * been shut down while preparing for system reboot. + */ + if (ima_measurements_suspended) { + audit_cause = "measurements_suspended"; + audit_info = 0; + result = -ENODEV; + goto out; + } + if (!violation && !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) { if (ima_lookup_digest_entry(digest, entry->pcr)) { audit_cause = "hash_exists"; @@ -211,6 +230,31 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry) return result; } +static void ima_measurements_suspend(void) +{ + mutex_lock(&ima_extend_list_mutex); + ima_measurements_suspended = true; + mutex_unlock(&ima_extend_list_mutex); +} + +static int ima_reboot_notifier(struct notifier_block *nb, + unsigned long action, + void *data) +{ + ima_measurements_suspend(); + + return NOTIFY_DONE; +} + +static struct notifier_block ima_reboot_nb = { + .notifier_call = ima_reboot_notifier, +}; + +void __init ima_init_reboot_notifier(void) +{ + register_reboot_notifier(&ima_reboot_nb); +} + int __init ima_init_digests(void) { u16 digest_size;