[PATCH v8 0/7] ima: kexec: measure events between kexec load and execute

[PATCH v8 0/7] ima: kexec: measure events between kexec load and execute

[steven chen]

The current kernel behavior is IMA measurements snapshot is taken at
kexec 'load' and not at kexec 'execute'.  IMA log is then carried
over to the new kernel after kexec 'execute'.

New events can be measured during/after the IMA log snapshot at kexec 
'load' and before the system boots to the new kernel.  In this scenario,
the TPM PCRs are extended with these events, but they are not carried
over to the new kernel after kexec soft reboot since the snapshot is
already taken.  This results in mismatch between TPM PCR quotes and the
actual IMA measurements list after kexec soft reboot, which in turn
results in remote attestation failure.

To solve this problem - 
 - allocate the necessary buffer at kexec 'load' time,
 - populate the buffer with the IMA measurements at kexec 'execute' time, 
 - and measure two new IMA events 'kexec_load' and 'kexec_execute' as
   critical data to help detect missing events after kexec soft reboot.

The solution details include:
 - refactoring the existing code to allocate a buffer to hold IMA
   measurements at kexec 'load', and dump the measurements at kexec
   'execute'

 - kexec functionality for mapping the segments from the current kernel
   to the subsequent one, 

 - necessary changes to the kexec_file_load syscall, enabling it to call
   the ima functions,

 - registering a reboot notifier which gets called during kexec 'execute',

 - introducing a new Kconfig option to configure the extra memory to be
   allocated for passing IMA log from the current Kernel to the next,
   
 - introducing two new events to be measured by IMA during kexec, to
   help diagnose if the IMA log was copied fully or partially, from the
   current Kernel to the next,

 - excluding IMA segment while calculating and storing digest in function
   kexec_calculate_store_digests(), since IMA segment can be modified
   after the digest is computed during kexec 'load'.  This will ensure
   that the segment is not added to the 'purgatory_sha_regions', and thus
   not verified by verify_sha256_digest().

The changes proposed in this series ensure the integrity of the IMA
measurements is preserved across kexec soft reboots, thus significantly
improving the security of the kernel post kexec soft reboots.

There were previous attempts to fix this issue [1], [2], [3].  But they
were not merged into the mainline kernel.

We took inspiration from the past work [1] and [2] while working on this
patch series.

V4 of this series is available here[6] for reference.

V5 of this series is available here[7] for reference.

V6 of this series is available here[8] for reference.

References:
-----------

[1] [PATHC v2 5/9] ima: on soft reboot, save the measurement list
https://lore.kernel.org/lkml/1472596811-9596-6-git-send-email-zohar@linux.vnet.ibm.com/

[2] PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.
https://lkml.org/lkml/2016/8/16/577

[3] [PATCH 1/6] kexec_file: Add buffer hand-over support
https://lore.kernel.org/linuxppc-dev/1466473476-10104-6-git-send-email-bauerman@linux.vnet.ibm.com/T/

[4] [PATCH v2 0/7] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20231005182602.634615-1-tusharsu@linux.microsoft.com/

[5] [PATCH v3 0/7] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20231216010729.2904751-1-tusharsu@linux.microsoft.com/

[6] [PATCH v4 0/7] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20240122183804.3293904-1-tusharsu@linux.microsoft.com/

[7] [PATCH v5 0/8] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20240214153827.1087657-1-tusharsu@linux.microsoft.com/

[8] [PATCH v6 0/7] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20250124225547.22684-1-chenste@linux.microsoft.com/

[9] [PATCH v7 0/7] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20250203232033.64123-1-chenste@linux.microsoft.com/

Change Log v8:
 - Incorporated feedback from the community (Stefan Berger, Mimi Zohar) 
   on v7 of this series[9].
 - Rebased the patch series to mainline 6.14.0-rc1.
 - Verified all the patches are bisect-safe by booting into each
   patch and verifying multiple kexec 'load' operations work,
   and also verifying kexec soft reboot works, and IMA log gets
   carried over for each patch.

Change Log v7:
 - Incorporated feedback from the community (Stefan Berger, Tyler Hicks) 
   on v6 of this series[8].
 - Verified all the patches are bisect-safe by booting into each
   patch and verifying multiple kexec 'load' operations work,
   and also verifying kexec soft reboot works, and IMA log gets
   carried over for each patch.

Change Log v6:
 - Incorporated feedback from the community (Stefan Berger, Mimi Zohar,
   and Petr Tesařík) on v5 of this series[7].
 - Rebased the patch series to mainline 6.12.0.
 - Verified all the patches are bisect-safe by booting into each
   patch and verifying multiple kexec 'load' operations work,
   and also verifying kexec soft reboot works, and IMA log gets
   carried over for each patch.
 - Compared the memory size allocated with memory size of the entire 
   measurement record. If there is not enough memory, it will copy as many
   IMA measurement records as possible, and this situation will result
   in a failure of remote attestation.
 - [PATCH V5 6/8] was removed. Per petr comment on [PATCH V5 6/8], during
   the handover, other CPUs are taken offline (look for
   migrate_to_reboot_cpu() in kernel/kexec_core.c) and even the reboot CPU
   will be sufficiently shut down as not to be able to add any more
   measurements.

Change Log v5:
 - Incorporated feedback from the community (Stefan Berger and
   Mimi Zohar) on v4 of this series[6].
 - Rebased the patch series to mainline 6.8.0-rc1.
 - Verified all the patches are bisect-safe by booting into each
   patch and verifying multiple kexec 'load' operations work,
   and also verifying kexec soft reboot works, and IMA log gets
   carried over for each patch.
 - Divided the patch #4 in the v4 of the series[6] into two separate
   patches. One to setup the infrastructure/stub functions to prepare
   the IMA log copy from Kexec 'load' to 'execute', and another one
   to actually copy the log.
 - Updated the config description for IMA_KEXEC_EXTRA_MEMORY_KB
   to remove unnecessary references related to backwards compatibility.
 - Fixed a typo in log message/removed an extra line etc.
 - Updated patch descriptions as necessary.

Change Log v4:
 - Incorporated feedback from the community (Stefan Berger and
   Mimi Zohar) on v3 of this series[5].
 - Rearranged patches so that they remain bisect-safe i.e. the
   system can go through kexec soft reboot, and IMA log is carried
   over after each patch.
 - Verified all the patches are bisect-safe by booting into each
   patch and verifying kexec soft reboot works, and IMA log gets
   carried over.
 - Suspend-resume measurements is now a separate patch (patch #5)
   and all the relevant code is part of the same patch.
 - Excluding IMA segment from segment digest verification is now a
   separate patch. (patch #3).
 - Registering reboot notifier and functions related to move ima 
   log copy from kexec load to execute are now part of the same
   patch (patch #4) to protect bisect-safeness of the series.
 - Updated the title of patch #6 as per the feedback.
 - The default value of kexec extra memory for IMA measurements
   is set to half the PAGESIZE to maintain backwards compatibility.
 - Added number of IMA measurement records as part of 'kexec_load' 
   and 'kexec_execute' IMA critical data events.
 - Updated patch descriptions as necessary.

Change Log v3:
 - Incorporated feedback from the community (Stefan Berger and
   Mimi Zohar) on v2 of this series[4].
 - Renamed functions and removed extraneous checks and code comments.
 - Updated patch descriptions and titles as necessary.
 - Updated kexec_calculate_store_digests() in patch 2/7 to exclude ima
   segment from calculating and storing digest.
 - Updated patch 3/7 to use kmalloc_array instead of kmalloc and freed
   memory early to avoid potential memory leak.
 - Updated patch 6/7 to change Kconfig option IMA_KEXEC_EXTRA_PAGES to
   IMA_KEXEC_EXTRA_MEMORY_KB to allocate the memory in kb rather than
   in number of pages.
 - Optimized patch 7/7 not to free and alloc memory if the buffer size
   hasn't changed during multiple kexec 'load' operations.
 - Fixed a bug in patch 7/7 to measure multiple 'kexec_load' events even
   if buffer size hasn't changed.
 - Verified the patches are bisect-safe by compiling and booting into
   each patch individually.


Change Log v2:
 - Incorporated feedback from the community on v1 series.
 - Refactored the existing ima_dump_measurement_list to move buffer
   allocation functionality to ima_alloc_kexec_buf() function.
 - Introduced a new Kconfig option to configure the memory.
 - Updated the logic to copy the IMA log only in case of kexec soft 
   reboot, and not on kexec crash.
 - Updated the logic to copy as many IMA events as possible in case of
   memory constraint, rather than just bailing out.
 - Introduced two new events to be measured by IMA during kexec, to
   help diagnose if the IMA log was copied fully or partially from the
   current Kernel to the next.
 - Refactored patches to ensure no warnings during individual patch
   compilation.
 - Used virt_to_page instead of phys_to_page.
 - Updated patch descriptions as necessary.

steven chen (7):
  ima: define and call ima_alloc_kexec_file_buf
  kexec: define functions to map and unmap segments
  ima: kexec: skip IMA segment validation after kexec soft reboot
  ima: kexec: define functions to copy IMA log at soft boot
  ima: kexec: move IMA log copy from kexec load to execute
  ima: make the kexec extra memory configurable
  ima: measure kexec load and exec events as critical data

 include/linux/ima.h                |   3 +
 include/linux/kexec.h              |   8 ++
 kernel/kexec_core.c                |  54 ++++++++
 kernel/kexec_file.c                |  30 +++++
 security/integrity/ima/Kconfig     |  10 ++
 security/integrity/ima/ima.h       |   7 +
 security/integrity/ima/ima_kexec.c | 203 ++++++++++++++++++++++++-----
 security/integrity/ima/ima_queue.c |   9 +-
 8 files changed, 287 insertions(+), 37 deletions(-)

-- 
2.25.1

[PATCH v8 1/7] ima: define and call ima_alloc_kexec_file_buf

[steven chen]

Carrying the IMA measurement list across kexec requires allocating a
buffer and copying the measurement records.  Separate allocating the
buffer and copying the measurement records into separate functions in
order to allocate the buffer at kexec 'load' and copy the measurements
at kexec 'execute'.

This patch includes the following changes:
 - Refactor ima_dump_measurement_list() to move the memory allocation
   to a separate function ima_alloc_kexec_file_buf() which allocates
   buffer of size 'kexec_segment_size' at kexec 'load'.
 - Make the local variable ima_kexec_file in ima_dump_measurement_list()
   a local static to the file, so that it can be accessed from 
   ima_alloc_kexec_file_buf(). Compare actual memory required to ensure 
   there is enough memory for the entire measurement record.
 - Copy as many measurement events as possible.
 - Make necessary changes to the function ima_add_kexec_buffer() to call
   the above two functions.
 - Compared the memory size allocated with memory size of the entire 
   measurement record. If there is not enough memory, it will copy as many
   IMA measurement records as possible, and this situation will result
   in a failure of remote attestation.

Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |   1 +
 security/integrity/ima/ima_kexec.c | 102 +++++++++++++++++++++--------
 security/integrity/ima/ima_queue.c |   4 +-
 3 files changed, 77 insertions(+), 30 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 24d09ea91b87..4428fcf42167 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -274,6 +274,7 @@ 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);
+int ima_get_binary_runtime_entry_size(struct ima_template_entry *entry);
 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 9d45f4d26f73..89088f1fa989 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -15,63 +15,97 @@
 #include "ima.h"
 
 #ifdef CONFIG_IMA_KEXEC
+static struct seq_file ima_kexec_file;
+
+static void ima_reset_kexec_file(struct seq_file *sf)
+{
+	sf->buf = NULL;
+	sf->size = 0;
+	sf->read_pos = 0;
+	sf->count = 0;
+}
+
+static void ima_free_kexec_file_buf(struct seq_file *sf)
+{
+	vfree(sf->buf);
+	ima_reset_kexec_file(sf);
+}
+
+static int ima_alloc_kexec_file_buf(size_t segment_size)
+{
+	/*
+	 * kexec 'load' may be called multiple times.
+	 * Free and realloc the buffer only if the segment_size is
+	 * changed from the previous kexec 'load' call.
+	 */
+	if (ima_kexec_file.buf && ima_kexec_file.size == segment_size)
+		goto out;
+
+	ima_free_kexec_file_buf(&ima_kexec_file);
+
+	/* segment size can't change between kexec load and execute */
+	ima_kexec_file.buf = vmalloc(segment_size);
+	if (!ima_kexec_file.buf)
+		return -ENOMEM;
+
+	ima_kexec_file.size = segment_size;
+
+out:
+	ima_kexec_file.read_pos = 0;
+	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
+
+	return 0;
+}
+
 static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 				     unsigned long segment_size)
 {
 	struct ima_queue_entry *qe;
-	struct seq_file file;
 	struct ima_kexec_hdr khdr;
 	int ret = 0;
+	size_t entry_size = 0;
 
-	/* segment size can't change between kexec load and execute */
-	file.buf = vmalloc(segment_size);
-	if (!file.buf) {
-		ret = -ENOMEM;
-		goto out;
+	if (!ima_kexec_file.buf) {
+		pr_err("Kexec file buf not allocated\n");
+		return -EINVAL;
 	}
 
-	file.file = NULL;
-	file.size = segment_size;
-	file.read_pos = 0;
-	file.count = sizeof(khdr);	/* reserved space */
-
 	memset(&khdr, 0, sizeof(khdr));
 	khdr.version = 1;
 	/* This is an append-only list, no need to hold the RCU read lock */
+	/* Copy as many IMA measurements list records as possible */
 	list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
-		if (file.count < file.size) {
+		entry_size += ima_get_binary_runtime_entry_size(qe->entry);
+		if (entry_size <= segment_size) {
 			khdr.count++;
-			ima_measurements_show(&file, qe);
+			ima_measurements_show(&ima_kexec_file, qe);
 		} else {
 			ret = -EINVAL;
+			pr_err("IMA log file is too big for Kexec buf\n");
 			break;
 		}
 	}
 
-	if (ret < 0)
-		goto out;
-
 	/*
 	 * fill in reserved space with some buffer details
 	 * (eg. version, buffer size, number of measurements)
 	 */
-	khdr.buffer_size = file.count;
+	khdr.buffer_size = ima_kexec_file.count;
 	if (ima_canonical_fmt) {
 		khdr.version = cpu_to_le16(khdr.version);
 		khdr.count = cpu_to_le64(khdr.count);
 		khdr.buffer_size = cpu_to_le64(khdr.buffer_size);
 	}
-	memcpy(file.buf, &khdr, sizeof(khdr));
+	memcpy(ima_kexec_file.buf, &khdr, sizeof(khdr));
 
 	print_hex_dump_debug("ima dump: ", DUMP_PREFIX_NONE, 16, 1,
-			     file.buf, file.count < 100 ? file.count : 100,
+			     ima_kexec_file.buf, ima_kexec_file.count < 100 ?
+			     ima_kexec_file.count : 100,
 			     true);
 
-	*buffer_size = file.count;
-	*buffer = file.buf;
-out:
-	if (ret == -EINVAL)
-		vfree(file.buf);
+	*buffer_size = ima_kexec_file.count;
+	*buffer = ima_kexec_file.buf;
+
 	return ret;
 }
 
@@ -90,7 +124,7 @@ void ima_add_kexec_buffer(struct kimage *image)
 
 	/* use more understandable variable names than defined in kbuf */
 	void *kexec_buffer = NULL;
-	size_t kexec_buffer_size;
+	size_t kexec_buffer_size = 0;
 	size_t kexec_segment_size;
 	int ret;
 
@@ -110,13 +144,19 @@ void ima_add_kexec_buffer(struct kimage *image)
 		return;
 	}
 
-	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
-				  kexec_segment_size);
-	if (!kexec_buffer) {
+	ret = ima_alloc_kexec_file_buf(kexec_segment_size);
+	if (ret < 0) {
 		pr_err("Not enough memory for the kexec measurement buffer.\n");
 		return;
 	}
 
+	ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
+					kexec_segment_size);
+	if (ret < 0) {
+		pr_err("Failed to dump IMA measurements. Error:%d.\n", ret);
+		return;
+	}
+
 	kbuf.buffer = kexec_buffer;
 	kbuf.bufsz = kexec_buffer_size;
 	kbuf.memsz = kexec_segment_size;
