[PATCH v2 0/3] ima: keep kexec dump state tied to installed image

[PATCH v2 0/3] ima: keep kexec dump state tied to installed image

[Jérémy Jean]

kexec_file_load() can change the global IMA buffer size or destination
mapping before the new image is installed. If the load fails, the old
image remains installed with this mismatched state. Unload and legacy
replacement can also free pages without clearing the mapping. The later
measurement copy can then overwrite the installed image or freed pages.

In this v2:
* Patch 1/3 updates IMA only after the machine post-load hook succeeds.
* Patch 2/3 keeps the dump size tied to the installed image.
* Patch 3/3 checks mapping ownership and clear stale state on unload or legacy
  replacement.

Jérémy Jean (3):
  kexec_file: update IMA after machine post-load
  ima: keep kexec dump state tied to installed image
  ima: clear kexec state when replacing file image

 include/linux/ima.h                |  1 +
 kernel/kexec.c                     | 11 ++++
 kernel/kexec_file.c                | 15 ++++-
 security/integrity/ima/ima_kexec.c | 98 +++++++++++++++++++-----------
 4 files changed, 88 insertions(+), 37 deletions(-)

[PATCH v2 1/3] kexec_file: update IMA after machine post-load

[Jérémy Jean]

ima_kexec_post_load() replaces the global IMA mapping with a mapping for
the new image. If machine_kexec_post_load() fails, the new image is
freed but that mapping remains. The installed image no longer receives
the execute-time IMA update, and the notifier can write the
measurement list into freed pages.

Call the machine hook first. Return its error before updating IMA so IMA
changes the mapping only for an image which will be installed.

Fixes: 9f0ec4b16f2b ("ima: kexec: move IMA log copy from kexec load to execute")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 kernel/kexec_file.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b668ce8d68ee..59f9ba1b0cd7 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -206,11 +206,17 @@ kimage_validate_signature(struct kimage *image)
 
 static int kexec_post_load(struct kimage *image, unsigned long flags)
 {
+	int ret;
+
+	ret = machine_kexec_post_load(image);
+	if (ret)
+		return ret;
+
 #ifdef CONFIG_IMA_KEXEC
 	if (!(flags & KEXEC_FILE_ON_CRASH))
 		ima_kexec_post_load(image);
 #endif
-	return machine_kexec_post_load(image);
+	return 0;
 }
 
 /*

Re: [PATCH v2 1/3] kexec_file: update IMA after machine post-load

[sashiko-bot]

> ima_kexec_post_load() replaces the global IMA mapping with a mapping for
> the new image. If machine_kexec_post_load() fails, the new image is
> freed but that mapping remains. The installed image no longer receives
> the execute-time IMA update, and the notifier can write the
> measurement list into freed pages.
> 
> Call the machine hook first. Return its error before updating IMA so IMA
> changes the mapping only for an image which will be installed.
> 
> Fixes: 9f0ec4b16f2b ("ima: kexec: move IMA log copy from kexec load to execute")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112125246.2715174-2-Jeremy.Jean@oss.cyber.gouv.fr?part=1

[PATCH v2 2/3] ima: keep kexec dump state tied to installed image

[Jérémy Jean]

ima_add_kexec_buffer() updates the global measurement scratch buffer
and segment size for the new image. If kexec_file_load() fails, the
new image is freed but the scratch buffer and segment size remain. When
kexec starts the installed image, ima_update_kexec_buffer() dumps the
measurement list into the IMA segment. A larger failed segment causes an
OOB write, while a smaller one truncates the log and reports the wrong
size in the kexec_execute event. KASAN for instsance reports the OOB
writes as:

  BUG: KASAN: vmalloc-out-of-bounds in ima_update_kexec_buffer+0x265/0x280
  Write of size 868426 at addr ffffc9000004e000 by task init/1
  ...
  __asan_memcpy+0x3b/0x60
  ima_update_kexec_buffer+0x265/0x280

Leave installed scratch state unchanged until all later load steps pass.
Store new buffers in the candidate and reuse only equal-size installed
buffers. Emit kexec_load before those steps with the local size.
Allocation failure still omits the optional segment.

Fixes: 9f0ec4b16f2b ("ima: kexec: move IMA log copy from kexec load to execute")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 security/integrity/ima/ima_kexec.c | 74 ++++++++++++++++--------------
 1 file changed, 39 insertions(+), 35 deletions(-)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 0d845693a1f7..3aa8b1096f81 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -35,7 +35,8 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
 	sf->count = 0;
 }
 
-void ima_measure_kexec_event(const char *event_name)
+static void ima_measure_kexec_event_size(const char *event_name,
+					 size_t segment_size)
 {
 	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
 	size_t buf_size = 0;
@@ -48,35 +49,26 @@ void ima_measure_kexec_event(const char *event_name)
 	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);
+		      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)
+void ima_measure_kexec_event(const char *event_name)
 {
-	/*
-	 * 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_measure_kexec_event_size(event_name, kexec_segment_size);
+}
 
-	ima_free_kexec_file_buf(&ima_kexec_file);
+static int ima_alloc_kexec_file_buf(struct kimage *image, size_t segment_size)
+{
+	if (ima_kexec_file.buf && ima_kexec_file.size == segment_size)
+		return 0;
 
 	/* segment size can't change between kexec load and execute */
