On Tue Nov 12, 2024 at 6:52 PM EET, Stefan Berger wrote:
> 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
>
> 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 in
> 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 <tusha...@linux.microsoft.com>
> Signed-off-by: Stefan Berger <stef...@linux.ibm.com>
>
> ---
> v2:
> - followed Mimi's suggestions
>
> ---
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_init.c | 2 ++
> security/integrity/ima/ima_queue.c | 43 ++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+)
>
> 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..9b3c9587313f 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)
> @@ -176,6 +183,17 @@ int ima_add_template_entry(struct ima_template_entry
> *entry, int violation,
> }
> }
>
> + /*
> + * ima_measurements_suspended will be set before the TPM subsystem has
> + * been shut down.
> + */
> + if (ima_measurements_suspended) {
> + audit_cause = "measurements_suspended";
> + audit_info = 0;
> + result = -ENODEV;
> + goto out;
> + }
> +
> result = ima_add_digest_entry(entry,
> !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE));
> if (result < 0) {
> @@ -211,6 +229,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;
Reviewed-by: Jarkko Sakkinen <jar...@kernel.org>
BR, Jarkko
