On Mon, 2024-11-18 at 09:57 -0500, Stefan Berger wrote:
> From: Stefan Berger <[email protected]>
> 
> 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 <[email protected]>
> Signed-off-by: Stefan Berger <[email protected]>

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;
>  }


Reply via email to