[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@xxxxxxxxxxxxxxx
Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
---
 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) {