@@ -131,6 +171,12 @@ void ima_add_kexec_buffer(struct kimage *image)
 	image->ima_buffer_size = kexec_segment_size;
 	image->ima_buffer = kexec_buffer;
 
+	/*
+	 * kexec owns kexec_buffer after kexec_add_buffer() is called
+	 * and it will vfree() that buffer.
+	 */
+	ima_reset_kexec_file(&ima_kexec_file);
+
 	kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
 		      kbuf.mem);
 }
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 83d53824aa98..3dfd178d4292 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -78,7 +78,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
  * binary_runtime_measurement list entry, which contains a
  * couple of variable length fields (e.g template name and data).
  */
-static int get_binary_runtime_size(struct ima_template_entry *entry)
+int ima_get_binary_runtime_entry_size(struct ima_template_entry *entry)
 {
 	int size = 0;
 
@@ -122,7 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 	if (binary_runtime_size != ULONG_MAX) {
 		int size;
 
-		size = get_binary_runtime_size(entry);
+		size = ima_get_binary_runtime_entry_size(entry);
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
-- 
2.25.1

[PATCH v8 2/7] kexec: define functions to map and unmap segments

[steven chen]

Currently, the mechanism to map and unmap segments to the kimage
structure is not available to the subsystems outside of kexec.  This
functionality is needed when IMA is allocating the memory segments
during kexec 'load' operation.  Implement functions to map and unmap
segments to kimage.

Implement kimage_map_segment() to enable mapping of IMA buffer source
pages to the kimage structure post kexec 'load'.  This function,
accepting a kimage pointer, an address, and a size, will gather the
source pages within the specified address range, create an array of page
pointers, and map these to a contiguous virtual address range.  The
function returns the start of this range if successful, or NULL if
unsuccessful.

Implement kimage_unmap_segment() for unmapping segments
using vunmap().

From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
 include/linux/kexec.h |  5 ++++
 kernel/kexec_core.c   | 54 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index f0e9f8eda7a3..4dbf806bccef 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -467,6 +467,8 @@ extern bool kexec_file_dbg_print;
 #define kexec_dprintk(fmt, arg...) \
         do { if (kexec_file_dbg_print) pr_info(fmt, ##arg); } while (0)
 
+extern void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size);
+extern void kimage_unmap_segment(void *buffer);
 #else /* !CONFIG_KEXEC_CORE */
 struct pt_regs;
 struct task_struct;
@@ -474,6 +476,9 @@ static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
 static inline int kexec_crash_loaded(void) { return 0; }
+static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
+{ return NULL; }
+static inline void kimage_unmap_segment(void *buffer) { }
 #define kexec_in_progress false
 #endif /* CONFIG_KEXEC_CORE */
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index c0bdc1686154..63e4d16b6023 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -867,6 +867,60 @@ int kimage_load_segment(struct kimage *image,
 	return result;
 }
 
+void *kimage_map_segment(struct kimage *image,
+			 unsigned long addr, unsigned long size)
+{
+	unsigned long eaddr = addr + size;
+	unsigned long src_page_addr, dest_page_addr;
+	unsigned int npages;
+	struct page **src_pages;
+	int i;
+	kimage_entry_t *ptr, entry;
+	void *vaddr = NULL;
+
+	/*
+	 * Collect the source pages and map them in a contiguous VA range.
+	 */
+	npages = PFN_UP(eaddr) - PFN_DOWN(addr);
+	src_pages = kmalloc_array(npages, sizeof(*src_pages), GFP_KERNEL);
+	if (!src_pages) {
+		pr_err("Could not allocate ima pages array.\n");
+		return NULL;
+	}
+
+	i = 0;
+	for_each_kimage_entry(image, ptr, entry) {
+		if (entry & IND_DESTINATION) {
+			dest_page_addr = entry & PAGE_MASK;
+		} else if (entry & IND_SOURCE) {
+			if (dest_page_addr >= addr && dest_page_addr < eaddr) {
+				src_page_addr = entry & PAGE_MASK;
+				src_pages[i++] =
+					virt_to_page(__va(src_page_addr));
+				if (i == npages)
+					break;
+				dest_page_addr += PAGE_SIZE;
+			}
+		}
+	}
+
+	/* Sanity check. */
+	WARN_ON(i < npages);
+
+	vaddr = vmap(src_pages, npages, VM_MAP, PAGE_KERNEL);
+	kfree(src_pages);
+
+	if (!vaddr)
+		pr_err("Could not map ima buffer.\n");
+
+	return vaddr;
+}
+
+void kimage_unmap_segment(void *segment_buffer)
+{
+	vunmap(segment_buffer);
+}
+
 struct kexec_load_limit {
 	/* Mutex protects the limit count. */
 	struct mutex mutex;
-- 
2.25.1

[PATCH v8 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot

[steven chen]

kexec_calculate_store_digests() calculates and stores the digest of the
segment at kexec_file_load syscall where the IMA segment is also
allocated.  With this series, the IMA segment will be updated with the
measurement log at kexec execute stage when soft reboot is initiated. 
Therefore, it may fail digest verification in verify_sha256_digest() 
after kexec soft reboot into the new kernel. Therefore, the digest 
calculation/verification of the IMA segment needs to be skipped.

Skip the calculating and storing digest of the IMA segment in
kexec_calculate_store_digests() so that it is not added to the
'purgatory_sha_regions'.

Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
no change is needed in verify_sha256_digest() in this context.

With this change, the IMA segment is not included in the digest
calculation, storage, and verification.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 include/linux/kexec.h              |  3 +++
 kernel/kexec_file.c                | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_kexec.c |  3 +++
 3 files changed, 28 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 4dbf806bccef..bd554ced9fb2 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -362,6 +362,9 @@ struct kimage {
 
 	phys_addr_t ima_buffer_addr;
 	size_t ima_buffer_size;
+
+	unsigned long ima_segment_index;
+	bool is_ima_segment_index_set;
 #endif
 
 	/* Core ELF header buffer */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 3eedb8c226ad..606132253c79 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
 }
 #endif
 
+#ifdef CONFIG_IMA_KEXEC
+static bool check_ima_segment_index(struct kimage *image, int i)
+{
+	if (image->is_ima_segment_index_set && i == image->ima_segment_index)
+		return true;
+	else
+		return false;
+}
+#else
+static bool check_ima_segment_index(struct kimage *image, int i)
+{
+	return false;
+}
+#endif
+
 static int kexec_calculate_store_digests(struct kimage *image);
 
 /* Maximum size in bytes for kernel/initrd files. */
@@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage *image)
 		if (ksegment->kbuf == pi->purgatory_buf)
 			continue;
 
+		/*
+		 * Skip the segment if ima_segment_index is set and matches
+		 * the current index
+		 */
+		if (check_ima_segment_index(image, i))
+			continue;
+
 		ret = crypto_shash_update(desc, ksegment->kbuf,
 					  ksegment->bufsz);
 		if (ret)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 89088f1fa989..704676fa6615 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -160,6 +160,7 @@ void ima_add_kexec_buffer(struct kimage *image)
 	kbuf.buffer = kexec_buffer;
 	kbuf.bufsz = kexec_buffer_size;
 	kbuf.memsz = kexec_segment_size;
+	image->is_ima_segment_index_set = false;
 	ret = kexec_add_buffer(&kbuf);
 	if (ret) {
 		pr_err("Error passing over kexec measurement buffer.\n");
@@ -170,6 +171,8 @@ void ima_add_kexec_buffer(struct kimage *image)
 	image->ima_buffer_addr = kbuf.mem;
 	image->ima_buffer_size = kexec_segment_size;
 	image->ima_buffer = kexec_buffer;
+	image->ima_segment_index = image->nr_segments - 1;
+	image->is_ima_segment_index_set = true;
 
 	/*
 	 * kexec owns kexec_buffer after kexec_add_buffer() is called
-- 
2.25.1

[PATCH v8 4/7] ima: kexec: define functions to copy IMA log at soft boot

[steven chen]

IMA log is copied to the new Kernel during kexec 'load' using 
ima_dump_measurement_list().  The log copy at kexec 'load' may result in
loss of IMA measurements during kexec soft reboot.  It needs to be copied
over during kexec 'execute'.  Setup the needed infrastructure to move the
IMA log copy from kexec 'load' to 'execute'. 

Define a new IMA hook ima_update_kexec_buffer() as a stub function.
It will be used to call ima_dump_measurement_list() during kexec 
'execute'.   

Implement ima_kexec_post_load() function to be invoked after the new 
Kernel image has been loaded for kexec. ima_kexec_post_load() maps the 
IMA buffer to a segment in the newly loaded Kernel.  It also registers 
the reboot notifier_block to trigger ima_update_kexec_buffer() at 
exec 'execute'.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
 include/linux/ima.h                |  3 ++
 security/integrity/ima/ima_kexec.c | 46 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0bae61a15b60..8e29cb4e6a01 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -32,6 +32,9 @@ static inline void ima_appraise_parse_cmdline(void) {}
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
+extern void ima_kexec_post_load(struct kimage *image);
+#else
+static inline void ima_kexec_post_load(struct kimage *image) {}
 #endif
 
 #else
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 704676fa6615..0fa65f91414b 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -12,10 +12,14 @@
 #include <linux/kexec.h>
 #include <linux/of.h>
 #include <linux/ima.h>
+#include <linux/reboot.h>
+#include <asm/page.h>
 #include "ima.h"
 
 #ifdef CONFIG_IMA_KEXEC
 static struct seq_file ima_kexec_file;
+static void *ima_kexec_buffer;
+static bool ima_kexec_update_registered;
 
 static void ima_reset_kexec_file(struct seq_file *sf)
 {
@@ -183,6 +187,48 @@ void ima_add_kexec_buffer(struct kimage *image)
 	kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
 		      kbuf.mem);
 }
+
+/*
+ * Called during kexec execute so that IMA can update the measurement list.
+ */
+static int ima_update_kexec_buffer(struct notifier_block *self,
+				   unsigned long action, void *data)
+{
+	return NOTIFY_OK;
+}
+
+struct notifier_block update_buffer_nb = {
+	.notifier_call = ima_update_kexec_buffer,
+};
+
+/*
+ * Create a mapping for the source pages that contain the IMA buffer
+ * so we can update it later.
+ */
+void ima_kexec_post_load(struct kimage *image)
+{
+	if (ima_kexec_buffer) {
+		kimage_unmap_segment(ima_kexec_buffer);
+		ima_kexec_buffer = NULL;
+	}
+
+	if (!image->ima_buffer_addr)
+		return;
+
+	ima_kexec_buffer = kimage_map_segment(image,
+					      image->ima_buffer_addr,
+					      image->ima_buffer_size);
+	if (!ima_kexec_buffer) {
+		pr_err("Could not map measurements buffer.\n");
+		return;
+	}
+
+	if (!ima_kexec_update_registered) {
+		register_reboot_notifier(&update_buffer_nb);
+		ima_kexec_update_registered = true;
+	}
+}
+
 #endif /* IMA_KEXEC */
 
 /*
-- 
2.25.1

[PATCH v8 5/7] ima: kexec: move IMA log copy from kexec load to execute

[steven chen]

ima_dump_measurement_list() is called during kexec 'load', which may
result in loss of IMA measurements during kexec soft reboot.  It needs
to be called during kexec 'execute'.

This patch includes the following changes:
 - Implement kimage_file_post_load() function to be invoked after the new
   Kernel image has been loaded for kexec.
 - Call kimage_file_post_load() from kexec_file_load() syscall only for
   kexec soft reboot scenarios and not for KEXEC_FILE_ON_CRASH.  It will
   map the IMA segment, and register reboot notifier for the function
   ima_update_kexec_buffer() which would copy the IMA log at kexec soft
   reboot.
 - Make kexec_segment_size variable local static to the file, for it to be
   accessible both during kexec 'load' and 'execute'.
 - Move ima_dump_measurement_list() call from ima_add_kexec_buffer()
   to ima_update_kexec_buffer().
 - Remove ima_reset_kexec_file() call from ima_add_kexec_buffer(), now
   that the buffer is being copied at kexec 'execute', and resetting the
   file at kexec 'load' will corrupt the buffer.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
 kernel/kexec_file.c                |  8 ++++++
 security/integrity/ima/ima_kexec.c | 43 +++++++++++++++++++-----------
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 606132253c79..76b6a877b842 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -201,6 +201,11 @@ kimage_validate_signature(struct kimage *image)
 }
 #endif
 
+static void kimage_file_post_load(struct kimage *image)
+{
+	ima_kexec_post_load(image);
+}
+
 /*
  * In file mode list of segments is prepared by kernel. Copy relevant
  * data from user space, do error checking, prepare segment list
@@ -428,6 +433,9 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 
 	kimage_terminate(image);
 
+	if (!(flags & KEXEC_FILE_ON_CRASH))
+		kimage_file_post_load(image);
+
 	ret = machine_kexec_post_load(image);
 	if (ret)
 		goto out;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 0fa65f91414b..f9dd7ff95b84 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -19,6 +19,7 @@
 #ifdef CONFIG_IMA_KEXEC
 static struct seq_file ima_kexec_file;
 static void *ima_kexec_buffer;
+static size_t kexec_segment_size;
 static bool ima_kexec_update_registered;
 
 static void ima_reset_kexec_file(struct seq_file *sf)
@@ -129,7 +130,6 @@ void ima_add_kexec_buffer(struct kimage *image)
 	/* use more understandable variable names than defined in kbuf */
 	void *kexec_buffer = NULL;
 	size_t kexec_buffer_size = 0;
-	size_t kexec_segment_size;
 	int ret;
 
 	/*
@@ -154,13 +154,6 @@ void ima_add_kexec_buffer(struct kimage *image)
 		return;
 	}
 
-	ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
-					kexec_segment_size);
-	if (ret < 0) {
-		pr_err("Failed to dump IMA measurements. Error:%d.\n", ret);
-		return;
-	}
-
 	kbuf.buffer = kexec_buffer;
 	kbuf.bufsz = kexec_buffer_size;
 	kbuf.memsz = kexec_segment_size;
@@ -178,12 +171,6 @@ void ima_add_kexec_buffer(struct kimage *image)
 	image->ima_segment_index = image->nr_segments - 1;
 	image->is_ima_segment_index_set = true;
 
-	/*
-	 * kexec owns kexec_buffer after kexec_add_buffer() is called
-	 * and it will vfree() that buffer.
-	 */
-	ima_reset_kexec_file(&ima_kexec_file);
-
 	kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
 		      kbuf.mem);
 }
@@ -194,7 +181,33 @@ void ima_add_kexec_buffer(struct kimage *image)
 static int ima_update_kexec_buffer(struct notifier_block *self,
 				   unsigned long action, void *data)
 {
-	return NOTIFY_OK;
+	void *buf = NULL;
+	size_t buf_size = 0;
+	int ret = NOTIFY_OK;
+
+	if (!kexec_in_progress) {
+		pr_info("No kexec in progress.\n");
+		return ret;
+	}
+
+	if (!ima_kexec_buffer) {
+		pr_err("Kexec buffer not set.\n");
+		return ret;
+	}
+
+	ret = ima_dump_measurement_list(&buf_size, &buf,
+					kexec_segment_size);
+
+	if (ret)
+		pr_err("Dump measurements failed. Error:%d\n", ret);
+
+	if (buf_size != 0)
+		memcpy(ima_kexec_buffer, buf, buf_size);
+
+	kimage_unmap_segment(ima_kexec_buffer);
+	ima_kexec_buffer = NULL;
+
+	return ret;
 }
 
 struct notifier_block update_buffer_nb = {
-- 
2.25.1

[PATCH v8 6/7] ima: make the kexec extra memory configurable

[steven chen]

The extra memory allocated for carrying the IMA measurement list across
kexec is hard-coded as half a PAGE.  Make it configurable.

Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
extra memory (in kb) to be allocated for IMA measurements added during
kexec soft reboot.  Ensure the default value of the option is set such
that extra half a page of memory for additional measurements is allocated
for the additional measurements.

Update ima_add_kexec_buffer() function to allocate memory based on the
Kconfig option value, rather than the currently hard-coded one.

Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 security/integrity/ima/Kconfig     | 10 ++++++++++
 security/integrity/ima/ima_kexec.c | 16 ++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 475c32615006..d73c96c3c1c9 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
 	help
 	   This option disables htable to allow measurement of duplicate records.
 
+config IMA_KEXEC_EXTRA_MEMORY_KB
+	int "Extra memory for IMA measurements added during kexec soft reboot"
+	depends on IMA_KEXEC
+	default 0
+	help
+	  IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
+	  allocated (in kb) for IMA measurements added during kexec soft reboot.
+	  If set to the default value of 0, an extra half page of memory for those
+	  additional measurements will be allocated.
+
 endif
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index f9dd7ff95b84..6c8c203ad81e 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -126,22 +126,26 @@ void ima_add_kexec_buffer(struct kimage *image)
 				  .buf_min = 0, .buf_max = ULONG_MAX,
 				  .top_down = true };
 	unsigned long binary_runtime_size;
-
+	unsigned long extra_memory;
 	/* use more understandable variable names than defined in kbuf */
 	void *kexec_buffer = NULL;
 	size_t kexec_buffer_size = 0;
 	int ret;
 
 	/*
-	 * Reserve an extra half page of memory for additional measurements
-	 * added during the kexec load.
+	 * Reserve extra memory for measurements added during kexec.
 	 */
-	binary_runtime_size = ima_get_binary_runtime_size();
+	if (CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB <= 0)
+		extra_memory = PAGE_SIZE / 2;
+	else
+		extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
+	binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
+
 	if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
 		kexec_segment_size = ULONG_MAX;
 	else
-		kexec_segment_size = ALIGN(ima_get_binary_runtime_size() +
-					   PAGE_SIZE / 2, PAGE_SIZE);
+		kexec_segment_size = ALIGN(binary_runtime_size, PAGE_SIZE);
+
 	if ((kexec_segment_size == ULONG_MAX) ||
 	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
 		pr_err("Binary measurement list too large.\n");
-- 
2.25.1

[PATCH v8 7/7] ima: measure kexec load and exec events as critical data

[steven chen]

The amount of memory allocated at kexec load, even with the extra memory
allocated, might not be large enough for the entire measurement list.  The
indeterminate interval between kexec 'load' and 'execute' could exacerbate
this problem.

Define two new IMA events, 'kexec_load' and 'kexec_execute', to be 
measured as critical data at kexec 'load' and 'execute' respectively.
Report the allocated kexec segment size, IMA binary log size and the
runtime measurements count as part of those events.

These events, and the values reported through them, serve as markers in
the IMA log to verify the IMA events are captured during kexec soft
reboot.  The presence of a 'kexec_load' event in between the last two
'boot_aggregate' events in the IMA log implies this is a kexec soft
reboot, and not a cold-boot. And the absence of 'kexec_execute' event
after kexec soft reboot implies missing events in that window which
results in inconsistency with TPM PCR quotes, necessitating a cold boot
for a successful remote attestation.

The 'kexec_load' event IMA log can be found using the following command:
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
   grep kexec_load

The 'kexec_load' event IMA log can be found using the following command:
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
   grep kexec_execute

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |  6 ++++++
 security/integrity/ima/ima_kexec.c | 21 +++++++++++++++++++++
 security/integrity/ima/ima_queue.c |  5 +++++
 3 files changed, 32 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4428fcf42167..1452c98242a4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -240,6 +240,12 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
 				   unsigned long flags, bool create);
 #endif
 
+#ifdef CONFIG_IMA_KEXEC
+void ima_measure_kexec_event(const char *event_name);
+#else
+static inline void ima_measure_kexec_event(const char *event_name) {}
+#endif
+
 /*
  * The default binary_runtime_measurements list format is defined as the
  * platform native format.  The canonical format is defined as little-endian.
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 6c8c203ad81e..8d0782e51ffa 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 256
+
 static struct seq_file ima_kexec_file;
 static void *ima_kexec_buffer;
 static size_t kexec_segment_size;
@@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
 	ima_reset_kexec_file(sf);
 }
 
+void ima_measure_kexec_event(const char *event_name)
+{
+	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
+	size_t buf_size = 0;
+	long len;
+
+	buf_size = ima_get_binary_runtime_size();
+	len = atomic_long_read(&ima_htable.len);
+
+	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
+		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
+		 "ima_runtime_measurements_count=%ld;",
+		 kexec_segment_size, buf_size, len);
+
+	ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event,
+				  strlen(ima_kexec_event), false, NULL, 0);
+}
+
 static int ima_alloc_kexec_file_buf(size_t segment_size)
 {
 	/*
@@ -58,6 +78,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
 out:
 	ima_kexec_file.read_pos = 0;
 	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
+	ima_measure_kexec_event("kexec_load");
 
 	return 0;
 }
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 3dfd178d4292..6afb46989cf6 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -241,6 +241,11 @@ static int ima_reboot_notifier(struct notifier_block *nb,
 			       unsigned long action,
 			       void *data)
 {
+#ifdef CONFIG_IMA_KEXEC
+	if (action == SYS_RESTART && data && !strcmp(data, "kexec reboot"))
+		ima_measure_kexec_event("kexec_execute");
+#endif
+
 	ima_measurements_suspend();
 
 	return NOTIFY_DONE;
-- 
2.25.1

Re: [PATCH v8 4/7] ima: kexec: define functions to copy IMA log at soft boot

[Stefan Berger]

On 2/18/25 5:54 PM, steven chen wrote:
> IMA log is copied to the new Kernel during kexec 'load' using

The IMA log is currently copied to the new kernel ...


> ima_dump_measurement_list().  The log copy at kexec 'load' may result in
> loss of IMA measurements during kexec soft reboot.  It needs to be copied

However, the log copied at kexec 'load' may result in loss of IMA 
measurements due to missed measurements that only occurred after kexec 
'load'. Therefore, the log needs to be copied during kexec 'execute'. 
Setup the ...

> over during kexec 'execute'.  Setup the needed infrastructure to move the
> IMA log copy from kexec 'load' to 'execute'.
> 
> Define a new IMA hook ima_update_kexec_buffer() as a stub function.
> It will be used to call ima_dump_measurement_list() during kexec
> 'execute'.
> 
> Implement ima_kexec_post_load() function to be invoked after the new
> Kernel image has been loaded for kexec. ima_kexec_post_load() maps the
> IMA buffer to a segment in the newly loaded Kernel.  It also registers
> the reboot notifier_block to trigger ima_update_kexec_buffer() at
> exec 'execute'.

kexec 'execute'

> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>

With the above changes:

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> ---
>   include/linux/ima.h                |  3 ++
>   security/integrity/ima/ima_kexec.c | 46 ++++++++++++++++++++++++++++++
>   2 files changed, 49 insertions(+)
> 
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0bae61a15b60..8e29cb4e6a01 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -32,6 +32,9 @@ static inline void ima_appraise_parse_cmdline(void) {}
>   
>   #ifdef CONFIG_IMA_KEXEC
>   extern void ima_add_kexec_buffer(struct kimage *image);
> +extern void ima_kexec_post_load(struct kimage *image);
> +#else
> +static inline void ima_kexec_post_load(struct kimage *image) {}
>   #endif
>   
>   #else
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 704676fa6615..0fa65f91414b 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -12,10 +12,14 @@
>   #include <linux/kexec.h>
>   #include <linux/of.h>
>   #include <linux/ima.h>
> +#include <linux/reboot.h>
> +#include <asm/page.h>
>   #include "ima.h"
>   
>   #ifdef CONFIG_IMA_KEXEC
>   static struct seq_file ima_kexec_file;
> +static void *ima_kexec_buffer;
> +static bool ima_kexec_update_registered;
>   
>   static void ima_reset_kexec_file(struct seq_file *sf)
>   {
> @@ -183,6 +187,48 @@ void ima_add_kexec_buffer(struct kimage *image)
>   	kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
>   		      kbuf.mem);
>   }
> +
> +/*
> + * Called during kexec execute so that IMA can update the measurement list.
> + */
> +static int ima_update_kexec_buffer(struct notifier_block *self,
> +				   unsigned long action, void *data)
> +{
> +	return NOTIFY_OK;
> +}
> +
> +struct notifier_block update_buffer_nb = {
> +	.notifier_call = ima_update_kexec_buffer,
> +};
> +
> +/*
> + * Create a mapping for the source pages that contain the IMA buffer
> + * so we can update it later.
> + */
> +void ima_kexec_post_load(struct kimage *image)
> +{
> +	if (ima_kexec_buffer) {
> +		kimage_unmap_segment(ima_kexec_buffer);
> +		ima_kexec_buffer = NULL;
> +	}
> +
> +	if (!image->ima_buffer_addr)
> +		return;
> +
> +	ima_kexec_buffer = kimage_map_segment(image,
> +					      image->ima_buffer_addr,
> +					      image->ima_buffer_size);
> +	if (!ima_kexec_buffer) {
> +		pr_err("Could not map measurements buffer.\n");
> +		return;
> +	}
> +
> +	if (!ima_kexec_update_registered) {
> +		register_reboot_notifier(&update_buffer_nb);
> +		ima_kexec_update_registered = true;
> +	}
> +}
> +
>   #endif /* IMA_KEXEC */
>   
>   /*

Re: [PATCH v8 5/7] ima: kexec: move IMA log copy from kexec load to execute

[Stefan Berger]

On 2/18/25 5:55 PM, steven chen wrote:
> ima_dump_measurement_list() is called during kexec 'load', which may
> result in loss of IMA measurements during kexec soft reboot.  It needs

... due to missed measurements that only occurred after kexec 'load'. 
Therefore, this function needs to be ...

> to be called during kexec 'execute'.
> 
> This patch includes the following changes:
>   - Implement kimage_file_post_load() function to be invoked after the new
>     Kernel image has been loaded for kexec.

s/Kernel/kernel

>   - Call kimage_file_post_load() from kexec_file_load() syscall only for
>     kexec soft reboot scenarios and not for KEXEC_FILE_ON_CRASH.  It will
>     map the IMA segment, and register reboot notifier for the function
>     ima_update_kexec_buffer() which would copy the IMA log at kexec soft
>     reboot.
>   - Make kexec_segment_size variable local static to the file, for it to be

... to the file so that it becomes accessible ...

>     accessible both during kexec 'load' and 'execute'.
>   - Move ima_dump_measurement_list() call from ima_add_kexec_buffer()
>     to ima_update_kexec_buffer().
>   - Remove ima_reset_kexec_file() call from ima_add_kexec_buffer(), now
>     that the buffer is being copied at kexec 'execute', and resetting the
>     file at kexec 'load' will corrupt the buffer.

s/will/would

> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>

With  the above changes:

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> ---
>   kernel/kexec_file.c                |  8 ++++++
>   security/integrity/ima/ima_kexec.c | 43 +++++++++++++++++++-----------
>   2 files changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 606132253c79..76b6a877b842 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -201,6 +201,11 @@ kimage_validate_signature(struct kimage *image)
>   }
>   #endif
>   
> +static void kimage_file_post_load(struct kimage *image)
> +{
> +	ima_kexec_post_load(image);
> +}
> +
>   /*
>    * In file mode list of segments is prepared by kernel. Copy relevant
>    * data from user space, do error checking, prepare segment list
> @@ -428,6 +433,9 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>   
>   	kimage_terminate(image);
>   
> +	if (!(flags & KEXEC_FILE_ON_CRASH))
> +		kimage_file_post_load(image);
> +
>   	ret = machine_kexec_post_load(image);
>   	if (ret)
>   		goto out;
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 0fa65f91414b..f9dd7ff95b84 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -19,6 +19,7 @@
>   #ifdef CONFIG_IMA_KEXEC
>   static struct seq_file ima_kexec_file;
>   static void *ima_kexec_buffer;
> +static size_t kexec_segment_size;
>   static bool ima_kexec_update_registered;
>   
>   static void ima_reset_kexec_file(struct seq_file *sf)
> @@ -129,7 +130,6 @@ void ima_add_kexec_buffer(struct kimage *image)
>   	/* use more understandable variable names than defined in kbuf */
>   	void *kexec_buffer = NULL;
>   	size_t kexec_buffer_size = 0;
> -	size_t kexec_segment_size;
>   	int ret;
>   
>   	/*
> @@ -154,13 +154,6 @@ void ima_add_kexec_buffer(struct kimage *image)
>   		return;
>   	}
>   
> -	ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> -					kexec_segment_size);
> -	if (ret < 0) {
> -		pr_err("Failed to dump IMA measurements. Error:%d.\n", ret);
> -		return;
> -	}
> -
>   	kbuf.buffer = kexec_buffer;
>   	kbuf.bufsz = kexec_buffer_size;
>   	kbuf.memsz = kexec_segment_size;
> @@ -178,12 +171,6 @@ void ima_add_kexec_buffer(struct kimage *image)
>   	image->ima_segment_index = image->nr_segments - 1;
>   	image->is_ima_segment_index_set = true;
>   
> -	/*
> -	 * kexec owns kexec_buffer after kexec_add_buffer() is called
> -	 * and it will vfree() that buffer.
> -	 */
> -	ima_reset_kexec_file(&ima_kexec_file);
> -
>   	kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
>   		      kbuf.mem);
>   }
> @@ -194,7 +181,33 @@ void ima_add_kexec_buffer(struct kimage *image)
>   static int ima_update_kexec_buffer(struct notifier_block *self,
>   				   unsigned long action, void *data)
>   {
> -	return NOTIFY_OK;
> +	void *buf = NULL;
> +	size_t buf_size = 0;
> +	int ret = NOTIFY_OK;
> +
> +	if (!kexec_in_progress) {
> +		pr_info("No kexec in progress.\n");
> +		return ret;
> +	}
> +
> +	if (!ima_kexec_buffer) {
> +		pr_err("Kexec buffer not set.\n");
> +		return ret;
> +	}
> +
> +	ret = ima_dump_measurement_list(&buf_size, &buf,
> +					kexec_segment_size);
> +
> +	if (ret)
> +		pr_err("Dump measurements failed. Error:%d\n", ret);
> +
> +	if (buf_size != 0)
> +		memcpy(ima_kexec_buffer, buf, buf_size);
> +
> +	kimage_unmap_segment(ima_kexec_buffer);
> +	ima_kexec_buffer = NULL;
> +
> +	return ret;
>   }
>   
>   struct notifier_block update_buffer_nb = {

Re: [PATCH v8 7/7] ima: measure kexec load and exec events as critical data

[Stefan Berger]

On 2/18/25 5:55 PM, steven chen wrote:
> The amount of memory allocated at kexec load, even with the extra memory
> allocated, might not be large enough for the entire measurement list.  The
> indeterminate interval between kexec 'load' and 'execute' could exacerbate
> this problem.
> 
> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
> measured as critical data at kexec 'load' and 'execute' respectively.
> Report the allocated kexec segment size, IMA binary log size and the
> runtime measurements count as part of those events.
> 
> These events, and the values reported through them, serve as markers in
> the IMA log to verify the IMA events are captured during kexec soft
> reboot.  The presence of a 'kexec_load' event in between the last two
> 'boot_aggregate' events in the IMA log implies this is a kexec soft
> reboot, and not a cold-boot. And the absence of 'kexec_execute' event
> after kexec soft reboot implies missing events in that window which
> results in inconsistency with TPM PCR quotes, necessitating a cold boot
> for a successful remote attestation.
> 
> The 'kexec_load' event IMA log can be found using the following command:
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>     grep kexec_load
> 
> The 'kexec_load' event IMA log can be found using the following command:
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>     grep kexec_execute
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
>   security/integrity/ima/ima.h       |  6 ++++++
>   security/integrity/ima/ima_kexec.c | 21 +++++++++++++++++++++
>   security/integrity/ima/ima_queue.c |  5 +++++
>   3 files changed, 32 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 4428fcf42167..1452c98242a4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -240,6 +240,12 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>   				   unsigned long flags, bool create);
>   #endif
>   
> +#ifdef CONFIG_IMA_KEXEC
> +void ima_measure_kexec_event(const char *event_name);
> +#else
> +static inline void ima_measure_kexec_event(const char *event_name) {}
> +#endif
> +
>   /*
>    * The default binary_runtime_measurements list format is defined as the
>    * platform native format.  The canonical format is defined as little-endian.
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 6c8c203ad81e..8d0782e51ffa 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 256
> +
>   static struct seq_file ima_kexec_file;
>   static void *ima_kexec_buffer;
>   static size_t kexec_segment_size;
> @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
>   	ima_reset_kexec_file(sf);
>   }
>   
> +void ima_measure_kexec_event(const char *event_name)
> +{
> +	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
> +	size_t buf_size = 0;
> +	long len;
> +
> +	buf_size = ima_get_binary_runtime_size();
> +	len = atomic_long_read(&ima_htable.len);
> +
> +	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
> +		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
> +		 "ima_runtime_measurements_count=%ld;",
> +		 kexec_segment_size, buf_size, len);

Indentation and n = scnprintf(...), as Mimi mentioned in v7.

> +
> +	ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event,
> +				  strlen(ima_kexec_event), false, NULL, 0);

Replace strlen(ima_kexec_event) with 'n'.

With these fixes:

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> +}
> +
>   static int ima_alloc_kexec_file_buf(size_t segment_size)
>   {
>   	/*
> @@ -58,6 +78,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
>   out:
>   	ima_kexec_file.read_pos = 0;
>   	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
> +	ima_measure_kexec_event("kexec_load");
>   
>   	return 0;
>   }
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 3dfd178d4292..6afb46989cf6 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -241,6 +241,11 @@ static int ima_reboot_notifier(struct notifier_block *nb,
>   			       unsigned long action,
>   			       void *data)
>   {
> +#ifdef CONFIG_IMA_KEXEC
> +	if (action == SYS_RESTART && data && !strcmp(data, "kexec reboot"))
> +		ima_measure_kexec_event("kexec_execute");
> +#endif
> +
>   	ima_measurements_suspend();
>   
>   	return NOTIFY_DONE;

Re: [PATCH v8 4/7] ima: kexec: define functions to copy IMA log at soft boot

[steven chen]

On 2/19/2025 7:37 AM, Stefan Berger wrote:
>
>
> On 2/18/25 5:54 PM, steven chen wrote:
>> IMA log is copied to the new Kernel during kexec 'load' using
>
> The IMA log is currently copied to the new kernel ...
>
>
>> ima_dump_measurement_list().  The log copy at kexec 'load' may result in
>> loss of IMA measurements during kexec soft reboot.  It needs to be 
>> copied
>
> However, the log copied at kexec 'load' may result in loss of IMA 
> measurements due to missed measurements that only occurred after kexec 
> 'load'. Therefore, the log needs to be copied during kexec 'execute'. 
> Setup the ...
>
>> over during kexec 'execute'.  Setup the needed infrastructure to move 
>> the
>> IMA log copy from kexec 'load' to 'execute'.
>>
>> Define a new IMA hook ima_update_kexec_buffer() as a stub function.
>> It will be used to call ima_dump_measurement_list() during kexec
>> 'execute'.
>>
>> Implement ima_kexec_post_load() function to be invoked after the new
>> Kernel image has been loaded for kexec. ima_kexec_post_load() maps the
>> IMA buffer to a segment in the newly loaded Kernel.  It also registers
>> the reboot notifier_block to trigger ima_update_kexec_buffer() at
>> exec 'execute'.
>
> kexec 'execute'
>
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>
> With the above changes:
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>
>> ---
>>   include/linux/ima.h                |  3 ++
>>   security/integrity/ima/ima_kexec.c | 46 ++++++++++++++++++++++++++++++
>>   2 files changed, 49 insertions(+)
>>
>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>> index 0bae61a15b60..8e29cb4e6a01 100644
>> --- a/include/linux/ima.h
>> +++ b/include/linux/ima.h
>> @@ -32,6 +32,9 @@ static inline void ima_appraise_parse_cmdline(void) {}
>>     #ifdef CONFIG_IMA_KEXEC
>>   extern void ima_add_kexec_buffer(struct kimage *image);
>> +extern void ima_kexec_post_load(struct kimage *image);
>> +#else
>> +static inline void ima_kexec_post_load(struct kimage *image) {}
>>   #endif
>>     #else
>> diff --git a/security/integrity/ima/ima_kexec.c 
>> b/security/integrity/ima/ima_kexec.c
>> index 704676fa6615..0fa65f91414b 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -12,10 +12,14 @@
>>   #include <linux/kexec.h>
>>   #include <linux/of.h>
>>   #include <linux/ima.h>
>> +#include <linux/reboot.h>
>> +#include <asm/page.h>
>>   #include "ima.h"
>>     #ifdef CONFIG_IMA_KEXEC
>>   static struct seq_file ima_kexec_file;
>> +static void *ima_kexec_buffer;
>> +static bool ima_kexec_update_registered;
>>     static void ima_reset_kexec_file(struct seq_file *sf)
>>   {
>> @@ -183,6 +187,48 @@ void ima_add_kexec_buffer(struct kimage *image)
>>       kexec_dprintk("kexec measurement buffer for the loaded kernel 
>> at 0x%lx.\n",
>>                 kbuf.mem);
>>   }
>> +
>> +/*
>> + * Called during kexec execute so that IMA can update the 
>> measurement list.
>> + */
>> +static int ima_update_kexec_buffer(struct notifier_block *self,
>> +                   unsigned long action, void *data)
>> +{
>> +    return NOTIFY_OK;
>> +}
>> +
>> +struct notifier_block update_buffer_nb = {
>> +    .notifier_call = ima_update_kexec_buffer,
>> +};
>> +
>> +/*
>> + * Create a mapping for the source pages that contain the IMA buffer
>> + * so we can update it later.
>> + */
>> +void ima_kexec_post_load(struct kimage *image)
>> +{
>> +    if (ima_kexec_buffer) {
>> +        kimage_unmap_segment(ima_kexec_buffer);
>> +        ima_kexec_buffer = NULL;
>> +    }
>> +
>> +    if (!image->ima_buffer_addr)
>> +        return;
>> +
>> +    ima_kexec_buffer = kimage_map_segment(image,
>> +                          image->ima_buffer_addr,
>> +                          image->ima_buffer_size);
>> +    if (!ima_kexec_buffer) {
>> +        pr_err("Could not map measurements buffer.\n");
>> +        return;
>> +    }
>> +
>> +    if (!ima_kexec_update_registered) {
>> +        register_reboot_notifier(&update_buffer_nb);
>> +        ima_kexec_update_registered = true;
>> +    }
>> +}
>> +
>>   #endif /* IMA_KEXEC */
>>     /*

Hi Stefan, thanks for your comments. I will update in next version.

Re: [PATCH v8 5/7] ima: kexec: move IMA log copy from kexec load to execute

[steven chen]

On 2/19/2025 7:57 AM, Stefan Berger wrote:
>
>
> On 2/18/25 5:55 PM, steven chen wrote:
>> ima_dump_measurement_list() is called during kexec 'load', which may
>> result in loss of IMA measurements during kexec soft reboot.  It needs
>
> ... due to missed measurements that only occurred after kexec 'load'. 
> Therefore, this function needs to be ...
>
>> to be called during kexec 'execute'.
>>
>> This patch includes the following changes:
>>   - Implement kimage_file_post_load() function to be invoked after 
>> the new
>>     Kernel image has been loaded for kexec.
>
> s/Kernel/kernel
>
>>   - Call kimage_file_post_load() from kexec_file_load() syscall only for
>>     kexec soft reboot scenarios and not for KEXEC_FILE_ON_CRASH.  It 
>> will
>>     map the IMA segment, and register reboot notifier for the function
>>     ima_update_kexec_buffer() which would copy the IMA log at kexec soft
>>     reboot.
>>   - Make kexec_segment_size variable local static to the file, for it 
>> to be
>
> ... to the file so that it becomes accessible ...
>
>>     accessible both during kexec 'load' and 'execute'.
>>   - Move ima_dump_measurement_list() call from ima_add_kexec_buffer()
>>     to ima_update_kexec_buffer().
>>   - Remove ima_reset_kexec_file() call from ima_add_kexec_buffer(), now
>>     that the buffer is being copied at kexec 'execute', and resetting 
>> the
>>     file at kexec 'load' will corrupt the buffer.
>
> s/will/would
>
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>
> With  the above changes:
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>
>> ---
>>   kernel/kexec_file.c                |  8 ++++++
>>   security/integrity/ima/ima_kexec.c | 43 +++++++++++++++++++-----------
>>   2 files changed, 36 insertions(+), 15 deletions(-)
>>
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 606132253c79..76b6a877b842 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -201,6 +201,11 @@ kimage_validate_signature(struct kimage *image)
>>   }
>>   #endif
>>   +static void kimage_file_post_load(struct kimage *image)
>> +{
>> +    ima_kexec_post_load(image);
>> +}
>> +
>>   /*
>>    * In file mode list of segments is prepared by kernel. Copy relevant
>>    * data from user space, do error checking, prepare segment list
>> @@ -428,6 +433,9 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, 
>> int, initrd_fd,
>>         kimage_terminate(image);
>>   +    if (!(flags & KEXEC_FILE_ON_CRASH))
>> +        kimage_file_post_load(image);
>> +
>>       ret = machine_kexec_post_load(image);
>>       if (ret)
>>           goto out;
>> diff --git a/security/integrity/ima/ima_kexec.c 
>> b/security/integrity/ima/ima_kexec.c
>> index 0fa65f91414b..f9dd7ff95b84 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -19,6 +19,7 @@
>>   #ifdef CONFIG_IMA_KEXEC
>>   static struct seq_file ima_kexec_file;
>>   static void *ima_kexec_buffer;
>> +static size_t kexec_segment_size;
>>   static bool ima_kexec_update_registered;
>>     static void ima_reset_kexec_file(struct seq_file *sf)
>> @@ -129,7 +130,6 @@ void ima_add_kexec_buffer(struct kimage *image)
>>       /* use more understandable variable names than defined in kbuf */
>>       void *kexec_buffer = NULL;
>>       size_t kexec_buffer_size = 0;
>> -    size_t kexec_segment_size;
>>       int ret;
>>         /*
>> @@ -154,13 +154,6 @@ void ima_add_kexec_buffer(struct kimage *image)
>>           return;
>>       }
>>   -    ret = ima_dump_measurement_list(&kexec_buffer_size, 
>> &kexec_buffer,
>> -                    kexec_segment_size);
>> -    if (ret < 0) {
>> -        pr_err("Failed to dump IMA measurements. Error:%d.\n", ret);
>> -        return;
>> -    }
>> -
>>       kbuf.buffer = kexec_buffer;
>>       kbuf.bufsz = kexec_buffer_size;
>>       kbuf.memsz = kexec_segment_size;
>> @@ -178,12 +171,6 @@ void ima_add_kexec_buffer(struct kimage *image)
>>       image->ima_segment_index = image->nr_segments - 1;
>>       image->is_ima_segment_index_set = true;
>>   -    /*
>> -     * kexec owns kexec_buffer after kexec_add_buffer() is called
>> -     * and it will vfree() that buffer.
>> -     */
>> -    ima_reset_kexec_file(&ima_kexec_file);
>> -
>>       kexec_dprintk("kexec measurement buffer for the loaded kernel 
>> at 0x%lx.\n",
>>                 kbuf.mem);
>>   }
>> @@ -194,7 +181,33 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   static int ima_update_kexec_buffer(struct notifier_block *self,
>>                      unsigned long action, void *data)
>>   {
>> -    return NOTIFY_OK;
>> +    void *buf = NULL;
>> +    size_t buf_size = 0;
>> +    int ret = NOTIFY_OK;
>> +
>> +    if (!kexec_in_progress) {
>> +        pr_info("No kexec in progress.\n");
>> +        return ret;
>> +    }
>> +
>> +    if (!ima_kexec_buffer) {
>> +        pr_err("Kexec buffer not set.\n");
>> +        return ret;
>> +    }
>> +
>> +    ret = ima_dump_measurement_list(&buf_size, &buf,
>> +                    kexec_segment_size);
>> +
>> +    if (ret)
>> +        pr_err("Dump measurements failed. Error:%d\n", ret);
>> +
>> +    if (buf_size != 0)
>> +        memcpy(ima_kexec_buffer, buf, buf_size);
>> +
>> +    kimage_unmap_segment(ima_kexec_buffer);
>> +    ima_kexec_buffer = NULL;
>> +
>> +    return ret;
>>   }
>>     struct notifier_block update_buffer_nb = {

Hi Stefan, thanks for your comments. I will update in next version.

Re: [PATCH v8 7/7] ima: measure kexec load and exec events as critical data

[steven chen]

On 2/19/2025 8:23 AM, Stefan Berger wrote:
>
>
> On 2/18/25 5:55 PM, steven chen wrote:
>> The amount of memory allocated at kexec load, even with the extra memory
>> allocated, might not be large enough for the entire measurement 
>> list.  The
>> indeterminate interval between kexec 'load' and 'execute' could 
>> exacerbate
>> this problem.
>>
>> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
>> measured as critical data at kexec 'load' and 'execute' respectively.
>> Report the allocated kexec segment size, IMA binary log size and the
>> runtime measurements count as part of those events.
>>
>> These events, and the values reported through them, serve as markers in
>> the IMA log to verify the IMA events are captured during kexec soft
>> reboot.  The presence of a 'kexec_load' event in between the last two
>> 'boot_aggregate' events in the IMA log implies this is a kexec soft
>> reboot, and not a cold-boot. And the absence of 'kexec_execute' event
>> after kexec soft reboot implies missing events in that window which
>> results in inconsistency with TPM PCR quotes, necessitating a cold boot
>> for a successful remote attestation.
>>
>> The 'kexec_load' event IMA log can be found using the following command:
>> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>>     grep kexec_load
>>
>> The 'kexec_load' event IMA log can be found using the following command:
>> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>>     grep kexec_execute
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima.h       |  6 ++++++
>>   security/integrity/ima/ima_kexec.c | 21 +++++++++++++++++++++
>>   security/integrity/ima/ima_queue.c |  5 +++++
>>   3 files changed, 32 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index 4428fcf42167..1452c98242a4 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -240,6 +240,12 @@ void ima_post_key_create_or_update(struct key 
>> *keyring, struct key *key,
>>                      unsigned long flags, bool create);
>>   #endif
>>   +#ifdef CONFIG_IMA_KEXEC
>> +void ima_measure_kexec_event(const char *event_name);
>> +#else
>> +static inline void ima_measure_kexec_event(const char *event_name) {}
>> +#endif
>> +
>>   /*
>>    * The default binary_runtime_measurements list format is defined 
>> as the
>>    * platform native format.  The canonical format is defined as 
>> little-endian.
>> diff --git a/security/integrity/ima/ima_kexec.c 
>> b/security/integrity/ima/ima_kexec.c
>> index 6c8c203ad81e..8d0782e51ffa 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 256
>> +
>>   static struct seq_file ima_kexec_file;
>>   static void *ima_kexec_buffer;
>>   static size_t kexec_segment_size;
>> @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct 
>> seq_file *sf)
>>       ima_reset_kexec_file(sf);
>>   }
>>   +void ima_measure_kexec_event(const char *event_name)
>> +{
>> +    char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
>> +    size_t buf_size = 0;
>> +    long len;
>> +
>> +    buf_size = ima_get_binary_runtime_size();
>> +    len = atomic_long_read(&ima_htable.len);
>> +
>> +    scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> +          "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
>> +         "ima_runtime_measurements_count=%ld;",
>> +         kexec_segment_size, buf_size, len);
>
> Indentation and n = scnprintf(...), as Mimi mentioned in v7.
>
>> +
>> +    ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event,
>> +                  strlen(ima_kexec_event), false, NULL, 0);
>
> Replace strlen(ima_kexec_event) with 'n'.
>
> With these fixes:
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>
>> +}
>> +
>>   static int ima_alloc_kexec_file_buf(size_t segment_size)
>>   {
>>       /*
>> @@ -58,6 +78,7 @@ static int ima_alloc_kexec_file_buf(size_t 
>> segment_size)
>>   out:
>>       ima_kexec_file.read_pos = 0;
>>       ima_kexec_file.count = sizeof(struct ima_kexec_hdr);    /* 
>> reserved space */
>> +    ima_measure_kexec_event("kexec_load");
>>         return 0;
>>   }
>> diff --git a/security/integrity/ima/ima_queue.c 
>> b/security/integrity/ima/ima_queue.c
>> index 3dfd178d4292..6afb46989cf6 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -241,6 +241,11 @@ static int ima_reboot_notifier(struct 
>> notifier_block *nb,
>>                      unsigned long action,
>>                      void *data)
>>   {
>> +#ifdef CONFIG_IMA_KEXEC
>> +    if (action == SYS_RESTART && data && !strcmp(data, "kexec reboot"))
>> +        ima_measure_kexec_event("kexec_execute");
>> +#endif
>> +
>>       ima_measurements_suspend();
>>         return NOTIFY_DONE;

Hi Stefan, thanks for your comments. I will update in next version.

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[kernel test robot]

Hi steven,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.14-rc3 next-20250219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/steven-chen/ima-define-and-call-ima_alloc_kexec_file_buf/20250219-065931
base:   linus/master
patch link:    https://lore.kernel.org/r/20250218225502.747963-3-chenste%40linux.microsoft.com
patch subject: [PATCH v8 2/7] kexec: define functions to map and unmap segments
config: x86_64-buildonly-randconfig-004-20250220 (https://download.01.org/0day-ci/archive/20250220/202502200848.MJEuphR1-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502200848.MJEuphR1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502200848.MJEuphR1-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/crash_dump.h:5,
                    from mm/mm_init.c:30:
>> include/linux/kexec.h:479:47: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
     479 | static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
         |                                               ^~~~~~


vim +479 include/linux/kexec.h

   466	
   467	#define kexec_dprintk(fmt, arg...) \
   468	        do { if (kexec_file_dbg_print) pr_info(fmt, ##arg); } while (0)
   469	
   470	extern void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size);
   471	extern void kimage_unmap_segment(void *buffer);
   472	#else /* !CONFIG_KEXEC_CORE */
   473	struct pt_regs;
   474	struct task_struct;
   475	static inline void __crash_kexec(struct pt_regs *regs) { }
   476	static inline void crash_kexec(struct pt_regs *regs) { }
   477	static inline int kexec_should_crash(struct task_struct *p) { return 0; }
   478	static inline int kexec_crash_loaded(void) { return 0; }
 > 479	static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
   480	{ return NULL; }
   481	static inline void kimage_unmap_segment(void *buffer) { }
   482	#define kexec_in_progress false
   483	#endif /* CONFIG_KEXEC_CORE */
   484	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v8 5/7] ima: kexec: move IMA log copy from kexec load to execute

[kernel test robot]

Hi steven,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.14-rc3 next-20250219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/steven-chen/ima-define-and-call-ima_alloc_kexec_file_buf/20250219-065931
base:   linus/master
patch link:    https://lore.kernel.org/r/20250218225502.747963-6-chenste%40linux.microsoft.com
patch subject: [PATCH v8 5/7] ima: kexec: move IMA log copy from kexec load to execute
config: x86_64-buildonly-randconfig-001-20250220 (https://download.01.org/0day-ci/archive/20250220/202502200920.rOyK1770-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502200920.rOyK1770-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502200920.rOyK1770-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/kexec_file.c: In function 'kimage_file_post_load':
>> kernel/kexec_file.c:206:9: error: implicit declaration of function 'ima_kexec_post_load'; did you mean 'machine_kexec_post_load'? [-Werror=implicit-function-declaration]
     206 |         ima_kexec_post_load(image);
         |         ^~~~~~~~~~~~~~~~~~~
         |         machine_kexec_post_load
   cc1: some warnings being treated as errors


vim +206 kernel/kexec_file.c

   203	
   204	static void kimage_file_post_load(struct kimage *image)
   205	{
 > 206		ima_kexec_post_load(image);
   207	}
   208	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v8 1/7] ima: define and call ima_alloc_kexec_file_buf

[Mimi Zohar]

On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
> Carrying the IMA measurement list across kexec requires allocating a
> buffer and copying the measurement records.  Separate allocating the
> buffer and copying the measurement records into separate functions in
> order to allocate the buffer at kexec 'load' and copy the measurements
> at kexec 'execute'.
> 
> This patch includes the following changes:
>  - Refactor ima_dump_measurement_list() to move the memory allocation
>    to a separate function ima_alloc_kexec_file_buf() which allocates
>    buffer of size 'kexec_segment_size' at kexec 'load'.
>  - Make the local variable ima_kexec_file in ima_dump_measurement_list()
>    a local static to the file, so that it can be accessed from 
>    ima_alloc_kexec_file_buf(). Compare actual memory required to ensure 
>    there is enough memory for the entire measurement record.
>  - Copy as many measurement events as possible.
>  - Make necessary changes to the function ima_add_kexec_buffer() to call
>    the above two functions.
>  - Compared the memory size allocated with memory size of the entire 
>    measurement record. If there is not enough memory, it will copy as many
>    IMA measurement records as possible, and this situation will result
>    in a failure of remote attestation.
> 
> Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>

Steven, thank you again for picking up this patch set.

As previously explained, there is no tag named "Author" in
https://www.kernel.org/doc/Documentation/process/submitting-patches.rst.  To give credit
to the original author use "Co-developed-by".  The "Co-developed-by:" tag is immediately
followed by the original author's "Signed-off-by:" tag.  Please refer to the document for
an example.

> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>

<--- "Co-developed-by:" would go here.
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
>  security/integrity/ima/ima.h       |   1 +
>  security/integrity/ima/ima_kexec.c | 102 +++++++++++++++++++++--------
>  security/integrity/ima/ima_queue.c |   4 +-
>  3 files changed, 77 insertions(+), 30 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 24d09ea91b87..4428fcf42167 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -274,6 +274,7 @@ 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);
> +int ima_get_binary_runtime_entry_size(struct ima_template_entry *entry);
>  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 9d45f4d26f73..89088f1fa989 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -15,63 +15,97 @@
>  #include "ima.h"
>  
>  #ifdef CONFIG_IMA_KEXEC
> +static struct seq_file ima_kexec_file;
> +
> +static void ima_reset_kexec_file(struct seq_file *sf)
> +{
> +	sf->buf = NULL;
> +	sf->size = 0;
> +	sf->read_pos = 0;
> +	sf->count = 0;
> +}
> +
> +static void ima_free_kexec_file_buf(struct seq_file *sf)
> +{
> +	vfree(sf->buf);
> +	ima_reset_kexec_file(sf);
> +}
> +
> +static int ima_alloc_kexec_file_buf(size_t segment_size)
> +{
> +	/*
> +	 * kexec 'load' may be called multiple times.
> +	 * Free and realloc the buffer only if the segment_size is
> +	 * changed from the previous kexec 'load' call.
> +	 */
> +	if (ima_kexec_file.buf && ima_kexec_file.size == segment_size)
> +		goto out;
> +
> +	ima_free_kexec_file_buf(&ima_kexec_file);
> +
> +	/* segment size can't change between kexec load and execute */
> +	ima_kexec_file.buf = vmalloc(segment_size);
> +	if (!ima_kexec_file.buf)
> +		return -ENOMEM;
> +
> +	ima_kexec_file.size = segment_size;
> +
> +out:
> +	ima_kexec_file.read_pos = 0;
> +	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
> +
> +	return 0;
> +}
> +

<--- ima_dump_measurement_list() function comment goes here.

>  static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>  				     unsigned long segment_size)
>  {
>  	struct ima_queue_entry *qe;
> -	struct seq_file file;
>  	struct ima_kexec_hdr khdr;
>  	int ret = 0;
> +	size_t entry_size = 0;
>  
> -	/* segment size can't change between kexec load and execute */
> -	file.buf = vmalloc(segment_size);
> -	if (!file.buf) {
> -		ret = -ENOMEM;
> -		goto out;
> +	if (!ima_kexec_file.buf) {
> +		pr_err("Kexec file buf not allocated\n");
> +		return -EINVAL;
>  	}
>  
> -	file.file = NULL;
> -	file.size = segment_size;
> -	file.read_pos = 0;
> -	file.count = sizeof(khdr);	/* reserved space */
> -
>  	memset(&khdr, 0, sizeof(khdr));
>  	khdr.version = 1;
>  	/* This is an append-only list, no need to hold the RCU read lock */
> +	/* Copy as many IMA measurements list records as possible */

Having two consecutive comments like this looks weird.  Please refer to section "8)
Commenting" of https://www.kernel.org/doc/Documentation/process/coding-style.rst. 

The first comment is particular to list_for_each_entry_rcu() and should remain here.  The
latter comment is more generic and should be included as part of a function comment.

>  	list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
> -		if (file.count < file.size) {
> +		entry_size += ima_get_binary_runtime_entry_size(qe->entry);
> +		if (entry_size <= segment_size) {

As much as possible splitting a function shouldn't change the existing code.  It makes it
harder to review.  This sort of change should be a separate patch with the Subject topic
line something like "ima: copy only complete measurement records across kexec".  Making
this change as the first patch in the patch set will also allow it to be backported.

>  			khdr.count++;
> -			ima_measurements_show(&file, qe);
> +			ima_measurements_show(&ima_kexec_file, qe);
>  		} else {
>  			ret = -EINVAL;
> +			pr_err("IMA log file is too big for Kexec buf\n");
>  			break;

We really DO want to "Copy as many IMA measurements list records as possible" as possible.
However, the code doesn't match the comment, since the caller of
ima_dump_measurement_list() treats -EINVAL as an error and bails.

>  		}
>  	}
>  
> -	if (ret < 0)
> -		goto out;
> -
>  	/*
>  	 * fill in reserved space with some buffer details
>  	 * (eg. version, buffer size, number of measurements)
>  	 */
> -	khdr.buffer_size = file.count;
> +	khdr.buffer_size = ima_kexec_file.count;
>  	if (ima_canonical_fmt) {
>  		khdr.version = cpu_to_le16(khdr.version);
>  		khdr.count = cpu_to_le64(khdr.count);
>  		khdr.buffer_size = cpu_to_le64(khdr.buffer_size);
>  	}
> -	memcpy(file.buf, &khdr, sizeof(khdr));
> +	memcpy(ima_kexec_file.buf, &khdr, sizeof(khdr));
>  
>  	print_hex_dump_debug("ima dump: ", DUMP_PREFIX_NONE, 16, 1,
> -			     file.buf, file.count < 100 ? file.count : 100,
> +			     ima_kexec_file.buf, ima_kexec_file.count < 100 ?
> +			     ima_kexec_file.count : 100,
>  			     true);
>  
> -	*buffer_size = file.count;
> -	*buffer = file.buf;
> -out:
> -	if (ret == -EINVAL)
> -		vfree(file.buf);
> +	*buffer_size = ima_kexec_file.count;
> +	*buffer = ima_kexec_file.buf;
> +
>  	return ret;
>  }
>  
> @@ -90,7 +124,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>  
>  	/* use more understandable variable names than defined in kbuf */
>  	void *kexec_buffer = NULL;
> -	size_t kexec_buffer_size;
> +	size_t kexec_buffer_size = 0;
>  	size_t kexec_segment_size;
>  	int ret;
>  
> @@ -110,13 +144,19 @@ void ima_add_kexec_buffer(struct kimage *image)
>  		return;
>  	}
>  
> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> -				  kexec_segment_size);
> -	if (!kexec_buffer) {
> +	ret = ima_alloc_kexec_file_buf(kexec_segment_size);
> +	if (ret < 0) {
>  		pr_err("Not enough memory for the kexec measurement buffer.\n");
>  		return;
>  	}
>  
> +	ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> +					kexec_segment_size);
> +	if (ret < 0) {
> +		pr_err("Failed to dump IMA measurements. Error:%d.\n", ret);
> +		return;
> +	}
> +