-	ima_kexec_file.buf = vmalloc(segment_size);
-	if (!ima_kexec_file.buf)
+	image->ima_buffer = vmalloc(segment_size);
+	if (!image->ima_buffer)
 		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 */
-	ima_measure_kexec_event("kexec_load");
-
 	return 0;
 }
 
@@ -160,10 +152,8 @@ void ima_add_kexec_buffer(struct kimage *image)
 				  .top_down = true };
 	unsigned long binary_runtime_size;
 	unsigned long extra_memory;
+	size_t segment_size;
 
-	/* use more understandable variable names than defined in kbuf */
-	size_t kexec_buffer_size = 0;
-	void *kexec_buffer = NULL;
 	int ret;
 
 	if (image->type == KEXEC_TYPE_CRASH)
@@ -182,36 +172,36 @@ void ima_add_kexec_buffer(struct kimage *image)
 			      extra_memory;
 
 	if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
-		kexec_segment_size = ULONG_MAX;
+		segment_size = ULONG_MAX;
 	else
-		kexec_segment_size = ALIGN(binary_runtime_size, PAGE_SIZE);
+		segment_size = ALIGN(binary_runtime_size, PAGE_SIZE);
 
-	if ((kexec_segment_size == ULONG_MAX) ||
-	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
+	if (segment_size == ULONG_MAX ||
+	    (segment_size >> PAGE_SHIFT) > totalram_pages() / 2) {
 		pr_err("Binary measurement list too large.\n");
 		return;
 	}
 
-	ret = ima_alloc_kexec_file_buf(kexec_segment_size);
+	ret = ima_alloc_kexec_file_buf(image, segment_size);
 	if (ret < 0) {
 		pr_err("Not enough memory for the kexec measurement buffer.\n");
 		return;
 	}
 
-	kbuf.buffer = kexec_buffer;
-	kbuf.bufsz = kexec_buffer_size;
-	kbuf.memsz = kexec_segment_size;
+	ima_measure_kexec_event_size("kexec_load", segment_size);
+
+	kbuf.memsz = 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");
-		vfree(kexec_buffer);
+		vfree(image->ima_buffer);
+		image->ima_buffer = NULL;
 		return;
 	}
 
 	image->ima_buffer_addr = kbuf.mem;
-	image->ima_buffer_size = kexec_segment_size;
-	image->ima_buffer = kexec_buffer;
+	image->ima_buffer_size = segment_size;
 	image->ima_segment_index = image->nr_segments - 1;
 	image->is_ima_segment_index_set = true;
 
@@ -269,8 +259,22 @@ void ima_kexec_post_load(struct kimage *image)
 		ima_kexec_buffer = NULL;
 	}
 
