On Mon, 2024-11-18 at 09:57 -0500, Stefan Berger wrote:
> From: Stefan Berger <stefanb@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
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;
> }
