[PATCH] ima: Suspend PCR extends and log appends when rebooting

[Mimi Zohar]

On Fri, 2024-10-11 at 11:05 -0400, Stefan Berger wrote:


> To avoid the following types of error messages from the TPM driver, suspend
> PCR extends once the 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.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu at linux.microsoft.com>
> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
> ---

No need to send a separate post about the patch.  Any comments not part of the
patch description belong here.

>  security/integrity/ima/ima.h       |  1 +
>  security/integrity/ima/ima_init.c  | 16 ++++++++++++++++
>  security/integrity/ima/ima_queue.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c51e24d24d1e..d3f46232b8b9 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -274,6 +274,7 @@ bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
>  int ima_restore_measurement_entry(struct ima_template_entry *entry);
>  int ima_restore_measurement_list(loff_t bufsize, void *buf);
>  int ima_measurements_show(struct seq_file *m, void *v);
> +void ima_measurements_suspend(void);
>  unsigned long ima_get_binary_runtime_size(void);
>  int ima_init_template(void);
>  void ima_init_template_list(void);
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 4e208239a40e..ac630d8d3049 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/err.h>
>  #include <linux/ima.h>
> +#include <linux/reboot.h>
>  #include <generated/utsrelease.h>
>  
>  #include "ima.h"
> @@ -115,6 +116,19 @@ void __init ima_load_x509(void)
>  }
>  #endif
>  
> +static int ima_reboot_notify(struct notifier_block *nb,
> +                      unsigned long action,
> +                      void *data)
> +{
> +	ima_measurements_suspend();
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block ima_reboot_notifier = {
> +	.notifier_call = ima_reboot_notify,
> +};
> +
>  int __init ima_init(void)
>  {
>  	int rc;
> @@ -152,6 +166,8 @@ int __init ima_init(void)
>  
>  	ima_init_key_queue();
>  
> +	register_reboot_notifier(&ima_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..b8613b6c1a39 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -44,6 +44,12 @@ struct ima_h_table ima_htable = {
>   */
>  static DEFINE_MUTEX(ima_extend_list_mutex);
>  
> +/*
> + * Used internally by the kernel to suspend-resume ima measurements.
> + * Protected by ima_extend_list_mutex.
> + */

Tushar's original patch incorrectly provided both a function to suspend and
resume appending records to the IMA measurement list and extending the TPM PCR.
This patch only suspends appending records to the IMA measurement list and
extending the TPM PCR.  Please replace the word "suspend-resume" in the comment
above with just "suspend".

> +static bool suspend_ima_measurements;
> +
>  /* 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)
> @@ -148,6 +154,13 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
>  	return result;
>  }
>  
> +void ima_measurements_suspend(void)
> +{
> +	mutex_lock(&ima_extend_list_mutex);
> +	suspend_ima_measurements = true;
> +	mutex_unlock(&ima_extend_list_mutex);
> +}
> +
>  /*
>   * Add template entry to the measurement list and hash table, and
>   * extend the pcr.
> @@ -176,6 +189,20 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  		}
>  	}
>  
> +	/*
> +	 * suspend_ima_measurements will be set if the system is
> +	 * undergoing kexec soft boot to a new kernel.
> +	 * suspending measurements in this short window ensures the
> +	 * consistency of the IMA measurement list during copying
> +	 * of the kexec buffer.
> +	 */

Tushar's patch did not register a reboot notifier.  Please update the above
comment accordingly.

> +	if (suspend_ima_measurements) {
> +		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) {

thanks,

Mimi