As mentioned above, we really do want to copy as many measurement records as possible
across kexec.

>  	kbuf.buffer = kexec_buffer;
>  	kbuf.bufsz = kexec_buffer_size;
>  	kbuf.memsz = kexec_segment_size;
> @@ -131,6 +171,12 @@ void ima_add_kexec_buffer(struct kimage *image)
>  	image->ima_buffer_size = kexec_segment_size;
>  	image->ima_buffer = kexec_buffer;
>  
> +	/*
> +	 * kexec owns kexec_buffer after kexec_add_buffer() is called
> +	 * and it will vfree() that buffer.
> +	 */
> +	ima_reset_kexec_file(&ima_kexec_file);
> +
>  	kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
>  		      kbuf.mem);
>  }
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 83d53824aa98..3dfd178d4292 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -78,7 +78,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8
> *digest_value,
>   * binary_runtime_measurement list entry, which contains a
>   * couple of variable length fields (e.g template name and data).
>   */
> -static int get_binary_runtime_size(struct ima_template_entry *entry)
> +int ima_get_binary_runtime_entry_size(struct ima_template_entry *entry)
>  {
>  	int size = 0;
>  
> @@ -122,7 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
>  	if (binary_runtime_size != ULONG_MAX) {
>  		int size;
>  
> -		size = get_binary_runtime_size(entry);
> +		size = ima_get_binary_runtime_entry_size(entry);
>  		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
>  		     binary_runtime_size + size : ULONG_MAX;
>  	}

This change would be included in the new first patch named something like "ima: copy only
complete measurement records across kexec".

Thanks,

Mimi

Re: [PATCH v8 1/7] ima: define and call ima_alloc_kexec_file_buf

[James Bottomley]

On Thu, 2025-02-20 at 09:53 -0500, Mimi Zohar wrote:
> On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
[...
> > Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> 
> Steven, thank you again for picking up this patch set.
> 
> As previously explained, there is no tag named "Author" in
> https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
> .  To give credit to the original author use "Co-developed-by".

Just on this, only use the co-developed-by if you actually *modified*
the patch.  If you're just transmitting the patch unmodified you can
give original author credit by including a 

From: original author <email>

Followed by a blank line at the beginning of the email.  That makes the
git author field contan whatever the From: line says.  You still need a
signoff from yourself in the original patch because you transmitted it.

Some people also consider minor modifications to be insufficient to
disturb the original copyright ownership and simply document what they
did in square brackets under their signoff, like this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b5d1e6ee761a109400e97ac6a1b91c57d0f6a43a

Regards,

James

Re: [PATCH v8 1/7] ima: define and call ima_alloc_kexec_file_buf

[Mimi Zohar]

On Thu, 2025-02-20 at 10:04 -0500, James Bottomley wrote:
> On Thu, 2025-02-20 at 09:53 -0500, Mimi Zohar wrote:
> > On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
> [...
> > > Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > 
> > Steven, thank you again for picking up this patch set.
> > 
> > As previously explained, there is no tag named "Author" in
> > https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
> > .  To give credit to the original author use "Co-developed-by".
> 
> Just on this, only use the co-developed-by if you actually *modified*
> the patch.  If you're just transmitting the patch unmodified you can
> give original author credit by including a 
> 
> From: original author <email>
> 
> Followed by a blank line at the beginning of the email.  That makes the
> git author field contan whatever the From: line says.  You still need a
> signoff from yourself in the original patch because you transmitted it.
> 
> Some people also consider minor modifications to be insufficient to
> disturb the original copyright ownership and simply document what they
> did in square brackets under their signoff, like this:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b5d1e6ee761a109400e97ac6a1b91c57d0f6a43a

Originally I had said:

   > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
   > Signed-off-by: steven chen <chenste@linux.microsoft.com>

   Before the "Co-developed-by" tag was defined, it was implied simply by this ordering
   of the "Signed-off-by" tags.
   
   For those patches you didn't modify, simply import Tushar's patch with him as the
   author and add your Signed-off-by tag after his.

Thanks, James, for the explanation of using "From: original author <email>" to force the
author to be Tushar.

Mimi

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Mimi Zohar]

Hi Steven,

On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
> Currently, the mechanism to map and unmap segments to the kimage
> structure is not available to the subsystems outside of kexec.  This
> functionality is needed when IMA is allocating the memory segments
> during kexec 'load' operation.  Implement functions to map and unmap
> segments to kimage.

Obviously up to now Kexec was mapping the segments. Missing from this patch description is
the reason "why" these functions are needed now.  It's not enough to say "is needed when
IMA is allocating the memory segments during kexec 'load' operation".  The question is why
does "IMA" need to allocate the memory segments.  Don't make the kexec/kexec_dump
maintainers guess.

Refer to the section "Describe your changes" in
https://www.kernel.org/doc/Documentation/process/submitting-patches.rst

> 
> Implement kimage_map_segment() to enable mapping of IMA buffer source
> pages to the kimage structure post kexec 'load'.  This function,
> accepting a kimage pointer, an address, and a size, will gather the
> source pages within the specified address range, create an array of page
> pointers, and map these to a contiguous virtual address range.  The
> function returns the start of this range if successful, or NULL if
> unsuccessful.
> 
> Implement kimage_unmap_segment() for unmapping segments
> using vunmap().
> 
> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>

Again, no such thing as an "Author" tag.  Refer to the comments on 1/7.

> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

As previously requested, please add the Cc's inline here and in all the kexec/kdump
related patches:

Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Baoquan He <bhe@redhat.com> 
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Dave Young <dyoung@redhat.com>

> Signed-off-by: steven chen <chenste@linux.microsoft.com>

thanks,

Mimi

Re: [PATCH v8 6/7] ima: make the kexec extra memory configurable

[Mimi Zohar]

On Tue, 2025-02-18 at 14:55 -0800, steven chen wrote:
> The extra memory allocated for carrying the IMA measurement list across
> kexec is hard-coded as half a PAGE.  Make it configurable.
> 
> Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
> extra memory (in kb) to be allocated for IMA measurements added during
> kexec soft reboot.  Ensure the default value of the option is set such
> that extra half a page of memory for additional measurements is allocated
> for the additional measurements.
> 
> Update ima_add_kexec_buffer() function to allocate memory based on the
> Kconfig option value, rather than the currently hard-coded one.
> 
> Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

Re: [PATCH v8 7/7] ima: measure kexec load and exec events as critical data

[Mimi Zohar]

On Tue, 2025-02-18 at 14:55 -0800, steven chen wrote:
> The amount of memory allocated at kexec load, even with the extra memory
> allocated, might not be large enough for the entire measurement list.  The
> indeterminate interval between kexec 'load' and 'execute' could exacerbate
> this problem.
> 
> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be 
> measured as critical data at kexec 'load' and 'execute' respectively.
> Report the allocated kexec segment size, IMA binary log size and the
> runtime measurements count as part of those events.
> 
> These events, and the values reported through them, serve as markers in
> the IMA log to verify the IMA events are captured during kexec soft
> reboot.  The presence of a 'kexec_load' event in between the last two
> 'boot_aggregate' events in the IMA log implies this is a kexec soft
> reboot, and not a cold-boot. And the absence of 'kexec_execute' event
> after kexec soft reboot implies missing events in that window which
> results in inconsistency with TPM PCR quotes, necessitating a cold boot
> for a successful remote attestation.
> 
> The 'kexec_load' event IMA log can be found using the following command:
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>    grep kexec_load
> 
> The 'kexec_load' event IMA log can be found using the following command:

-> kexec_execute

> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>    grep kexec_execute

These critical data events are displayed as hex encoded ascii in the
ascii_runtime_measurement_list.  Verifying the critical data hash requires calculating the
hash of the decoded ascii string.  For example:

sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep  kexec_load
| cut -d' ' -f 6 | xxd -r -p | sha256sum

> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
>  security/integrity/ima/ima.h       |  6 ++++++
>  security/integrity/ima/ima_kexec.c | 21 +++++++++++++++++++++
>  security/integrity/ima/ima_queue.c |  5 +++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 4428fcf42167..1452c98242a4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -240,6 +240,12 @@ void ima_post_key_create_or_update(struct key *keyring, struct key
> *key,
>  				   unsigned long flags, bool create);
>  #endif
>  
> +#ifdef CONFIG_IMA_KEXEC
> +void ima_measure_kexec_event(const char *event_name);
> +#else
> +static inline void ima_measure_kexec_event(const char *event_name) {}
> +#endif
> +
>  /*
>   * The default binary_runtime_measurements list format is defined as the
>   * platform native format.  The canonical format is defined as little-endian.
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 6c8c203ad81e..8d0782e51ffa 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 256
> +
>  static struct seq_file ima_kexec_file;
>  static void *ima_kexec_buffer;
>  static size_t kexec_segment_size;
> @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
>  	ima_reset_kexec_file(sf);
>  }
>  
> +void ima_measure_kexec_event(const char *event_name)
> +{
> +	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
> +	size_t buf_size = 0;
> +	long len;
> +
> +	buf_size = ima_get_binary_runtime_size();
> +	len = atomic_long_read(&ima_htable.len);
> +
> +	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
> +		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
> +		 "ima_runtime_measurements_count=%ld;",
> +		 kexec_segment_size, buf_size, len);
> +
> +	ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event,
> +				  strlen(ima_kexec_event), false, NULL, 0);

As previously mentioned, scnprintf() returns the length.  No need to use strlen() here.

Mimi

> +}
> +
>  static int ima_alloc_kexec_file_buf(size_t segment_size)
>  {
>  	/*
> @@ -58,6 +78,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
>  out:
>  	ima_kexec_file.read_pos = 0;
>  	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
> +	ima_measure_kexec_event("kexec_load");
>  
>  	return 0;
>  }
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 3dfd178d4292..6afb46989cf6 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -241,6 +241,11 @@ static int ima_reboot_notifier(struct notifier_block *nb,
>  			       unsigned long action,
>  			       void *data)
>  {
> +#ifdef CONFIG_IMA_KEXEC
> +	if (action == SYS_RESTART && data && !strcmp(data, "kexec reboot"))
> +		ima_measure_kexec_event("kexec_execute");
> +#endif
> +
>  	ima_measurements_suspend();
>  
>  	return NOTIFY_DONE;

Re: [PATCH v8 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot

[Mimi Zohar]

On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
> kexec_calculate_store_digests() calculates and stores the digest of the
> segment at kexec_file_load syscall where the IMA segment is also
> allocated.  With this series, the IMA segment will be updated with the
> measurement log at kexec execute stage when soft reboot is initiated. 
> Therefore, it may fail digest verification in verify_sha256_digest() 
> after kexec soft reboot into the new kernel. Therefore, the digest 
> calculation/verification of the IMA segment needs to be skipped.
> 
> Skip the calculating and storing digest of the IMA segment in
> kexec_calculate_store_digests() so that it is not added to the
> 'purgatory_sha_regions'.
> 
> Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
> no change is needed in verify_sha256_digest() in this context.
> 
> With this change, the IMA segment is not included in the digest
> calculation, storage, and verification.

Basically you're saying because the hash verification will fail, don't include
the IMA buffer.  What's missing is the reason for not caring whether the IMA
hash is included or not.

I understand this is the best we can do without making some major kexec changes.

> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

After updating the patch description,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  include/linux/kexec.h              |  3 +++
>  kernel/kexec_file.c                | 22 ++++++++++++++++++++++
>  security/integrity/ima/ima_kexec.c |  3 +++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 4dbf806bccef..bd554ced9fb2 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -362,6 +362,9 @@ struct kimage {
>  
>  	phys_addr_t ima_buffer_addr;
>  	size_t ima_buffer_size;
> +
> +	unsigned long ima_segment_index;
> +	bool is_ima_segment_index_set;
>  #endif
>  
>  	/* Core ELF header buffer */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3eedb8c226ad..606132253c79 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_IMA_KEXEC
> +static bool check_ima_segment_index(struct kimage *image, int i)
> +{
> +	if (image->is_ima_segment_index_set && i == image->ima_segment_index)
> +		return true;
> +	else
> +		return false;
> +}
> +#else
> +static bool check_ima_segment_index(struct kimage *image, int i)
> +{
> +	return false;
> +}
> +#endif
> +
>  static int kexec_calculate_store_digests(struct kimage *image);
>  
>  /* Maximum size in bytes for kernel/initrd files. */
> @@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage
> *image)
>  		if (ksegment->kbuf == pi->purgatory_buf)
>  			continue;
>  
> +		/*
> +		 * Skip the segment if ima_segment_index is set and matches
> +		 * the current index
> +		 */
> +		if (check_ima_segment_index(image, i))
> +			continue;
> +
>  		ret = crypto_shash_update(desc, ksegment->kbuf,
>  					  ksegment->bufsz);
>  		if (ret)
> diff --git a/security/integrity/ima/ima_kexec.c
> b/security/integrity/ima/ima_kexec.c
> index 89088f1fa989..704676fa6615 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -160,6 +160,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>  	kbuf.buffer = kexec_buffer;
>  	kbuf.bufsz = kexec_buffer_size;
>  	kbuf.memsz = kexec_segment_size;
> +	image->is_ima_segment_index_set = false;
>  	ret = kexec_add_buffer(&kbuf);
>  	if (ret) {
>  		pr_err("Error passing over kexec measurement buffer.\n");
> @@ -170,6 +171,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>  	image->ima_buffer_addr = kbuf.mem;
>  	image->ima_buffer_size = kexec_segment_size;
>  	image->ima_buffer = kexec_buffer;
> +	image->ima_segment_index = image->nr_segments - 1;
> +	image->is_ima_segment_index_set = true;
>  
>  	/*
>  	 * kexec owns kexec_buffer after kexec_add_buffer() is called

Re: [PATCH v8 4/7] ima: kexec: define functions to copy IMA log at soft boot

[Mimi Zohar]

On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
> IMA log is copied to the new Kernel during kexec 'load' using 
> ima_dump_measurement_list().  The log copy at kexec 'load' may result in
> loss of IMA measurements during kexec soft reboot.  It needs to be copied
> over during kexec 'execute'.  Setup the needed infrastructure to move the
> IMA log copy from kexec 'load' to 'execute'. 
> 
> Define a new IMA hook ima_update_kexec_buffer() as a stub function.
> It will be used to call ima_dump_measurement_list() during kexec 
> 'execute'.   
> 
> Implement ima_kexec_post_load() function to be invoked after the new 
> Kernel image has been loaded for kexec. ima_kexec_post_load() maps the 
> IMA buffer to a segment in the newly loaded Kernel.  It also registers 
> the reboot notifier_block to trigger ima_update_kexec_buffer() at 
> exec 'execute'.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
>  include/linux/ima.h                |  3 ++
>  security/integrity/ima/ima_kexec.c | 46 ++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0bae61a15b60..8e29cb4e6a01 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -32,6 +32,9 @@ static inline void ima_appraise_parse_cmdline(void) {}
>  
>  #ifdef CONFIG_IMA_KEXEC
>  extern void ima_add_kexec_buffer(struct kimage *image);
> +extern void ima_kexec_post_load(struct kimage *image);
> +#else
> +static inline void ima_kexec_post_load(struct kimage *image) {}
>  #endif
>  
>  #else
> diff --git a/security/integrity/ima/ima_kexec.c
> b/security/integrity/ima/ima_kexec.c
> index 704676fa6615..0fa65f91414b 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -12,10 +12,14 @@
>  #include <linux/kexec.h>
>  #include <linux/of.h>
>  #include <linux/ima.h>
> +#include <linux/reboot.h>
> +#include <asm/page.h>
>  #include "ima.h"
>  
>  #ifdef CONFIG_IMA_KEXEC
>  static struct seq_file ima_kexec_file;
> +static void *ima_kexec_buffer;
> +static bool ima_kexec_update_registered;
>  
>  static void ima_reset_kexec_file(struct seq_file *sf)
>  {
> @@ -183,6 +187,48 @@ void ima_add_kexec_buffer(struct kimage *image)
>  	kexec_dprintk("kexec measurement buffer for the loaded kernel at
> 0x%lx.\n",
>  		      kbuf.mem);
>  }
> +
> +/*
> + * Called during kexec execute so that IMA can update the measurement list.
> + */
> +static int ima_update_kexec_buffer(struct notifier_block *self,
> +				   unsigned long action, void *data)
> +{
> +	return NOTIFY_OK;
> +}
> +
> +struct notifier_block update_buffer_nb = {
> +	.notifier_call = ima_update_kexec_buffer,
> +};
> +
> +/*
> + * Create a mapping for the source pages that contain the IMA buffer
> + * so we can update it later.
> + */

Hi Steven,

It does more than just that.  It also registers a second IMA reboot notifier. 
(Is a second reboot notifier really necessary?)  It seems that the
ima_reboot_notifier() is executed after this one, otherwise the kexec_execute
would be missing.  However, I'm not sure that is guaranteed.

I'm wondering if this patch should be limited to saving the map segments.  In
any case, using the reboot notifier is relatively new and should at least be
reflected here and in the patch description.

thanks,

Mimi

> +void ima_kexec_post_load(struct kimage *image)
> +{
> +	if (ima_kexec_buffer) {
> +		kimage_unmap_segment(ima_kexec_buffer);
> +		ima_kexec_buffer = NULL;
> +	}
> +
> +	if (!image->ima_buffer_addr)
> +		return;
> +
> +	ima_kexec_buffer = kimage_map_segment(image,
> +					      image->ima_buffer_addr,
> +					      image->ima_buffer_size);
> +	if (!ima_kexec_buffer) {
> +		pr_err("Could not map measurements buffer.\n");
> +		return;
> +	}
> +
> +	if (!ima_kexec_update_registered) {
> +		register_reboot_notifier(&update_buffer_nb);
> +		ima_kexec_update_registered = true;
> +	}
> +}
> +
>  #endif /* IMA_KEXEC */
>  
>  /*

Re: [PATCH v8 4/7] ima: kexec: define functions to copy IMA log at soft boot

[Stefan Berger]

On 2/21/25 2:07 PM, Mimi Zohar wrote:
> On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
>> IMA log is copied to the new Kernel during kexec 'load' using
>> ima_dump_measurement_list().  The log copy at kexec 'load' may result in
>> loss of IMA measurements during kexec soft reboot.  It needs to be copied
>> over during kexec 'execute'.  Setup the needed infrastructure to move the
>> IMA log copy from kexec 'load' to 'execute'.
>>
>> Define a new IMA hook ima_update_kexec_buffer() as a stub function.
>> It will be used to call ima_dump_measurement_list() during kexec
>> 'execute'.
>>
>> Implement ima_kexec_post_load() function to be invoked after the new
>> Kernel image has been loaded for kexec. ima_kexec_post_load() maps the
>> IMA buffer to a segment in the newly loaded Kernel.  It also registers
>> the reboot notifier_block to trigger ima_update_kexec_buffer() at
>> exec 'execute'.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> ---
>>   include/linux/ima.h                |  3 ++
>>   security/integrity/ima/ima_kexec.c | 46 ++++++++++++++++++++++++++++++
>>   2 files changed, 49 insertions(+)
>>
>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>> index 0bae61a15b60..8e29cb4e6a01 100644
>> --- a/include/linux/ima.h
>> +++ b/include/linux/ima.h
>> @@ -32,6 +32,9 @@ static inline void ima_appraise_parse_cmdline(void) {}
>>   
>>   #ifdef CONFIG_IMA_KEXEC
>>   extern void ima_add_kexec_buffer(struct kimage *image);
>> +extern void ima_kexec_post_load(struct kimage *image);
>> +#else
>> +static inline void ima_kexec_post_load(struct kimage *image) {}
>>   #endif
>>   
>>   #else
>> diff --git a/security/integrity/ima/ima_kexec.c
>> b/security/integrity/ima/ima_kexec.c
>> index 704676fa6615..0fa65f91414b 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -12,10 +12,14 @@
>>   #include <linux/kexec.h>
>>   #include <linux/of.h>
>>   #include <linux/ima.h>
>> +#include <linux/reboot.h>
>> +#include <asm/page.h>
>>   #include "ima.h"
>>   
>>   #ifdef CONFIG_IMA_KEXEC
>>   static struct seq_file ima_kexec_file;
>> +static void *ima_kexec_buffer;
>> +static bool ima_kexec_update_registered;
>>   
>>   static void ima_reset_kexec_file(struct seq_file *sf)
>>   {
>> @@ -183,6 +187,48 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   	kexec_dprintk("kexec measurement buffer for the loaded kernel at
>> 0x%lx.\n",
>>   		      kbuf.mem);
>>   }
>> +
>> +/*
>> + * Called during kexec execute so that IMA can update the measurement list.
>> + */
>> +static int ima_update_kexec_buffer(struct notifier_block *self,
>> +				   unsigned long action, void *data)
>> +{
>> +	return NOTIFY_OK;
>> +}
>> +
>> +struct notifier_block update_buffer_nb = {
>> +	.notifier_call = ima_update_kexec_buffer,
>> +};
>> +
>> +/*
>> + * Create a mapping for the source pages that contain the IMA buffer
>> + * so we can update it later.
>> + */
> 
> Hi Steven,
> 
> It does more than just that.  It also registers a second IMA reboot notifier.
> (Is a second reboot notifier really necessary?)  It seems that the
> ima_reboot_notifier() is executed after this one, otherwise the kexec_execute
> would be missing.  However, I'm not sure that is guaranteed.

By setting .priority = -1 in update_buffer_nb we could ensure that it 
will be called after the other notifier that does the suspend, so we 
would be sure that logging suspends before dumping the log to the buffer.

> 
> I'm wondering if this patch should be limited to saving the map segments.  In
> any case, using the reboot notifier is relatively new and should at least be
> reflected here and in the patch description.
> 
> thanks,
> 
> Mimi
> 
>> +void ima_kexec_post_load(struct kimage *image)
>> +{
>> +	if (ima_kexec_buffer) {
>> +		kimage_unmap_segment(ima_kexec_buffer);
>> +		ima_kexec_buffer = NULL;
>> +	}
>> +
>> +	if (!image->ima_buffer_addr)
>> +		return;
>> +
>> +	ima_kexec_buffer = kimage_map_segment(image,
>> +					      image->ima_buffer_addr,
>> +					      image->ima_buffer_size);
>> +	if (!ima_kexec_buffer) {
>> +		pr_err("Could not map measurements buffer.\n");
>> +		return;
>> +	}
>> +
>> +	if (!ima_kexec_update_registered) {
>> +		register_reboot_notifier(&update_buffer_nb);
>> +		ima_kexec_update_registered = true;
>> +	}
>> +}
>> +
>>   #endif /* IMA_KEXEC */
>>   
>>   /*
> 

Re: [PATCH v8 1/7] ima: define and call ima_alloc_kexec_file_buf

[steven chen]

On 2/20/2025 8:23 AM, Mimi Zohar wrote:
> On Thu, 2025-02-20 at 10:04 -0500, James Bottomley wrote:
>> On Thu, 2025-02-20 at 09:53 -0500, Mimi Zohar wrote:
>>> On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
>> [...
>>>> Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>> Steven, thank you again for picking up this patch set.
>>>
>>> As previously explained, there is no tag named "Author" in
>>> https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
>>> .  To give credit to the original author use "Co-developed-by".
>> Just on this, only use the co-developed-by if you actually *modified*
>> the patch.  If you're just transmitting the patch unmodified you can
>> give original author credit by including a
>>
>> From: original author <email>
>>
>> Followed by a blank line at the beginning of the email.  That makes the
>> git author field contan whatever the From: line says.  You still need a
>> signoff from yourself in the original patch because you transmitted it.
>>
>> Some people also consider minor modifications to be insufficient to
>> disturb the original copyright ownership and simply document what they
>> did in square brackets under their signoff, like this:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b5d1e6ee761a109400e97ac6a1b91c57d0f6a43a
> Originally I had said:
>
>     > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>     > Signed-off-by: steven chen <chenste@linux.microsoft.com>
>
>     Before the "Co-developed-by" tag was defined, it was implied simply by this ordering
>     of the "Signed-off-by" tags.
>     
>     For those patches you didn't modify, simply import Tushar's patch with him as the
>     author and add your Signed-off-by tag after his.
>
> Thanks, James, for the explanation of using "From: original author <email>" to force the
> author to be Tushar.
>
> Mimi

Thanks, I will update in next version.

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[steven chen]

On 2/20/2025 9:22 AM, Mimi Zohar wrote:
> Hi Steven,
>
> On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
>> Currently, the mechanism to map and unmap segments to the kimage
>> structure is not available to the subsystems outside of kexec.  This
>> functionality is needed when IMA is allocating the memory segments
>> during kexec 'load' operation.  Implement functions to map and unmap
>> segments to kimage.
> Obviously up to now Kexec was mapping the segments. Missing from this patch description is
> the reason "why" these functions are needed now.  It's not enough to say "is needed when
> IMA is allocating the memory segments during kexec 'load' operation".  The question is why
> does "IMA" need to allocate the memory segments.  Don't make the kexec/kexec_dump
> maintainers guess.
>
> Refer to the section "Describe your changes" in
> https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
>
>> Implement kimage_map_segment() to enable mapping of IMA buffer source
>> pages to the kimage structure post kexec 'load'.  This function,
>> accepting a kimage pointer, an address, and a size, will gather the
>> source pages within the specified address range, create an array of page
>> pointers, and map these to a contiguous virtual address range.  The
>> function returns the start of this range if successful, or NULL if
>> unsuccessful.
>>
>> Implement kimage_unmap_segment() for unmapping segments
>> using vunmap().
>>
>> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Again, no such thing as an "Author" tag.  Refer to the comments on 1/7.
>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> As previously requested, please add the Cc's inline here and in all the kexec/kdump
> related patches:
>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> thanks,
>
> Mimi

Mimi, thanks. I will update in next version.

Steven

Re: [PATCH v8 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot

[steven chen]

On 2/21/2025 7:41 AM, Mimi Zohar wrote:
> On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
>> kexec_calculate_store_digests() calculates and stores the digest of the
>> segment at kexec_file_load syscall where the IMA segment is also
>> allocated.  With this series, the IMA segment will be updated with the
>> measurement log at kexec execute stage when soft reboot is initiated.
>> Therefore, it may fail digest verification in verify_sha256_digest()
>> after kexec soft reboot into the new kernel. Therefore, the digest
>> calculation/verification of the IMA segment needs to be skipped.
>>
>> Skip the calculating and storing digest of the IMA segment in
>> kexec_calculate_store_digests() so that it is not added to the
>> 'purgatory_sha_regions'.
>>
>> Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
>> no change is needed in verify_sha256_digest() in this context.
>>
>> With this change, the IMA segment is not included in the digest
>> calculation, storage, and verification.
> Basically you're saying because the hash verification will fail, don't include
> the IMA buffer.  What's missing is the reason for not caring whether the IMA
> hash is included or not.
>
> I understand this is the best we can do without making some major kexec changes.
>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> After updating the patch description,
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>
>> ---
>>   include/linux/kexec.h              |  3 +++
>>   kernel/kexec_file.c                | 22 ++++++++++++++++++++++
>>   security/integrity/ima/ima_kexec.c |  3 +++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 4dbf806bccef..bd554ced9fb2 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -362,6 +362,9 @@ struct kimage {
>>   
>>   	phys_addr_t ima_buffer_addr;
>>   	size_t ima_buffer_size;
>> +
>> +	unsigned long ima_segment_index;
>> +	bool is_ima_segment_index_set;
>>   #endif
>>   
>>   	/* Core ELF header buffer */
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 3eedb8c226ad..606132253c79 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
>>   }
>>   #endif
>>   
>> +#ifdef CONFIG_IMA_KEXEC
>> +static bool check_ima_segment_index(struct kimage *image, int i)
>> +{
>> +	if (image->is_ima_segment_index_set && i == image->ima_segment_index)
>> +		return true;
>> +	else
>> +		return false;
>> +}
>> +#else
>> +static bool check_ima_segment_index(struct kimage *image, int i)
>> +{
>> +	return false;
>> +}
>> +#endif
>> +
>>   static int kexec_calculate_store_digests(struct kimage *image);
>>   
>>   /* Maximum size in bytes for kernel/initrd files. */
>> @@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage
>> *image)
>>   		if (ksegment->kbuf == pi->purgatory_buf)
>>   			continue;
>>   
>> +		/*
>> +		 * Skip the segment if ima_segment_index is set and matches
>> +		 * the current index
>> +		 */
>> +		if (check_ima_segment_index(image, i))
>> +			continue;
>> +
>>   		ret = crypto_shash_update(desc, ksegment->kbuf,
>>   					  ksegment->bufsz);
>>   		if (ret)
>> diff --git a/security/integrity/ima/ima_kexec.c
>> b/security/integrity/ima/ima_kexec.c
>> index 89088f1fa989..704676fa6615 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -160,6 +160,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   	kbuf.buffer = kexec_buffer;
>>   	kbuf.bufsz = kexec_buffer_size;
>>   	kbuf.memsz = kexec_segment_size;
>> +	image->is_ima_segment_index_set = false;
>>   	ret = kexec_add_buffer(&kbuf);
>>   	if (ret) {
>>   		pr_err("Error passing over kexec measurement buffer.\n");
>> @@ -170,6 +171,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   	image->ima_buffer_addr = kbuf.mem;
>>   	image->ima_buffer_size = kexec_segment_size;
>>   	image->ima_buffer = kexec_buffer;
>> +	image->ima_segment_index = image->nr_segments - 1;
>> +	image->is_ima_segment_index_set = true;
>>   
>>   	/*
>>   	 * kexec owns kexec_buffer after kexec_add_buffer() is called

Thanks, Mimi. I will update in next version.

Re: [PATCH v8 7/7] ima: measure kexec load and exec events as critical data

[steven chen]

On 2/20/2025 4:46 PM, Mimi Zohar wrote:
> On Tue, 2025-02-18 at 14:55 -0800, steven chen wrote:
>> The amount of memory allocated at kexec load, even with the extra memory
>> allocated, might not be large enough for the entire measurement list.  The
>> indeterminate interval between kexec 'load' and 'execute' could exacerbate
>> this problem.
>>
>> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
>> measured as critical data at kexec 'load' and 'execute' respectively.
>> Report the allocated kexec segment size, IMA binary log size and the
>> runtime measurements count as part of those events.
>>
>> These events, and the values reported through them, serve as markers in
>> the IMA log to verify the IMA events are captured during kexec soft
>> reboot.  The presence of a 'kexec_load' event in between the last two
>> 'boot_aggregate' events in the IMA log implies this is a kexec soft
>> reboot, and not a cold-boot. And the absence of 'kexec_execute' event
>> after kexec soft reboot implies missing events in that window which
>> results in inconsistency with TPM PCR quotes, necessitating a cold boot
>> for a successful remote attestation.
>>
>> The 'kexec_load' event IMA log can be found using the following command:
>> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>>     grep kexec_load
>>
>> The 'kexec_load' event IMA log can be found using the following command:
> -> kexec_execute
>
>> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
>>     grep kexec_execute
> These critical data events are displayed as hex encoded ascii in the
> ascii_runtime_measurement_list.  Verifying the critical data hash requires calculating the
> hash of the decoded ascii string.  For example:
>
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep  kexec_load
> | cut -d' ' -f 6 | xxd -r -p | sha256sum
>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima.h       |  6 ++++++
>>   security/integrity/ima/ima_kexec.c | 21 +++++++++++++++++++++
>>   security/integrity/ima/ima_queue.c |  5 +++++
>>   3 files changed, 32 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index 4428fcf42167..1452c98242a4 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -240,6 +240,12 @@ void ima_post_key_create_or_update(struct key *keyring, struct key
>> *key,
>>   				   unsigned long flags, bool create);
>>   #endif
>>   
>> +#ifdef CONFIG_IMA_KEXEC
>> +void ima_measure_kexec_event(const char *event_name);
>> +#else
>> +static inline void ima_measure_kexec_event(const char *event_name) {}
>> +#endif
>> +
>>   /*
>>    * The default binary_runtime_measurements list format is defined as the
>>    * platform native format.  The canonical format is defined as little-endian.
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 6c8c203ad81e..8d0782e51ffa 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 256
>> +
>>   static struct seq_file ima_kexec_file;
>>   static void *ima_kexec_buffer;
>>   static size_t kexec_segment_size;
>> @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
>>   	ima_reset_kexec_file(sf);
>>   }
>>   
>> +void ima_measure_kexec_event(const char *event_name)
>> +{
>> +	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
>> +	size_t buf_size = 0;
>> +	long len;
>> +
>> +	buf_size = ima_get_binary_runtime_size();
>> +	len = atomic_long_read(&ima_htable.len);
>> +
>> +	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> +		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
>> +		 "ima_runtime_measurements_count=%ld;",
>> +		 kexec_segment_size, buf_size, len);
>> +
>> +	ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event,
>> +				  strlen(ima_kexec_event), false, NULL, 0);
> As previously mentioned, scnprintf() returns the length.  No need to use strlen() here.
>
> Mimi
>
>> +}
>> +
>>   static int ima_alloc_kexec_file_buf(size_t segment_size)
>>   {
>>   	/*
>> @@ -58,6 +78,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
>>   out:
>>   	ima_kexec_file.read_pos = 0;
>>   	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
>> +	ima_measure_kexec_event("kexec_load");
>>   
>>   	return 0;
>>   }
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> index 3dfd178d4292..6afb46989cf6 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -241,6 +241,11 @@ static int ima_reboot_notifier(struct notifier_block *nb,
>>   			       unsigned long action,
>>   			       void *data)
>>   {
>> +#ifdef CONFIG_IMA_KEXEC
>> +	if (action == SYS_RESTART && data && !strcmp(data, "kexec reboot"))
>> +		ima_measure_kexec_event("kexec_execute");
>> +#endif
>> +
>>   	ima_measurements_suspend();
>>   
>>   	return NOTIFY_DONE;

Thanks, I will update in next version

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Baoquan He]

Hi Steve, Mimi,

On 02/18/25 at 02:54pm, steven chen wrote:
> Currently, the mechanism to map and unmap segments to the kimage
> structure is not available to the subsystems outside of kexec.  This
> functionality is needed when IMA is allocating the memory segments
> during kexec 'load' operation.  Implement functions to map and unmap
> segments to kimage.

I am done with the whole patchset understanding. My concern is if this
TPM PCRs content can be carried over through newly introduced KHO. I can
see that these patchset doesn't introduce too much new code changes,
while if many conponents need do this, kexec reboot will be patched all
over its body and become ugly and hard to maintain.

Please check Mike Rapoport's v4 patchset to see if IMA can register
itself to KHO and do somthing during 2nd kernel init to restore those
TPM PCRs content to make sure all measurement logs are read correctly.
[PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)

Thanks
Baoquan

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[steven chen]

On 2/23/2025 10:14 PM, Baoquan He wrote:
> Hi Steve, Mimi,
>
> On 02/18/25 at 02:54pm, steven chen wrote:
>> Currently, the mechanism to map and unmap segments to the kimage
>> structure is not available to the subsystems outside of kexec.  This
>> functionality is needed when IMA is allocating the memory segments
>> during kexec 'load' operation.  Implement functions to map and unmap
>> segments to kimage.
> I am done with the whole patchset understanding. My concern is if this
> TPM PCRs content can be carried over through newly introduced KHO. I can
> see that these patchset doesn't introduce too much new code changes,
> while if many conponents need do this, kexec reboot will be patched all
> over its body and become ugly and hard to maintain.
>
> Please check Mike Rapoport's v4 patchset to see if IMA can register
> itself to KHO and do somthing during 2nd kernel init to restore those
> TPM PCRs content to make sure all measurement logs are read correctly.
> [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)
>
> Thanks
> Baoquan

Hi Baoquan,

For IMA, it appears that there are no current issues with TPM PCRs after 
a kernel soft reboot.

This patches is used to get currently missed IMA measurements during the 
kexec process copied to new kernel after the kernel soft reboot. I think 
it's ok to leave it at current location: it will be easy to maintain for 
IMA.

Overall, for these patches, do you see any major blockers for kexec?

If you have any specific concerns or need further details, please let me 
know.

Thanks,

Steven

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Baoquan He]

On 02/24/25 at 03:05pm, steven chen wrote:
> On 2/23/2025 10:14 PM, Baoquan He wrote:
> > Hi Steve, Mimi,
> > 
> > On 02/18/25 at 02:54pm, steven chen wrote:
> > > Currently, the mechanism to map and unmap segments to the kimage
> > > structure is not available to the subsystems outside of kexec.  This
> > > functionality is needed when IMA is allocating the memory segments
> > > during kexec 'load' operation.  Implement functions to map and unmap
> > > segments to kimage.
> > I am done with the whole patchset understanding. My concern is if this
> > TPM PCRs content can be carried over through newly introduced KHO. I can
> > see that these patchset doesn't introduce too much new code changes,
> > while if many conponents need do this, kexec reboot will be patched all
> > over its body and become ugly and hard to maintain.
> > 
> > Please check Mike Rapoport's v4 patchset to see if IMA can register
> > itself to KHO and do somthing during 2nd kernel init to restore those
> > TPM PCRs content to make sure all measurement logs are read correctly.
> > [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)
> > 
> > Thanks
> > Baoquan
> 
> Hi Baoquan,
> 
> For IMA, it appears that there are no current issues with TPM PCRs after a
> kernel soft reboot.

I mean using KHO to hold in 1st kernel and restore the IMA log in 2nd
kernel.

> 
> This patches is used to get currently missed IMA measurements during the
> kexec process copied to new kernel after the kernel soft reboot. I think
> it's ok to leave it at current location: it will be easy to maintain for
> IMA.

Yeah, but I am saying this patchset increase unnecessary code
complexity in kexec code maintaining.

> 
> Overall, for these patches, do you see any major blockers for kexec?
> 
> If you have any specific concerns or need further details, please let me
> know.

I have no concerns for this patchset implementation itself, I saw you using
vmap to maping the possible scattered source pages smartly and taking
the mapped buffer pointers to update later duing kexec jumping. That's very
great and clever method. BUT I am concerned about the solution, if we
can make use of the existed way of KHO to implement it more simply. Could
you please do investigation?

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[steven chen]

On 2/24/2025 4:18 PM, Baoquan He wrote:
> On 02/24/25 at 03:05pm, steven chen wrote:
>> On 2/23/2025 10:14 PM, Baoquan He wrote:
>>> Hi Steve, Mimi,
>>>
>>> On 02/18/25 at 02:54pm, steven chen wrote:
>>>> Currently, the mechanism to map and unmap segments to the kimage
>>>> structure is not available to the subsystems outside of kexec.  This
>>>> functionality is needed when IMA is allocating the memory segments
>>>> during kexec 'load' operation.  Implement functions to map and unmap
>>>> segments to kimage.
>>> I am done with the whole patchset understanding. My concern is if this
>>> TPM PCRs content can be carried over through newly introduced KHO. I can
>>> see that these patchset doesn't introduce too much new code changes,
>>> while if many conponents need do this, kexec reboot will be patched all
>>> over its body and become ugly and hard to maintain.
>>>
>>> Please check Mike Rapoport's v4 patchset to see if IMA can register
>>> itself to KHO and do somthing during 2nd kernel init to restore those
>>> TPM PCRs content to make sure all measurement logs are read correctly.
>>> [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)
>>>
>>> Thanks
>>> Baoquan
>> Hi Baoquan,
>>
>> For IMA, it appears that there are no current issues with TPM PCRs after a
>> kernel soft reboot.
> I mean using KHO to hold in 1st kernel and restore the IMA log in 2nd
> kernel.
>
>> This patches is used to get currently missed IMA measurements during the
>> kexec process copied to new kernel after the kernel soft reboot. I think
>> it's ok to leave it at current location: it will be easy to maintain for
>> IMA.
> Yeah, but I am saying this patchset increase unnecessary code
> complexity in kexec code maintaining.
>
>> Overall, for these patches, do you see any major blockers for kexec?
>>
>> If you have any specific concerns or need further details, please let me
>> know.
> I have no concerns for this patchset implementation itself, I saw you using
> vmap to maping the possible scattered source pages smartly and taking
> the mapped buffer pointers to update later duing kexec jumping. That's very
> great and clever method. BUT I am concerned about the solution, if we
> can make use of the existed way of KHO to implement it more simply. Could
> you please do investigation?

Hi Baoquan,

I will conduct an investigation. Thank you for your comments.

Steven

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Baoquan He]

On 02/25/25 at 10:35am, steven chen wrote:
> On 2/24/2025 4:18 PM, Baoquan He wrote:
> > On 02/24/25 at 03:05pm, steven chen wrote:
> > > On 2/23/2025 10:14 PM, Baoquan He wrote:
> > > > Hi Steve, Mimi,
> > > > 
> > > > On 02/18/25 at 02:54pm, steven chen wrote:
> > > > > Currently, the mechanism to map and unmap segments to the kimage
> > > > > structure is not available to the subsystems outside of kexec.  This
> > > > > functionality is needed when IMA is allocating the memory segments
> > > > > during kexec 'load' operation.  Implement functions to map and unmap
> > > > > segments to kimage.
> > > > I am done with the whole patchset understanding. My concern is if this
> > > > TPM PCRs content can be carried over through newly introduced KHO. I can
> > > > see that these patchset doesn't introduce too much new code changes,
> > > > while if many conponents need do this, kexec reboot will be patched all
> > > > over its body and become ugly and hard to maintain.
> > > > 
> > > > Please check Mike Rapoport's v4 patchset to see if IMA can register
> > > > itself to KHO and do somthing during 2nd kernel init to restore those
> > > > TPM PCRs content to make sure all measurement logs are read correctly.
> > > > [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)
> > > > 
> > > > Thanks
> > > > Baoquan
> > > Hi Baoquan,
> > > 
> > > For IMA, it appears that there are no current issues with TPM PCRs after a
> > > kernel soft reboot.
> > I mean using KHO to hold in 1st kernel and restore the IMA log in 2nd
> > kernel.
> > 
> > > This patches is used to get currently missed IMA measurements during the
> > > kexec process copied to new kernel after the kernel soft reboot. I think
> > > it's ok to leave it at current location: it will be easy to maintain for
> > > IMA.
> > Yeah, but I am saying this patchset increase unnecessary code
> > complexity in kexec code maintaining.
> > 
> > > Overall, for these patches, do you see any major blockers for kexec?
> > > 
> > > If you have any specific concerns or need further details, please let me
> > > know.
> > I have no concerns for this patchset implementation itself, I saw you using
> > vmap to maping the possible scattered source pages smartly and taking
> > the mapped buffer pointers to update later duing kexec jumping. That's very
> > great and clever method. BUT I am concerned about the solution, if we
> > can make use of the existed way of KHO to implement it more simply. Could
> > you please do investigation?
> 
> Hi Baoquan,
> 
> I will conduct an investigation. Thank you for your comments.

Thanks a lot, Steven.

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Mimi Zohar]

[Cc'ing Mike Rapoport]

On Mon, 2025-02-24 at 14:14 +0800, Baoquan He wrote:
> Hi Steve, Mimi,
> 
> On 02/18/25 at 02:54pm, steven chen wrote:
> > Currently, the mechanism to map and unmap segments to the kimage
> > structure is not available to the subsystems outside of kexec.  This
> > functionality is needed when IMA is allocating the memory segments
> > during kexec 'load' operation.  Implement functions to map and unmap
> > segments to kimage.
> 
> I am done with the whole patchset understanding. My concern is if this
> TPM PCRs content can be carried over through newly introduced KHO. I can
> see that these patchset doesn't introduce too much new code changes,
> while if many conponents need do this, kexec reboot will be patched all
> over its body and become ugly and hard to maintain.
> 
> Please check Mike Rapoport's v4 patchset to see if IMA can register
> itself to KHO and do somthing during 2nd kernel init to restore those
> TPM PCRs content to make sure all measurement logs are read correctly.
> [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)

Hi Baoquan,

I was hoping to look at Mike's patch set before responding, but perhaps it is
better to respond earlier rather than later with my initial thoughts.

The IMA measurement list isn't stored in contiguous memory, but has to be
marshalled before being carried across kexec, and then unmarshalled to restore
it after the kexec.  Roberto Sassu has been thinking about changing how the IMA
measurement list is stored so marshalling/unmarshalling wouldn't be necessary. 
Making both this change and using KHO going forward would be a good idea.

However, that sort of change wouldn't be appropriate to backport.  So the
question comes down to whether being unable to attest the measurement list,
because the measurements are copied too early at kexec load, but the TPM is
being extended through kexec exec, is considered a bug.  If that is the case,
then I suggest finish cleaning up and upstreaming this patch set so that it
could be backported.

thanks,

Mimi

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Baoquan He]

On 02/27/25 at 10:41am, Mimi Zohar wrote:
> [Cc'ing Mike Rapoport]
> 
> On Mon, 2025-02-24 at 14:14 +0800, Baoquan He wrote:
> > Hi Steve, Mimi,
> > 
> > On 02/18/25 at 02:54pm, steven chen wrote:
> > > Currently, the mechanism to map and unmap segments to the kimage
> > > structure is not available to the subsystems outside of kexec.  This
> > > functionality is needed when IMA is allocating the memory segments
> > > during kexec 'load' operation.  Implement functions to map and unmap
> > > segments to kimage.
> > 
> > I am done with the whole patchset understanding. My concern is if this
> > TPM PCRs content can be carried over through newly introduced KHO. I can
> > see that these patchset doesn't introduce too much new code changes,
> > while if many conponents need do this, kexec reboot will be patched all
> > over its body and become ugly and hard to maintain.
> > 
> > Please check Mike Rapoport's v4 patchset to see if IMA can register
> > itself to KHO and do somthing during 2nd kernel init to restore those
> > TPM PCRs content to make sure all measurement logs are read correctly.
> > [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)
> 
> Hi Baoquan,
> 
> I was hoping to look at Mike's patch set before responding, but perhaps it is
> better to respond earlier rather than later with my initial thoughts.
> 
> The IMA measurement list isn't stored in contiguous memory, but has to be
> marshalled before being carried across kexec, and then unmarshalled to restore
> it after the kexec.  Roberto Sassu has been thinking about changing how the IMA
> measurement list is stored so marshalling/unmarshalling wouldn't be necessary. 
> Making both this change and using KHO going forward would be a good idea.
> 
> However, that sort of change wouldn't be appropriate to backport.  So the
> question comes down to whether being unable to attest the measurement list,
> because the measurements are copied too early at kexec load, but the TPM is
> being extended through kexec exec, is considered a bug.  If that is the case,
> then I suggest finish cleaning up and upstreaming this patch set so that it
> could be backported.

Ah, I understand your concern. There are stable kernels or distros
kernels which need be taken care of. If then, we can continue to work on
polishing this patchset, as you have pointed out, there are still room
in this patchset to improve before merging.

Re: [PATCH v8 2/7] kexec: define functions to map and unmap segments

[Mimi Zohar]

On Fri, 2025-02-28 at 13:03 +0800, Baoquan He wrote:
> On 02/27/25 at 10:41am, Mimi Zohar wrote:
> > [Cc'ing Mike Rapoport]
> > 
> > On Mon, 2025-02-24 at 14:14 +0800, Baoquan He wrote:
> > > Hi Steve, Mimi,
> > > 
> > > On 02/18/25 at 02:54pm, steven chen wrote:
> > > > Currently, the mechanism to map and unmap segments to the kimage
> > > > structure is not available to the subsystems outside of kexec.  This
> > > > functionality is needed when IMA is allocating the memory segments
> > > > during kexec 'load' operation.  Implement functions to map and unmap
> > > > segments to kimage.
> > > 
> > > I am done with the whole patchset understanding. My concern is if this
> > > TPM PCRs content can be carried over through newly introduced KHO. I can
> > > see that these patchset doesn't introduce too much new code changes,
> > > while if many conponents need do this, kexec reboot will be patched all
> > > over its body and become ugly and hard to maintain.
> > > 
> > > Please check Mike Rapoport's v4 patchset to see if IMA can register
> > > itself to KHO and do somthing during 2nd kernel init to restore those
> > > TPM PCRs content to make sure all measurement logs are read correctly.
> > > [PATCH v4 00/14] kexec: introduce Kexec HandOver (KHO)
> > 
> > Hi Baoquan,
> > 
> > I was hoping to look at Mike's patch set before responding, but perhaps it is
> > better to respond earlier rather than later with my initial thoughts.
> > 
> > The IMA measurement list isn't stored in contiguous memory, but has to be
> > marshalled before being carried across kexec, and then unmarshalled to restore
> > it after the kexec.  Roberto Sassu has been thinking about changing how the IMA
> > measurement list is stored so marshalling/unmarshalling wouldn't be necessary. 
> > Making both this change and using KHO going forward would be a good idea.
> > 
> > However, that sort of change wouldn't be appropriate to backport.  So the
> > question comes down to whether being unable to attest the measurement list,
> > because the measurements are copied too early at kexec load, but the TPM is
> > being extended through kexec exec, is considered a bug.  If that is the case,
> > then I suggest finish cleaning up and upstreaming this patch set so that it
> > could be backported.
> 
> Ah, I understand your concern. There are stable kernels or distros
> kernels which need be taken care of. If then, we can continue to work on
> polishing this patchset, as you have pointed out, there are still room
> in this patchset to improve before merging.

Thanks, Baoquan!

I've already provided feedback on the IMA related patches.  Hopefully that will
be it.

Mimi