[PATCH v4 5/7] ima: suspend measurements during buffer copy at kexec execute
Tushar Sugandhi
tusharsu at linux.microsoft.com
Tue Jan 23 12:55:10 PST 2024
On 1/23/24 10:18, Stefan Berger wrote:
>
>
> On 1/22/24 13:38, Tushar Sugandhi wrote:
>> New measurements added to the IMA log while the log is being copied
>> during the kexec 'execute' may not get copied over. This can cause the
>> measurement log to be out of sync with the IMA TPM PCR, which could
>
> It could be out of sync with any PCR that IMA extends, most often only
> the IMA TPM PCR.
>
Agreed. That's what I meant. But I can see my above wording could be
misleading.
I will reword it to "This can cause the measurement log to be out of
sync with the TPM PCRs that IMA extends, which could result in..."
>> result in breaking the integrity of the measurements after kexec soft
>> reboot.
>>
>> Implement and call the functions ima_measurements_suspend() and
>> ima_measurements_resume() from ima_update_kexec_buffer().
>>
>> Add a check in the ima_add_template_entry() function not to measure
>> events when 'suspend_ima_measurements' flag is set.
>>
>> This ensures the integrity of the IMA log while it is being copied over
>> to the new Kernel during kexec 'execute'.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu at linux.microsoft.com>
>
> Reviewed-by: Stefan Berger <stefanb at linux.ibm.com>
>
Thanks for the tag. I will add it to this patch in the next iteration of
the series.
Please let me know if you want it to be added to any other patches.
~Tushar
>> ---
>> security/integrity/ima/ima.h | 2 ++
>> security/integrity/ima/ima_kexec.c | 7 +++++++
>> security/integrity/ima/ima_queue.c | 32 ++++++++++++++++++++++++++++++
>> 3 files changed, 41 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index c29db699c996..49a6047dd8eb 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -161,6 +161,8 @@ 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);
>> +void ima_measurements_resume(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_kexec.c
>> b/security/integrity/ima/ima_kexec.c
>> index 25150bfc7129..f26b5b342d84 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -179,6 +179,7 @@ static int ima_update_kexec_buffer(struct
>> notifier_block *self,
>> void *buf = NULL;
>> size_t buf_size;
>> int ret = NOTIFY_OK;
>> + bool resume = false;
>> if (!kexec_in_progress) {
>> pr_info("%s: No kexec in progress.\n", __func__);
>> @@ -190,12 +191,15 @@ static int ima_update_kexec_buffer(struct
>> notifier_block *self,
>> return ret;
>> }
>> + ima_measurements_suspend();
>> +
>> ret = ima_dump_measurement_list(&buf_size, &buf,
>> kexec_segment_size);
>> if (!buf) {
>> pr_err("%s: Dump measurements failed. Error:%d\n",
>> __func__, ret);
>> + resume = true;
>> goto out;
>> }
>> memcpy(ima_kexec_buffer, buf, buf_size);
>> @@ -203,6 +207,9 @@ static int ima_update_kexec_buffer(struct
>> notifier_block *self,
>> kimage_unmap_segment(ima_kexec_buffer);
>> ima_kexec_buffer = NULL;
>> + if (resume)
>> + ima_measurements_resume();
>> +
>> return ret;
>> }
>> diff --git a/security/integrity/ima/ima_queue.c
>> b/security/integrity/ima/ima_queue.c
>> index 532da87ce519..5946a26a2849 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -44,6 +44,11 @@ struct ima_h_table ima_htable = {
>> */
>> static DEFINE_MUTEX(ima_extend_list_mutex);
>> +/*
>> + * Used internally by the kernel to suspend-resume ima measurements.
>> + */
>> +static atomic_t 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 +153,20 @@ 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);
>> + atomic_set(&suspend_ima_measurements, 1);
>> + mutex_unlock(&ima_extend_list_mutex);
>> +}
>> +
>> +void ima_measurements_resume(void)
>> +{
>> + mutex_lock(&ima_extend_list_mutex);
>> + atomic_set(&suspend_ima_measurements, 0);
>> + mutex_unlock(&ima_extend_list_mutex);
>> +}
>> +
>> /*
>> * Add template entry to the measurement list and hash table, and
>> * extend the pcr.
>> @@ -176,6 +195,19 @@ 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.
>> + */
>> + if (atomic_read(&suspend_ima_measurements)) {
>> + audit_cause = "measurements_suspended";
>> + audit_info = 0;
>> + goto out;
>> + }
>> +
>> result = ima_add_digest_entry(entry,
>> !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE));
>> if (result < 0) {
More information about the kexec
mailing list