-	if (!image->ima_buffer_addr)
+	if (!image->ima_buffer_addr) {
+		kexec_segment_size = 0;
+		ima_free_kexec_file_buf(&ima_kexec_file);
 		return;
+	}
+
+	if (image->ima_buffer) {
+		ima_free_kexec_file_buf(&ima_kexec_file);
+		ima_kexec_file.buf = image->ima_buffer;
+		image->ima_buffer = NULL;
+	}
+
+	kexec_segment_size = image->ima_buffer_size;
+	ima_kexec_file.size = image->ima_buffer_size;
+	ima_kexec_file.read_pos = 0;
+	ima_kexec_file.count = sizeof(struct ima_kexec_hdr);	/* reserved space */
 
 	ima_kexec_buffer = kimage_map_segment(image, image->ima_segment_index);
 	if (!ima_kexec_buffer) {

Re: [PATCH v2 2/3] ima: keep kexec dump state tied to installed image

[sashiko-bot]

> ima_add_kexec_buffer() updates the global measurement scratch buffer
> and segment size for the new image. If kexec_file_load() fails, the
> new image is freed but the scratch buffer and segment size remain. When
> kexec starts the installed image, ima_update_kexec_buffer() dumps the
> measurement list into the IMA segment. A larger failed segment causes an
> OOB write, while a smaller one truncates the log and reports the wrong
> size in the kexec_execute event. KASAN for instsance reports the OOB
> writes as:
> 
>   BUG: KASAN: vmalloc-out-of-bounds in ima_update_kexec_buffer+0x265/0x280
>   Write of size 868426 at addr ffffc9000004e000 by task init/1
>   ...
>   __asan_memcpy+0x3b/0x60
>   ima_update_kexec_buffer+0x265/0x280
> 
> Leave installed scratch state unchanged until all later load steps pass.
> Store new buffers in the candidate and reuse only equal-size installed
> buffers. Emit kexec_load before those steps with the local size.
> Allocation failure still omits the optional segment.
> 
> Fixes: 9f0ec4b16f2b ("ima: kexec: move IMA log copy from kexec load to execute")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112125246.2715174-2-Jeremy.Jean@oss.cyber.gouv.fr?part=2

[PATCH v2 3/3] ima: clear kexec state when replacing file image

[Jérémy Jean]

ima_kexec_buffer remains mapped to the IMA segment of the installed
kexec_file_load image. KEXEC_FILE_UNLOAD and a successful normal
kexec_load can replace and free that image without clearing the mapping.
Executing a later kexec_load image can then write the measurement list
into the freed pages. The new image has an IMA segment size of zero, but
size does not establish ownership of the mapping.

Record the image whose segment is mapped and verify it before the
execute-time update. Clear the mapping and scratch state under kexec_lock
when a normal image is removed without another file image taking its
place.

Fixes: 9f0ec4b16f2b ("ima: kexec: move IMA log copy from kexec load to execute")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 include/linux/ima.h                |  1 +
 kernel/kexec.c                     | 11 +++++++++++
 kernel/kexec_file.c                |  7 ++++++-
 security/integrity/ima/ima_kexec.c | 24 ++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 8e08baf16c2f..d61b7b7756b7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -34,6 +34,7 @@ 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);
+void ima_kexec_cleanup(void);
 #else
 static inline void ima_kexec_post_load(struct kimage *image) {}
 #endif
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 90756dc6339b..297f4657869b 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -11,6 +11,7 @@
 #include <linux/file.h>
 #include <linux/security.h>
 #include <linux/kexec.h>
+#include <linux/ima.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
 #include <linux/syscalls.h>
