* [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute
@ 2025-03-04 19:03 steven chen
2025-03-04 19:03 ` [PATCH v9 1/7] ima: copy only complete measurement records across kexec steven chen
` (6 more replies)
0 siblings, 7 replies; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
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.
V7 of this series is available here[9] for reference.
V8 of this series is available here[10] 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/
[10] [PATCH v8 0/7] ima: kexec: measure events between kexec load and execute
https://lore.kernel.org/all/20250218225502.747963-1-chenste@linux.microsoft.com/
Change Log v9:
- Incorporated feedback from the community (Stefan Berger, Mimi Zohar,
and kernel test robot) on v8 of this series[10].
- Rebased the patch series to mainline 6.14.0-rc3.
- 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 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: copy only complete measurement records across kexec
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 | 9 ++
kernel/kexec_core.c | 54 ++++++++
kernel/kexec_file.c | 32 +++++
security/integrity/ima/Kconfig | 10 ++
security/integrity/ima/ima.h | 7 +
security/integrity/ima/ima_kexec.c | 210 ++++++++++++++++++++++++-----
security/integrity/ima/ima_queue.c | 9 +-
8 files changed, 297 insertions(+), 37 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-05 2:08 ` Mimi Zohar
2025-03-05 12:08 ` Baoquan He
2025-03-04 19:03 ` [PATCH v9 2/7] kexec: define functions to map and unmap segments steven chen
` (5 subsequent siblings)
6 siblings, 2 replies; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
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 only complete measurement records.
- 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. Copy only complete measurement records if there
is enough memory. If there is not enough memory, it will not copy
any IMA measurement records, and this situation will result in a
failure of remote attestation.
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 | 105 ++++++++++++++++++++++-------
security/integrity/ima/ima_queue.c | 4 +-
3 files changed, 83 insertions(+), 27 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..6195df128482 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -15,63 +15,106 @@
#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;
+}
+
+/*
+ * Copy the measurement list to the allocated memory
+ * compare the size of IMA measurement list with the size of the allocated memory
+ * if the size of the allocated memory is not less than the size of IMA measurement list
+ * copy the measurement list to the allocated memory.
+ * else return error
+ */
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 */
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;
+ *buffer_size = ima_kexec_file.count;
+ *buffer = ima_kexec_file.buf;
+
out:
- if (ret == -EINVAL)
- vfree(file.buf);
return ret;
}
@@ -90,7 +133,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 +153,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 +180,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v9 2/7] kexec: define functions to map and unmap segments
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
2025-03-04 19:03 ` [PATCH v9 1/7] ima: copy only complete measurement records across kexec steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-04 22:23 ` Jarkko Sakkinen
2025-03-06 6:35 ` Dan Carpenter
2025-03-04 19:03 ` [PATCH v9 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
` (4 subsequent siblings)
6 siblings, 2 replies; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The content of memory segments carried over to the new kernel during the
kexec systemcall can be changed at kexec 'execute' stage, but the size of
the memory segments cannot be changed at kexec 'execute' stage.
To copy IMA measurement logs during the kexec operation, IMA needs to
allocate memory at the kexec 'load' stage and map the segments to the
kimage structure. The mapped address will then be used to copy IMA
measurements during the kexec 'execute' stage.
Currently, the mechanism to map and unmap segments to the kimage
structure is not available to subsystems outside of kexec.
Implement kimage_map_segment() to enable IMA to map measurement log list to
the kimage structure during kexec 'load' stage. This function takes a kimage
pointer, a memory address, and a size, then gathers the
source pages within the specified address range, creates an array of page
pointers, and maps these to a contiguous virtual address range. The
function returns the start virtual address of this range if successful, or NULL on
failure.
Implement kimage_unmap_segment() for unmapping segments
using vunmap().
From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
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>
---
include/linux/kexec.h | 6 +++++
kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index f0e9f8eda7a3..7d6b12f8b8d0 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -467,13 +467,19 @@ 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;
+struct kimage;
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v9 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
2025-03-04 19:03 ` [PATCH v9 1/7] ima: copy only complete measurement records across kexec steven chen
2025-03-04 19:03 ` [PATCH v9 2/7] kexec: define functions to map and unmap segments steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-05 12:37 ` Baoquan He
2025-03-04 19:03 ` [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot steven chen
` (3 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The kexec_calculate_store_digests() function calculates and stores the
digest of the segment during the 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 the kexec execute stage when a soft reboot is initiated.
Therefore, the digests should be updated for the IMA segment in the
normal case.
The content of memory segments carried over to the new kernel during the
kexec systemcall can be changed at kexec 'execute' stage, but the size
and the location of the memory segments cannot be changed at kexec
'execute' stage.
However, during the kexec execute stage, if kexec_calculate_store_digests()
API is call to update the digest, it does not reuse the same memory segment
allocated during the kexec 'load' stage and the new memory segment required
cannot be transferred/mapped to the new kernel.
As a result, digest verification will fail in verify_sha256_digest()
after a kexec soft reboot into the new kernel. Therefore, the digest
calculation/verification of the IMA segment needs to be skipped.
To address this, skip the calculation and storage of the digest for 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>
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>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
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 7d6b12f8b8d0..107e726f2ef3 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 6195df128482..0465beca8867 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -169,6 +169,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");
@@ -179,6 +180,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
` (2 preceding siblings ...)
2025-03-04 19:03 ` [PATCH v9 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-12 8:57 ` kernel test robot
2025-03-04 19:03 ` [PATCH v9 5/7] ima: kexec: move IMA log copy from kexec load to execute steven chen
` (2 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The IMA log is currently copied to the new kernel during kexec 'load'
using ima_dump_measurement_list(). However, the log copied at kexec
'load' may result in loss of IMA measurements that only occurred after
kexec "load'. Therefore, the log needs to be copied 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
kexec 'execute'.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
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>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
include/linux/ima.h | 3 ++
security/integrity/ima/ima_kexec.c | 47 ++++++++++++++++++++++++++++++
2 files changed, 50 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 0465beca8867..074848dcd30f 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)
{
@@ -192,6 +196,49 @@ 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,
+ .priority = 1,
+};
+
+/*
+ * 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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v9 5/7] ima: kexec: move IMA log copy from kexec load to execute
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
` (3 preceding siblings ...)
2025-03-04 19:03 ` [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-04 19:03 ` [PATCH v9 6/7] ima: make the kexec extra memory configurable steven chen
2025-03-04 19:03 ` [PATCH v9 7/7] ima: measure kexec load and exec events as critical data steven chen
6 siblings, 0 replies; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
ima_dump_measurement_list() is called during kexec 'load', which may
result in loss of IMA measurements during kexec soft reboot. Due to
missed measurements that only occurred after kexec 'load', this function
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 so that it
becomes accessible both during kexec 'load' and 'execute'.
- Move ima_dump_measurement_list() call from ima_add_kexec_buffer()
to ima_update_kexec_buffer().
- Copy the measurement list as much as possible.
- 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' would corrupt the buffer.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
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>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
kernel/kexec_file.c | 10 +++++++
security/integrity/ima/ima_kexec.c | 48 ++++++++++++++++++------------
2 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 606132253c79..ab449b43aaee 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -201,6 +201,13 @@ kimage_validate_signature(struct kimage *image)
}
#endif
+static void kimage_file_post_load(struct kimage *image)
+{
+#ifdef CONFIG_IMA_KEXEC
+ ima_kexec_post_load(image);
+#endif
+}
+
/*
* In file mode list of segments is prepared by kernel. Copy relevant
* data from user space, do error checking, prepare segment list
@@ -428,6 +435,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 074848dcd30f..dd49658153ca 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)
@@ -66,7 +67,8 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
* compare the size of IMA measurement list with the size of the allocated memory
* if the size of the allocated memory is not less than the size of IMA measurement list
* copy the measurement list to the allocated memory.
- * else return error
+ * else
+ * copy the measurement list as much as possible.
*/
static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
unsigned long segment_size)
@@ -96,8 +98,6 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
}
}
- if (ret < 0)
- goto out;
/*
* fill in reserved space with some buffer details
* (eg. version, buffer size, number of measurements)
@@ -118,7 +118,6 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
*buffer_size = ima_kexec_file.count;
*buffer = ima_kexec_file.buf;
-out:
return ret;
}
@@ -138,7 +137,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;
/*
@@ -163,13 +161,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;
@@ -187,12 +178,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);
}
@@ -203,7 +188,32 @@ 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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v9 6/7] ima: make the kexec extra memory configurable
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
` (4 preceding siblings ...)
2025-03-04 19:03 ` [PATCH v9 5/7] ima: kexec: move IMA log copy from kexec load to execute steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-04 19:03 ` [PATCH v9 7/7] ima: measure kexec load and exec events as critical data steven chen
6 siblings, 0 replies; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
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>
---
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 dd49658153ca..9fb1bf5a592a 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -133,22 +133,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v9 7/7] ima: measure kexec load and exec events as critical data
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
` (5 preceding siblings ...)
2025-03-04 19:03 ` [PATCH v9 6/7] ima: make the kexec extra memory configurable steven chen
@ 2025-03-04 19:03 ` steven chen
2025-03-05 0:25 ` Mimi Zohar
6 siblings, 1 reply; 25+ messages in thread
From: steven chen @ 2025-03-04 19:03 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
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.
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, to verify the 'kexec_load' data hash:
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements
| grep kexec_load | cut -d' ' -f 6 | xxd -r -p | sha256sum
To verify the 'kexec_execute' data hash:
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements
| grep kexec_execute | 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>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
security/integrity/ima/ima.h | 6 ++++++
security/integrity/ima/ima_kexec.c | 20 ++++++++++++++++++++
security/integrity/ima/ima_queue.c | 5 +++++
3 files changed, 31 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 9fb1bf5a592a..e40c6da4504c 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,23 @@ 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);
+
+ int n = 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, n, false, NULL, 0);
+}
+
static int ima_alloc_kexec_file_buf(size_t segment_size)
{
/*
@@ -58,6 +77,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v9 2/7] kexec: define functions to map and unmap segments
2025-03-04 19:03 ` [PATCH v9 2/7] kexec: define functions to map and unmap segments steven chen
@ 2025-03-04 22:23 ` Jarkko Sakkinen
2025-03-05 0:55 ` steven chen
2025-03-06 6:35 ` Dan Carpenter
1 sibling, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2025-03-04 22:23 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, bhe, vgoyal, dyoung
On Tue, Mar 04, 2025 at 11:03:46AM -0800, steven chen wrote:
> The content of memory segments carried over to the new kernel during the
> kexec systemcall can be changed at kexec 'execute' stage, but the size of
> the memory segments cannot be changed at kexec 'execute' stage.
>
> To copy IMA measurement logs during the kexec operation, IMA needs to
> allocate memory at the kexec 'load' stage and map the segments to the
> kimage structure. The mapped address will then be used to copy IMA
> measurements during the kexec 'execute' stage.
>
> Currently, the mechanism to map and unmap segments to the kimage
> structure is not available to subsystems outside of kexec.
How does IMA work with kexec without having this? Just interested
(and confused).
>
> Implement kimage_map_segment() to enable IMA to map measurement log list to
> the kimage structure during kexec 'load' stage. This function takes a kimage
> pointer, a memory address, and a size, then gathers the
> source pages within the specified address range, creates an array of page
> pointers, and maps these to a contiguous virtual address range. The
> function returns the start virtual address of this range if successful, or NULL on
> failure.
>
> Implement kimage_unmap_segment() for unmapping segments
> using vunmap().
>
> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> 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>
> ---
> include/linux/kexec.h | 6 +++++
> kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index f0e9f8eda7a3..7d6b12f8b8d0 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -467,13 +467,19 @@ 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;
> +struct kimage;
> 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
>
>
BR, Jarkko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 7/7] ima: measure kexec load and exec events as critical data
2025-03-04 19:03 ` [PATCH v9 7/7] ima: measure kexec load and exec events as critical data steven chen
@ 2025-03-05 0:25 ` Mimi Zohar
2025-03-05 0:57 ` steven chen
0 siblings, 1 reply; 25+ messages in thread
From: Mimi Zohar @ 2025-03-05 0:25 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
Hi Steven,
On Tue, 2025-03-04 at 11:03 -0800, steven chen wrote:
> +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);
> +
> + int n = 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);
Variables should not be defined inline, but at the beginning of the function.
After doing that, scripts/checkpatch.pl complains about the formatting.
Mimi
> +
> + ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event, n, false, NULL, 0);
> +}
> +
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 2/7] kexec: define functions to map and unmap segments
2025-03-04 22:23 ` Jarkko Sakkinen
@ 2025-03-05 0:55 ` steven chen
2025-03-05 12:24 ` Baoquan He
0 siblings, 1 reply; 25+ messages in thread
From: steven chen @ 2025-03-05 0:55 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, bhe, vgoyal, dyoung
On 3/4/2025 2:23 PM, Jarkko Sakkinen wrote:
> On Tue, Mar 04, 2025 at 11:03:46AM -0800, steven chen wrote:
>> The content of memory segments carried over to the new kernel during the
>> kexec systemcall can be changed at kexec 'execute' stage, but the size of
>> the memory segments cannot be changed at kexec 'execute' stage.
>>
>> To copy IMA measurement logs during the kexec operation, IMA needs to
>> allocate memory at the kexec 'load' stage and map the segments to the
>> kimage structure. The mapped address will then be used to copy IMA
>> measurements during the kexec 'execute' stage.
>>
>> Currently, the mechanism to map and unmap segments to the kimage
>> structure is not available to subsystems outside of kexec.
> How does IMA work with kexec without having this? Just interested
> (and confused).
Currently, all IMA-related operations during a soft reboot, such as
memory allocation and IMA log list copy, are handled in the kexec 'load'
stage, so the map/unmap mechanism is not required.
The new design separates these two operations into different stages:
memory allocation remains in the kexec 'load' stage, while the IMA log
list copy is moved to the kexec 'execute' stage. Therefore, the
map/unmap mechanism is introduced.
Please refer to "[PATCH v9 0/7] ima: kexec: measure events between kexec
load and execute" for the reason why to add this.
Steven
>> Implement kimage_map_segment() to enable IMA to map measurement log list to
>> the kimage structure during kexec 'load' stage. This function takes a kimage
>> pointer, a memory address, and a size, then gathers the
>> source pages within the specified address range, creates an array of page
>> pointers, and maps these to a contiguous virtual address range. The
>> function returns the start virtual address of this range if successful, or NULL on
>> failure.
>>
>> Implement kimage_unmap_segment() for unmapping segments
>> using vunmap().
>>
>> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> 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>
>> ---
>> include/linux/kexec.h | 6 +++++
>> kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index f0e9f8eda7a3..7d6b12f8b8d0 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -467,13 +467,19 @@ 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;
>> +struct kimage;
>> 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
>>
>>
> BR, Jarkko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 7/7] ima: measure kexec load and exec events as critical data
2025-03-05 0:25 ` Mimi Zohar
@ 2025-03-05 0:57 ` steven chen
0 siblings, 0 replies; 25+ messages in thread
From: steven chen @ 2025-03-05 0:57 UTC (permalink / raw)
To: Mimi Zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On 3/4/2025 4:25 PM, Mimi Zohar wrote:
> Hi Steven,
>
> On Tue, 2025-03-04 at 11:03 -0800, steven chen wrote:
>> +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);
>> +
>> + int n = 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);
> Variables should not be defined inline, but at the beginning of the function.
> After doing that, scripts/checkpatch.pl complains about the formatting.
>
> Mimi
Hi Mimi,
I will update it in next release.
Thanks,
Steven
>> +
>> + ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event, n, false, NULL, 0);
>> +}
>> +
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-04 19:03 ` [PATCH v9 1/7] ima: copy only complete measurement records across kexec steven chen
@ 2025-03-05 2:08 ` Mimi Zohar
2025-03-05 11:34 ` Mimi Zohar
2025-03-05 12:08 ` Baoquan He
1 sibling, 1 reply; 25+ messages in thread
From: Mimi Zohar @ 2025-03-05 2:08 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Tue, 2025-03-04 at 11:03 -0800, steven chen wrote:
>
> - Compared the memory size allocated with memory size of the entire
> measurement record. Copy only complete measurement records if there
> is enough memory. If there is not enough memory, it will not copy
> any IMA measurement records, and this situation will result in a
> failure of remote attestation.
In discussions with Tushar, I was very clear that as many measurement records as
possible should be carried over to the kexec'ed kernel. The main change between
v8 and v9 was to make sure the last record copied was a complete record.
Mimi
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-05 2:08 ` Mimi Zohar
@ 2025-03-05 11:34 ` Mimi Zohar
0 siblings, 0 replies; 25+ messages in thread
From: Mimi Zohar @ 2025-03-05 11:34 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Tue, 2025-03-04 at 21:08 -0500, Mimi Zohar wrote:
> On Tue, 2025-03-04 at 11:03 -0800, steven chen wrote:
> >
> > - Compared the memory size allocated with memory size of the entire
> > measurement record. Copy only complete measurement records if there
> > is enough memory. If there is not enough memory, it will not copy
> > any IMA measurement records, and this situation will result in a
> > failure of remote attestation.
>
> In discussions with Tushar, I was very clear that as many measurement records as
> possible should be carried over to the kexec'ed kernel. The main change between
> v8 and v9 was to make sure the last record copied was a complete record.
Steven, let me clarify my comment on v8. The patch description said,
"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'."
The intention is fine, but it also did other things:
- only copied a full last measurement
- if there wasn't enough room, it didn't copy any measurement records.
Copying a full last measurement should be a separate, new patch to simplify
review. I'm asking you to separate that change from the rest of the patch, so
that it can be back ported independently of the rest of the patch set.
When splitting the function "that allocates the buffer and copies the
measurement records into separate functions", please make sure it still copies
as many measurement records as possible.
thanks,
Mimi
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-04 19:03 ` [PATCH v9 1/7] ima: copy only complete measurement records across kexec steven chen
2025-03-05 2:08 ` Mimi Zohar
@ 2025-03-05 12:08 ` Baoquan He
2025-03-05 12:27 ` Mimi Zohar
1 sibling, 1 reply; 25+ messages in thread
From: Baoquan He @ 2025-03-05 12:08 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 03/04/25 at 11:03am, 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:
I don't know why one patch need include so many changes. From below log,
it should be split into separate patches. It may not need to make one
patch to reflect one change, we should at least split and wrap several
kind of changes to ease patch understanding and reviewing. My personal
opinion.
> - 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 only complete measurement records.
> - 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. Copy only complete measurement records if there
> is enough memory. If there is not enough memory, it will not copy
> any IMA measurement records, and this situation will result in a
> failure of remote attestation.
>
> 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 | 105 ++++++++++++++++++++++-------
> security/integrity/ima/ima_queue.c | 4 +-
> 3 files changed, 83 insertions(+), 27 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..6195df128482 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -15,63 +15,106 @@
> #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;
> +}
> +
> +/*
> + * Copy the measurement list to the allocated memory
> + * compare the size of IMA measurement list with the size of the allocated memory
> + * if the size of the allocated memory is not less than the size of IMA measurement list
> + * copy the measurement list to the allocated memory.
> + * else return error
> + */
> 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 */
> 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;
> + *buffer_size = ima_kexec_file.count;
> + *buffer = ima_kexec_file.buf;
> +
> out:
> - if (ret == -EINVAL)
> - vfree(file.buf);
> return ret;
> }
>
> @@ -90,7 +133,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 +153,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 +180,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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 2/7] kexec: define functions to map and unmap segments
2025-03-05 0:55 ` steven chen
@ 2025-03-05 12:24 ` Baoquan He
2025-03-17 18:26 ` steven chen
0 siblings, 1 reply; 25+ messages in thread
From: Baoquan He @ 2025-03-05 12:24 UTC (permalink / raw)
To: steven chen
Cc: Jarkko Sakkinen, zohar, stefanb, roberto.sassu, roberto.sassu,
eric.snowberg, ebiederm, paul, code, bauermann, linux-integrity,
kexec, linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 03/04/25 at 04:55pm, steven chen wrote:
> On 3/4/2025 2:23 PM, Jarkko Sakkinen wrote:
> > On Tue, Mar 04, 2025 at 11:03:46AM -0800, steven chen wrote:
> > > The content of memory segments carried over to the new kernel during the
> > > kexec systemcall can be changed at kexec 'execute' stage, but the size of
> > > the memory segments cannot be changed at kexec 'execute' stage.
> > >
> > > To copy IMA measurement logs during the kexec operation, IMA needs to
> > > allocate memory at the kexec 'load' stage and map the segments to the
> > > kimage structure. The mapped address will then be used to copy IMA
> > > measurements during the kexec 'execute' stage.
> > >
> > > Currently, the mechanism to map and unmap segments to the kimage
> > > structure is not available to subsystems outside of kexec.
> > How does IMA work with kexec without having this? Just interested
> > (and confused).
> Currently, all IMA-related operations during a soft reboot, such as memory
> allocation and IMA log list copy, are handled in the kexec 'load' stage, so
> the map/unmap mechanism is not required.
>
> The new design separates these two operations into different stages: memory
> allocation remains in the kexec 'load' stage, while the IMA log list copy is
> moved to the kexec 'execute' stage. Therefore, the map/unmap mechanism is
> introduced.
I think the log can be improved. About the found problem and solution
part, we possible can describe them like below:
===
Currently, the kernel behaviour of kexec load is the IMA measurements
log is fetched from TPM PCRs and stored into buffer and hold. When
kexec reboot is triggered, the stored log buffer is carried over to the
2nd kernel. However, the time gap between kexec load and kexec reboot
could be very long. Then those new events extended into TPM PCRs during
the time window misses the chance to be carried over to 2nd kernel. This
results in mismatch between TPM PCR quotes and the actual IMA measurements
list after kexec reboot, which in turn results in remote attestation
failure.
To solve this problem, the new design is to defer the reading TPM PCRs
content out into kexec buffer to kexec reboot phase. While still
allocating the necessary buffer at kexec load time because it's not
appropriate to allocate memory at kexec reboot moment.
===
It may still need be improved, just for your reference. You can change
and add more detail needed and add them into your log.
>
> Please refer to "[PATCH v9 0/7] ima: kexec: measure events between kexec
> load and execute" for the reason why to add this.
>
> Steven
>
> > > Implement kimage_map_segment() to enable IMA to map measurement log list to
> > > the kimage structure during kexec 'load' stage. This function takes a kimage
> > > pointer, a memory address, and a size, then gathers the
> > > source pages within the specified address range, creates an array of page
> > > pointers, and maps these to a contiguous virtual address range. The
> > > function returns the start virtual address of this range if successful, or NULL on
> > > failure.
> > >
> > > Implement kimage_unmap_segment() for unmapping segments
> > > using vunmap().
> > >
> > > From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > 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>
> > > ---
> > > include/linux/kexec.h | 6 +++++
> > > kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 60 insertions(+)
> > >
> > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > index f0e9f8eda7a3..7d6b12f8b8d0 100644
> > > --- a/include/linux/kexec.h
> > > +++ b/include/linux/kexec.h
> > > @@ -467,13 +467,19 @@ 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;
> > > +struct kimage;
> > > 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;
When adding a new function, it's suggested to take the reverse xmas tree
style for local variable ordering usually.
> > > +
> > > + /*
> > > + * 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
> > >
> > >
> > BR, Jarkko
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-05 12:08 ` Baoquan He
@ 2025-03-05 12:27 ` Mimi Zohar
2025-03-06 22:45 ` steven chen
0 siblings, 1 reply; 25+ messages in thread
From: Mimi Zohar @ 2025-03-05 12:27 UTC (permalink / raw)
To: Baoquan He, steven chen
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On Wed, 2025-03-05 at 20:08 +0800, Baoquan He wrote:
> On 03/04/25 at 11:03am, 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:
>
> I don't know why one patch need include so many changes. From below log,
> it should be split into separate patches. It may not need to make one
> patch to reflect one change, we should at least split and wrap several
> kind of changes to ease patch understanding and reviewing. My personal
> opinion.
Agreed, well explained.
Mimi
>
> > - 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 only complete measurement records.
> > - 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. Copy only complete measurement records if there
> > is enough memory. If there is not enough memory, it will not copy
> > any IMA measurement records, and this situation will result in a
> > failure of remote attestation.
> >
> > 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>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot
2025-03-04 19:03 ` [PATCH v9 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
@ 2025-03-05 12:37 ` Baoquan He
0 siblings, 0 replies; 25+ messages in thread
From: Baoquan He @ 2025-03-05 12:37 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 03/04/25 at 11:03am, steven chen wrote:
> The kexec_calculate_store_digests() function calculates and stores the
> digest of the segment during the 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 the kexec execute stage when a soft reboot is initiated.
> Therefore, the digests should be updated for the IMA segment in the
> normal case.
>
> The content of memory segments carried over to the new kernel during the
> kexec systemcall can be changed at kexec 'execute' stage, but the size
> and the location of the memory segments cannot be changed at kexec
> 'execute' stage.
>
> However, during the kexec execute stage, if kexec_calculate_store_digests()
> API is call to update the digest, it does not reuse the same memory segment
~ called
> allocated during the kexec 'load' stage and the new memory segment required
> cannot be transferred/mapped to the new kernel.
>
> As a result, digest verification will fail in verify_sha256_digest()
> after a kexec soft reboot into the new kernel. Therefore, the digest
> calculation/verification of the IMA segment needs to be skipped.
>
> To address this, skip the calculation and storage of the digest for 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>
> 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>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> 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(+)
Other than the nit, LGTM.
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 7d6b12f8b8d0..107e726f2ef3 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 6195df128482..0465beca8867 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -169,6 +169,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");
> @@ -179,6 +180,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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 2/7] kexec: define functions to map and unmap segments
2025-03-04 19:03 ` [PATCH v9 2/7] kexec: define functions to map and unmap segments steven chen
2025-03-04 22:23 ` Jarkko Sakkinen
@ 2025-03-06 6:35 ` Dan Carpenter
1 sibling, 0 replies; 25+ messages in thread
From: Dan Carpenter @ 2025-03-06 6:35 UTC (permalink / raw)
To: oe-kbuild, steven chen, zohar, stefanb, roberto.sassu,
roberto.sassu, eric.snowberg, ebiederm, paul, code, bauermann,
linux-integrity, kexec, linux-security-module, linux-kernel
Cc: lkp, oe-kbuild-all, madvenka, nramas, James.Bottomley, bhe,
vgoyal, dyoung
Hi steven,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/steven-chen/ima-copy-only-complete-measurement-records-across-kexec/20250305-031719
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
patch link: https://lore.kernel.org/r/20250304190351.96975-3-chenste%40linux.microsoft.com
patch subject: [PATCH v9 2/7] kexec: define functions to map and unmap segments
config: x86_64-randconfig-161-20250306 (https://download.01.org/0day-ci/archive/20250306/202503061449.gbVGafZc-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202503061449.gbVGafZc-lkp@intel.com/
smatch warnings:
kernel/kexec_core.c:896 kimage_map_segment() error: uninitialized symbol 'dest_page_addr'.
vim +/dest_page_addr +896 kernel/kexec_core.c
bf06eab7ae0f04 steven chen 2025-03-04 870 void *kimage_map_segment(struct kimage *image,
bf06eab7ae0f04 steven chen 2025-03-04 871 unsigned long addr, unsigned long size)
bf06eab7ae0f04 steven chen 2025-03-04 872 {
bf06eab7ae0f04 steven chen 2025-03-04 873 unsigned long eaddr = addr + size;
bf06eab7ae0f04 steven chen 2025-03-04 874 unsigned long src_page_addr, dest_page_addr;
bf06eab7ae0f04 steven chen 2025-03-04 875 unsigned int npages;
bf06eab7ae0f04 steven chen 2025-03-04 876 struct page **src_pages;
bf06eab7ae0f04 steven chen 2025-03-04 877 int i;
bf06eab7ae0f04 steven chen 2025-03-04 878 kimage_entry_t *ptr, entry;
bf06eab7ae0f04 steven chen 2025-03-04 879 void *vaddr = NULL;
bf06eab7ae0f04 steven chen 2025-03-04 880
bf06eab7ae0f04 steven chen 2025-03-04 881 /*
bf06eab7ae0f04 steven chen 2025-03-04 882 * Collect the source pages and map them in a contiguous VA range.
bf06eab7ae0f04 steven chen 2025-03-04 883 */
bf06eab7ae0f04 steven chen 2025-03-04 884 npages = PFN_UP(eaddr) - PFN_DOWN(addr);
bf06eab7ae0f04 steven chen 2025-03-04 885 src_pages = kmalloc_array(npages, sizeof(*src_pages), GFP_KERNEL);
bf06eab7ae0f04 steven chen 2025-03-04 886 if (!src_pages) {
bf06eab7ae0f04 steven chen 2025-03-04 887 pr_err("Could not allocate ima pages array.\n");
bf06eab7ae0f04 steven chen 2025-03-04 888 return NULL;
bf06eab7ae0f04 steven chen 2025-03-04 889 }
bf06eab7ae0f04 steven chen 2025-03-04 890
bf06eab7ae0f04 steven chen 2025-03-04 891 i = 0;
bf06eab7ae0f04 steven chen 2025-03-04 892 for_each_kimage_entry(image, ptr, entry) {
bf06eab7ae0f04 steven chen 2025-03-04 893 if (entry & IND_DESTINATION) {
bf06eab7ae0f04 steven chen 2025-03-04 894 dest_page_addr = entry & PAGE_MASK;
Is the first entry always IND_DESTINATION?
bf06eab7ae0f04 steven chen 2025-03-04 895 } else if (entry & IND_SOURCE) {
bf06eab7ae0f04 steven chen 2025-03-04 @896 if (dest_page_addr >= addr && dest_page_addr < eaddr) {
^^^^^^^^^^^^^^
otherwise this is uninitialized
bf06eab7ae0f04 steven chen 2025-03-04 897 src_page_addr = entry & PAGE_MASK;
bf06eab7ae0f04 steven chen 2025-03-04 898 src_pages[i++] =
bf06eab7ae0f04 steven chen 2025-03-04 899 virt_to_page(__va(src_page_addr));
bf06eab7ae0f04 steven chen 2025-03-04 900 if (i == npages)
bf06eab7ae0f04 steven chen 2025-03-04 901 break;
bf06eab7ae0f04 steven chen 2025-03-04 902 dest_page_addr += PAGE_SIZE;
bf06eab7ae0f04 steven chen 2025-03-04 903 }
bf06eab7ae0f04 steven chen 2025-03-04 904 }
bf06eab7ae0f04 steven chen 2025-03-04 905 }
bf06eab7ae0f04 steven chen 2025-03-04 906
bf06eab7ae0f04 steven chen 2025-03-04 907 /* Sanity check. */
bf06eab7ae0f04 steven chen 2025-03-04 908 WARN_ON(i < npages);
bf06eab7ae0f04 steven chen 2025-03-04 909
bf06eab7ae0f04 steven chen 2025-03-04 910 vaddr = vmap(src_pages, npages, VM_MAP, PAGE_KERNEL);
bf06eab7ae0f04 steven chen 2025-03-04 911 kfree(src_pages);
bf06eab7ae0f04 steven chen 2025-03-04 912
bf06eab7ae0f04 steven chen 2025-03-04 913 if (!vaddr)
bf06eab7ae0f04 steven chen 2025-03-04 914 pr_err("Could not map ima buffer.\n");
bf06eab7ae0f04 steven chen 2025-03-04 915
bf06eab7ae0f04 steven chen 2025-03-04 916 return vaddr;
bf06eab7ae0f04 steven chen 2025-03-04 917 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-05 12:27 ` Mimi Zohar
@ 2025-03-06 22:45 ` steven chen
2025-03-07 2:51 ` Mimi Zohar
0 siblings, 1 reply; 25+ messages in thread
From: steven chen @ 2025-03-06 22:45 UTC (permalink / raw)
To: Mimi Zohar, Baoquan He
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 3/5/2025 4:27 AM, Mimi Zohar wrote:
> On Wed, 2025-03-05 at 20:08 +0800, Baoquan He wrote:
>> On 03/04/25 at 11:03am, 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:
>> I don't know why one patch need include so many changes. From below log,
>> it should be split into separate patches. It may not need to make one
>> patch to reflect one change, we should at least split and wrap several
>> kind of changes to ease patch understanding and reviewing. My personal
>> opinion.
> Agreed, well explained.
>
> Mimi
>
>>> - 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 only complete measurement records.
>>> - 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. Copy only complete measurement records if there
>>> is enough memory. If there is not enough memory, it will not copy
>>> any IMA measurement records, and this situation will result in a
>>> failure of remote attestation.
>>>
>>> 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>
I will split this patch into the following two patches:
ima: define and call ima_alloc_kexec_file_buf
ima: copy measurement records as much as possible across kexec
Thanks,
Steven
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-06 22:45 ` steven chen
@ 2025-03-07 2:51 ` Mimi Zohar
2025-03-11 12:44 ` Mimi Zohar
0 siblings, 1 reply; 25+ messages in thread
From: Mimi Zohar @ 2025-03-07 2:51 UTC (permalink / raw)
To: steven chen, Baoquan He
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On Thu, 2025-03-06 at 14:45 -0800, steven chen wrote:
> On 3/5/2025 4:27 AM, Mimi Zohar wrote:
> > On Wed, 2025-03-05 at 20:08 +0800, Baoquan He wrote:
> > > On 03/04/25 at 11:03am, 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:
> > > I don't know why one patch need include so many changes. From below log,
> > > it should be split into separate patches. It may not need to make one
> > > patch to reflect one change, we should at least split and wrap several
> > > kind of changes to ease patch understanding and reviewing. My personal
> > > opinion.
> > Agreed, well explained.
> >
> > Mimi
> >
> > > > - 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 only complete measurement records.
> > > > - 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. Copy only complete measurement records if there
> > > > is enough memory. If there is not enough memory, it will not copy
> > > > any IMA measurement records, and this situation will result in a
> > > > failure of remote attestation.
> > > >
> > > > 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>
>
> I will split this patch into the following two patches:
>
> ima: define and call ima_alloc_kexec_file_buf
> ima: copy measurement records as much as possible across kexec
Steven, breaking up code into patches is in order to simplify patch review.
This is done by limiting each patch to a single "logical change" [1]. For
example, the change below has nothing to do with "separate allocating the buffer
and copying the measurement records into separate functions".
/* This is an append-only list, no need to hold the RCU read lock */
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;
}
}
The original code potentially copied a partial last measurement record, not a
complete measurement record. For ease of review, the above change is fine, but
it needs to be a separate patch.
Patches:
1. ima: copy only complete measurement records across kexec
2. ima: define and call ima_alloc_kexec_file_buf()
The original code copied as many measurement records as possible. Please do not
change it.
thanks,
Mimi
[1] Refer to the section "Separate your changes" in
https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-07 2:51 ` Mimi Zohar
@ 2025-03-11 12:44 ` Mimi Zohar
2025-03-11 23:45 ` steven chen
0 siblings, 1 reply; 25+ messages in thread
From: Mimi Zohar @ 2025-03-11 12:44 UTC (permalink / raw)
To: steven chen, Baoquan He
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On Thu, 2025-03-06 at 21:51 -0500, Mimi Zohar wrote:
> On Thu, 2025-03-06 at 14:45 -0800, steven chen wrote:
> > On 3/5/2025 4:27 AM, Mimi Zohar wrote:
> > > On Wed, 2025-03-05 at 20:08 +0800, Baoquan He wrote:
> > > > On 03/04/25 at 11:03am, 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:
> > > > I don't know why one patch need include so many changes. From below log,
> > > > it should be split into separate patches. It may not need to make one
> > > > patch to reflect one change, we should at least split and wrap several
> > > > kind of changes to ease patch understanding and reviewing. My personal
> > > > opinion.
> > > Agreed, well explained.
> > >
> > > Mimi
> > >
> > > > > - 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 only complete measurement records.
> > > > > - 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. Copy only complete measurement records if there
> > > > > is enough memory. If there is not enough memory, it will not copy
> > > > > any IMA measurement records, and this situation will result in a
> > > > > failure of remote attestation.
> > > > >
> > > > > 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>
> >
> > I will split this patch into the following two patches:
> >
> > ima: define and call ima_alloc_kexec_file_buf
> > ima: copy measurement records as much as possible across kexec
>
> Steven, breaking up code into patches is in order to simplify patch review.
> This is done by limiting each patch to a single "logical change" [1]. For
> example, the change below has nothing to do with "separate allocating the buffer
> and copying the measurement records into separate functions".
>
> /* This is an append-only list, no need to hold the RCU read lock */
> 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;
> }
> }
>
> The original code potentially copied a partial last measurement record, not a
> complete measurement record. For ease of review, the above change is fine, but
> it needs to be a separate patch.
>
> Patches:
> 1. ima: copy only complete measurement records across kexec
> 2. ima: define and call ima_alloc_kexec_file_buf()
Steven,
The alternative would be to revert using ima_get_binary_runtime_entry_size() and
simply use "ima_kexec_file.count < ima_kexec_file.size". Only
ima_kexec_file.size would be initialized in ima_alloc_kexec_buf(). The rest
would remain in ima_dump_measurement_list(). get_binary_runtime_size() wouldn't
need to be made global.
To further simplify the patch review, first define a separate patch to just
rename the seq_file "file" to "ima_kexec_file".
Mimi
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 1/7] ima: copy only complete measurement records across kexec
2025-03-11 12:44 ` Mimi Zohar
@ 2025-03-11 23:45 ` steven chen
0 siblings, 0 replies; 25+ messages in thread
From: steven chen @ 2025-03-11 23:45 UTC (permalink / raw)
To: Mimi Zohar, Baoquan He
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 3/11/2025 5:44 AM, Mimi Zohar wrote:
> On Thu, 2025-03-06 at 21:51 -0500, Mimi Zohar wrote:
>> On Thu, 2025-03-06 at 14:45 -0800, steven chen wrote:
>>> On 3/5/2025 4:27 AM, Mimi Zohar wrote:
>>>> On Wed, 2025-03-05 at 20:08 +0800, Baoquan He wrote:
>>>>> On 03/04/25 at 11:03am, 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:
>>>>> I don't know why one patch need include so many changes. From below log,
>>>>> it should be split into separate patches. It may not need to make one
>>>>> patch to reflect one change, we should at least split and wrap several
>>>>> kind of changes to ease patch understanding and reviewing. My personal
>>>>> opinion.
>>>> Agreed, well explained.
>>>>
>>>> Mimi
>>>>
>>>>>> - 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 only complete measurement records.
>>>>>> - 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. Copy only complete measurement records if there
>>>>>> is enough memory. If there is not enough memory, it will not copy
>>>>>> any IMA measurement records, and this situation will result in a
>>>>>> failure of remote attestation.
>>>>>>
>>>>>> 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>
>>> I will split this patch into the following two patches:
>>>
>>> ima: define and call ima_alloc_kexec_file_buf
>>> ima: copy measurement records as much as possible across kexec
>> Steven, breaking up code into patches is in order to simplify patch review.
>> This is done by limiting each patch to a single "logical change" [1]. For
>> example, the change below has nothing to do with "separate allocating the buffer
>> and copying the measurement records into separate functions".
>>
>> /* This is an append-only list, no need to hold the RCU read lock */
>> 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;
>> }
>> }
>>
>> The original code potentially copied a partial last measurement record, not a
>> complete measurement record. For ease of review, the above change is fine, but
>> it needs to be a separate patch.
>>
>> Patches:
>> 1. ima: copy only complete measurement records across kexec
>> 2. ima: define and call ima_alloc_kexec_file_buf()
> Steven,
>
> The alternative would be to revert using ima_get_binary_runtime_entry_size() and
> simply use "ima_kexec_file.count < ima_kexec_file.size". Only
> ima_kexec_file.size would be initialized in ima_alloc_kexec_buf(). The rest
> would remain in ima_dump_measurement_list(). get_binary_runtime_size() wouldn't
> need to be made global.
>
> To further simplify the patch review, first define a separate patch to just
> rename the seq_file "file" to "ima_kexec_file".
>
> Mimi
Hi Mimi,
I will work on it.
Thanks,
Steven
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot
2025-03-04 19:03 ` [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot steven chen
@ 2025-03-12 8:57 ` kernel test robot
0 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2025-03-12 8:57 UTC (permalink / raw)
To: steven chen, zohar, stefanb, roberto.sassu, roberto.sassu,
eric.snowberg, ebiederm, paul, code, bauermann, linux-integrity,
kexec, linux-security-module, linux-kernel
Cc: oe-kbuild-all, madvenka, nramas, James.Bottomley, bhe, vgoyal,
dyoung
Hi steven,
kernel test robot noticed the following build warnings:
[auto build test WARNING on zohar-integrity/next-integrity]
[also build test WARNING on linus/master v6.14-rc6 next-20250311]
[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-copy-only-complete-measurement-records-across-kexec/20250305-031719
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
patch link: https://lore.kernel.org/r/20250304190351.96975-5-chenste%40linux.microsoft.com
patch subject: [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot
config: powerpc64-randconfig-r133-20250312 (https://download.01.org/0day-ci/archive/20250312/202503121600.IMBKp2gC-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250312/202503121600.IMBKp2gC-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/202503121600.IMBKp2gC-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
security/integrity/ima/ima_kexec.c:107:30: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [addressable] [assigned] [usertype] version @@ got restricted __le16 [usertype] @@
security/integrity/ima/ima_kexec.c:107:30: sparse: expected unsigned short [addressable] [assigned] [usertype] version
security/integrity/ima/ima_kexec.c:107:30: sparse: got restricted __le16 [usertype]
security/integrity/ima/ima_kexec.c:108:28: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [addressable] [assigned] [usertype] count @@ got restricted __le64 [usertype] @@
security/integrity/ima/ima_kexec.c:108:28: sparse: expected unsigned long long [addressable] [assigned] [usertype] count
security/integrity/ima/ima_kexec.c:108:28: sparse: got restricted __le64 [usertype]
security/integrity/ima/ima_kexec.c:109:34: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [addressable] [assigned] [usertype] buffer_size @@ got restricted __le64 [usertype] @@
security/integrity/ima/ima_kexec.c:109:34: sparse: expected unsigned long long [addressable] [assigned] [usertype] buffer_size
security/integrity/ima/ima_kexec.c:109:34: sparse: got restricted __le64 [usertype]
>> security/integrity/ima/ima_kexec.c:209:23: sparse: sparse: symbol 'update_buffer_nb' was not declared. Should it be static?
vim +/update_buffer_nb +209 security/integrity/ima/ima_kexec.c
208
> 209 struct notifier_block update_buffer_nb = {
210 .notifier_call = ima_update_kexec_buffer,
211 .priority = 1,
212 };
213
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v9 2/7] kexec: define functions to map and unmap segments
2025-03-05 12:24 ` Baoquan He
@ 2025-03-17 18:26 ` steven chen
0 siblings, 0 replies; 25+ messages in thread
From: steven chen @ 2025-03-17 18:26 UTC (permalink / raw)
To: Baoquan He
Cc: Jarkko Sakkinen, zohar, stefanb, roberto.sassu, roberto.sassu,
eric.snowberg, ebiederm, paul, code, bauermann, linux-integrity,
kexec, linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 3/5/2025 4:24 AM, Baoquan He wrote:
> On 03/04/25 at 04:55pm, steven chen wrote:
>> On 3/4/2025 2:23 PM, Jarkko Sakkinen wrote:
>>> On Tue, Mar 04, 2025 at 11:03:46AM -0800, steven chen wrote:
>>>> The content of memory segments carried over to the new kernel during the
>>>> kexec systemcall can be changed at kexec 'execute' stage, but the size of
>>>> the memory segments cannot be changed at kexec 'execute' stage.
>>>>
>>>> To copy IMA measurement logs during the kexec operation, IMA needs to
>>>> allocate memory at the kexec 'load' stage and map the segments to the
>>>> kimage structure. The mapped address will then be used to copy IMA
>>>> measurements during the kexec 'execute' stage.
>>>>
>>>> Currently, the mechanism to map and unmap segments to the kimage
>>>> structure is not available to subsystems outside of kexec.
>>> How does IMA work with kexec without having this? Just interested
>>> (and confused).
>> Currently, all IMA-related operations during a soft reboot, such as memory
>> allocation and IMA log list copy, are handled in the kexec 'load' stage, so
>> the map/unmap mechanism is not required.
>>
>> The new design separates these two operations into different stages: memory
>> allocation remains in the kexec 'load' stage, while the IMA log list copy is
>> moved to the kexec 'execute' stage. Therefore, the map/unmap mechanism is
>> introduced.
> I think the log can be improved. About the found problem and solution
> part, we possible can describe them like below:
>
> ===
> Currently, the kernel behaviour of kexec load is the IMA measurements
> log is fetched from TPM PCRs and stored into buffer and hold. When
> kexec reboot is triggered, the stored log buffer is carried over to the
> 2nd kernel. However, the time gap between kexec load and kexec reboot
> could be very long. Then those new events extended into TPM PCRs during
> the time window misses the chance to be carried over to 2nd kernel. This
> results in mismatch between TPM PCR quotes and the actual IMA measurements
> list after kexec reboot, which in turn results in remote attestation
> failure.
>
> To solve this problem, the new design is to defer the reading TPM PCRs
> content out into kexec buffer to kexec reboot phase. While still
> allocating the necessary buffer at kexec load time because it's not
> appropriate to allocate memory at kexec reboot moment.
> ===
>
> It may still need be improved, just for your reference. You can change
> and add more detail needed and add them into your log.
>
>> Please refer to "[PATCH v9 0/7] ima: kexec: measure events between kexec
>> load and execute" for the reason why to add this.
>>
>> Steven
>>
>>>> Implement kimage_map_segment() to enable IMA to map measurement log list to
>>>> the kimage structure during kexec 'load' stage. This function takes a kimage
>>>> pointer, a memory address, and a size, then gathers the
>>>> source pages within the specified address range, creates an array of page
>>>> pointers, and maps these to a contiguous virtual address range. The
>>>> function returns the start virtual address of this range if successful, or NULL on
>>>> failure.
>>>>
>>>> Implement kimage_unmap_segment() for unmapping segments
>>>> using vunmap().
>>>>
>>>> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>> 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>
>>>> ---
>>>> include/linux/kexec.h | 6 +++++
>>>> kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 60 insertions(+)
>>>>
>>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>>> index f0e9f8eda7a3..7d6b12f8b8d0 100644
>>>> --- a/include/linux/kexec.h
>>>> +++ b/include/linux/kexec.h
>>>> @@ -467,13 +467,19 @@ 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;
>>>> +struct kimage;
>>>> 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;
> When adding a new function, it's suggested to take the reverse xmas tree
> style for local variable ordering usually.
>
>>>> +
>>>> + /*
>>>> + * 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
>>>>
>>>>
>>> BR, Jarkko
>>
Hi Baoquan,
Thanks for your comments. I will update it in next version.
Steven
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2025-03-17 18:26 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-04 19:03 [PATCH v9 0/7] ima: kexec: measure events between kexec load and execute steven chen
2025-03-04 19:03 ` [PATCH v9 1/7] ima: copy only complete measurement records across kexec steven chen
2025-03-05 2:08 ` Mimi Zohar
2025-03-05 11:34 ` Mimi Zohar
2025-03-05 12:08 ` Baoquan He
2025-03-05 12:27 ` Mimi Zohar
2025-03-06 22:45 ` steven chen
2025-03-07 2:51 ` Mimi Zohar
2025-03-11 12:44 ` Mimi Zohar
2025-03-11 23:45 ` steven chen
2025-03-04 19:03 ` [PATCH v9 2/7] kexec: define functions to map and unmap segments steven chen
2025-03-04 22:23 ` Jarkko Sakkinen
2025-03-05 0:55 ` steven chen
2025-03-05 12:24 ` Baoquan He
2025-03-17 18:26 ` steven chen
2025-03-06 6:35 ` Dan Carpenter
2025-03-04 19:03 ` [PATCH v9 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
2025-03-05 12:37 ` Baoquan He
2025-03-04 19:03 ` [PATCH v9 4/7] ima: kexec: define functions to copy IMA log at soft boot steven chen
2025-03-12 8:57 ` kernel test robot
2025-03-04 19:03 ` [PATCH v9 5/7] ima: kexec: move IMA log copy from kexec load to execute steven chen
2025-03-04 19:03 ` [PATCH v9 6/7] ima: make the kexec extra memory configurable steven chen
2025-03-04 19:03 ` [PATCH v9 7/7] ima: measure kexec load and exec events as critical data steven chen
2025-03-05 0:25 ` Mimi Zohar
2025-03-05 0:57 ` steven chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).