On 10/27/23 07:56, Mimi Zohar wrote:
Thanks. I described the reasons in the code comment below, because I thought it is important enough to stay with the code to give context. But I can move it from the code comment to a patch description.Hi Tushar, On Thu, 2023-10-05 at 11:26 -0700, Tushar Sugandhi wrote:The window between kexec 'load' and 'execute' could be arbitrarily long. Even with the large chunk of memory allocated at kexec 'load', it may run out which would result in missing events in IMA log after the system soft reboots to the new Kernel. This would result in IMA measurements getting out of sync with the TPM PCR quotes which would result in remote attestation failing for that system. There is currently no way for the new Kernel to know if the IMA log TPM PCR quote out of sync problem is because of the missing measurements during kexec. Define two new IMA events, 'kexec_load' and 'kexec_execute', to be measured at kexec 'load' and 'execute' respectively. IMA measures 'boot_aggregate' as the first event when the system boots - either cold boot or kexec soft boot. In case when the system goes through multiple soft reboots, the number of 'boot_aggregate' events in IMA log corresponds to total number of boots (cold boot plus multiple kexec soft reboots). With this change, there would be additional 'kexec_load' and 'kexec_execute' events in between the two 'boot_aggregate' events. In rare cases, when the system runs out of memory during kexec soft reboot, 'kexec_execute' won't be copied since its one of the very last event measured just before kexec soft reboot. The absence of the event 'kexec_execute' in between the two boot_aggregate' events would signal the attestation service that the IMA log on the system is out of sync with TPM PCR quotes and the system needs to be cold booted for the remote attestation to succeed again. Signed-off-by: Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>Adding a new type of critical data is fine. The patch description should describe the reason for it. Please update the Subject line and the patch description accordingly.
--- security/integrity/ima/ima_kexec.c | 35 +++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index 6cd5f46a7208..0f9c424fe808 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -17,6 +17,8 @@ #include "ima.h"#ifdef CONFIG_IMA_KEXEC+#define IMA_KEXEC_EVENT_LEN 128 + struct seq_file ima_kexec_file; struct ima_kexec_hdr ima_khdr; static void *ima_kexec_buffer; @@ -34,6 +36,8 @@ void ima_clear_kexec_file(void)static int ima_alloc_kexec_buf(size_t kexec_segment_size){ + char ima_kexec_event[IMA_KEXEC_EVENT_LEN]; + if ((kexec_segment_size == 0) || (kexec_segment_size == ULONG_MAX) || ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) { @@ -64,6 +68,12 @@ static int ima_alloc_kexec_buf(size_t kexec_segment_size) memset(&ima_khdr, 0, sizeof(ima_khdr)); ima_khdr.version = 1;+ scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,+ "kexec_segment_size=%lu;", kexec_segment_size); + + ima_measure_critical_data("ima_kexec", "kexec_load", ima_kexec_event, + strlen(ima_kexec_event), false, NULL, 0); + return 0; }@@ -198,6 +208,7 @@ void ima_add_kexec_buffer(struct kimage *image)static int ima_update_kexec_buffer(struct notifier_block *self, unsigned long action, void *data) { + char ima_kexec_event[IMA_KEXEC_EVENT_LEN]; void *buf = NULL; size_t buf_size; bool resume = false; @@ -213,9 +224,31 @@ static int ima_update_kexec_buffer(struct notifier_block *self, return NOTIFY_OK; }+ buf_size = ima_get_binary_runtime_size();+ scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN, + "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;", + kexec_segment_size, buf_size); + + /* + * This is one of the very last events measured by IMA before kexec + * soft rebooting into the new Kernal. + * This event can be used as a marker after the system soft reboots + * to the new Kernel to check if there was sufficient memory allocated + * at kexec 'load' to capture the events measured between the window + * of kexec 'load' and 'execute'. + * This event needs to be present in the IMA log, in between the two + * 'boot_aggregate' events that are logged for the previous boot and + * the current soft reboot. If it is not present after the system soft + * reboots into the new Kernel, it would mean the IMA log is not + * consistent with the TPM PCR quotes, and the system needs to be + * cold-booted for the attestation to succeed again. + */Comments like this don't belong in the code. Please refer to section "8) Commenting" of Documentation/process/coding-style.rst.
Will do. ~Tushar
+ ima_measure_critical_data("ima_kexec", "kexec_execute", + ima_kexec_event, strlen(ima_kexec_event), + false, NULL, 0); + ima_measurements_suspend();- buf_size = ima_get_binary_runtime_size();ret = ima_dump_measurement_list(&buf_size, &buf, kexec_segment_size);
_______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