@@ -114,6 +115,10 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 
 	if (nr_segments == 0) {
 		/* Uninstall image */
+#ifdef CONFIG_IMA_KEXEC
+		if (!(flags & KEXEC_ON_CRASH))
+			ima_kexec_cleanup();
+#endif
 		kimage_free(xchg(dest_image, NULL));
 		ret = 0;
 		goto out_unlock;
@@ -163,6 +168,12 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 	if (ret)
 		goto out;
 
+	/* A kexec_load image has no IMA measurement segment. */
+#ifdef CONFIG_IMA_KEXEC
+	if (!(flags & KEXEC_ON_CRASH))
+		ima_kexec_cleanup();
+#endif
+
 	/* Install the new kernel and uninstall the old */
 	image = xchg(dest_image, image);
 
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 59f9ba1b0cd7..95b3a9ab1e3b 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -401,8 +401,13 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 #endif
 		dest_image = &kexec_image;
 
-	if (flags & KEXEC_FILE_UNLOAD)
+	if (flags & KEXEC_FILE_UNLOAD) {
+#ifdef CONFIG_IMA_KEXEC
+		if (image_type == KEXEC_TYPE_DEFAULT)
+			ima_kexec_cleanup();
+#endif
 		goto exchange;
+	}
 
 	/*
 	 * In case of crash, new kernel gets loaded in reserved region. It is
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 3aa8b1096f81..cc15aed8df48 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -25,6 +25,7 @@ static bool ima_kexec_update_registered;
 static struct seq_file ima_kexec_file;
 static size_t kexec_segment_size;
 static void *ima_kexec_buffer;
+static struct kimage *ima_kexec_image;
 
 static void ima_free_kexec_file_buf(struct seq_file *sf)
 {
@@ -229,6 +230,11 @@ static int ima_update_kexec_buffer(struct notifier_block *self,
 		return ret;
 	}
 
+	if (ima_kexec_image != kexec_image) {
+		pr_err("Kexec buffer does not match current image.\n");
+		return ret;
+	}
+
 	ret = ima_dump_measurement_list(&buf_size, &buf, kexec_segment_size);
 
 	if (ret)
@@ -239,6 +245,7 @@ static int ima_update_kexec_buffer(struct notifier_block *self,
 
 	kimage_unmap_segment(ima_kexec_buffer);
 	ima_kexec_buffer = NULL;
+	ima_kexec_image = NULL;
 
 	return ret;
 }
@@ -258,6 +265,7 @@ void ima_kexec_post_load(struct kimage *image)
 		kimage_unmap_segment(ima_kexec_buffer);
 		ima_kexec_buffer = NULL;
 	}
+	ima_kexec_image = image;
 
 	if (!image->ima_buffer_addr) {
 		kexec_segment_size = 0;
@@ -288,6 +296,22 @@ void ima_kexec_post_load(struct kimage *image)
 	}
 }
 
+/*
+ * Drop the dump state before its kexec image is replaced.
+ *
+ * This function assumes that kexec_lock is held.
+ */
+void ima_kexec_cleanup(void)
+{
+	if (ima_kexec_buffer) {
+		kimage_unmap_segment(ima_kexec_buffer);
+		ima_kexec_buffer = NULL;
+	}
+	ima_kexec_image = NULL;
+	kexec_segment_size = 0;
+	ima_free_kexec_file_buf(&ima_kexec_file);
+}
+
 #endif /* IMA_KEXEC */
 
 /*

Re: [PATCH v2 3/3] ima: clear kexec state when replacing file image

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] kexec: skipped ima_kexec_cleanup() with crash flags when
  !CONFIG_CRASH_DUMP
--

commit bddeb1d64059b337f4ac9e46300e35fd5ccaa690
Author: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

ima: clear kexec state when replacing file image

This commit tracks the kimage associated with ima_kexec_buffer and clears
the IMA kexec state when a normal kexec image is unloaded or replaced via
kexec_load() or kexec_file_load().

> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 90756dc6339b0..297f4657869bc 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
[ ... ]
> @@ -114,6 +115,10 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  
>  	if (nr_segments == 0) {
>  		/* Uninstall image */
> +#ifdef CONFIG_IMA_KEXEC
> +		if (!(flags & KEXEC_ON_CRASH))
> +			ima_kexec_cleanup();
> +#endif
>  		kimage_free(xchg(dest_image, NULL));
>  		ret = 0;
>  		goto out_unlock;
> @@ -163,6 +168,12 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  	if (ret)
>  		goto out;
>  
> +	/* A kexec_load image has no IMA measurement segment. */
> +#ifdef CONFIG_IMA_KEXEC
> +	if (!(flags & KEXEC_ON_CRASH))
> +		ima_kexec_cleanup();
> +#endif
> +
>  	/* Install the new kernel and uninstall the old */
>  	image = xchg(dest_image, image);
>  
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 6c5724aa3c749..866601712e994 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -399,8 +399,13 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>  #endif
>  		dest_image = &kexec_image;
>  
> -	if (flags & KEXEC_FILE_UNLOAD)
> +	if (flags & KEXEC_FILE_UNLOAD) {
> +#ifdef CONFIG_IMA_KEXEC
> +		if (image_type == KEXEC_TYPE_DEFAULT)
> +			ima_kexec_cleanup();
> +#endif
>  		goto exchange;
> +	}

[Severity: Medium]
Can these checks skip ima_kexec_cleanup() while still freeing kexec_image
when CONFIG_CRASH_DUMP is disabled?

In both do_kexec_load() and SYSCALL_DEFINE5(kexec_file_load), selecting
kexec_crash_image is guarded by CONFIG_CRASH_DUMP:

kernel/kexec.c:do_kexec_load() {
    ...
#ifdef CONFIG_CRASH_DUMP
	if (flags & KEXEC_ON_CRASH) {
		dest_image = &kexec_crash_image;
		if (kexec_crash_image)
			arch_kexec_unprotect_crashkres();
	} else
#endif
		dest_image = &kexec_image;
    ...
}

When CONFIG_IMA_KEXEC=y and CONFIG_CRASH_DUMP=n, calling kexec_load() with
KEXEC_ON_CRASH or kexec_file_load() with KEXEC_FILE_UNLOAD |
KEXEC_FILE_ON_CRASH sets dest_image to &kexec_image and frees the active
kexec_image, but the !(flags & KEXEC_ON_CRASH) and image_type ==
KEXEC_TYPE_DEFAULT checks skip ima_kexec_cleanup().  That leaves
ima_kexec_buffer mapped to freed pages and ima_kexec_image dangling; if a
subsequent load allocates a new struct kimage at the same slab address,
ima_update_kexec_buffer() sees ima_kexec_image == kexec_image and writes
the measurement list into the freed pages (use-after-free).

Checking dest_image == &kexec_image instead would match the image actually
being replaced or unloaded.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112125246.2715174-2-Jeremy.Jean@oss.cyber.gouv.fr?part=3