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. > + 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); > -- thanks, Mimi _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec