[PATCH 00/10] ima: measure events between kexec load and execute

[PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

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'.

Some devices can be configured to call kexec 'load' first, and followed
by kexec 'execute' after some time. (as opposed to calling 'load' and
'execute' in one single kexec command).  In such scenario, if new IMA
measurements are added between kexec 'load' and kexec 'execute', the
TPM PCRs are extended with the IMA events between 'load' and 'execute';
but those IMA events are not carried over to the new kernel after kexec
soft reboot.  This results in mismatch between TPM PCR quotes and the
actual IMA measurements list after the device boots into the new kexec
image.  This mismatch results in the remote attestation failing for that
device.

This patch series proposes a solution to solve this problem by allocating
the necessary buffer at kexec 'load' time, and populating the buffer
with the IMA measurements at kexec 'execute' time. 

The solution includes:
 - addition of new functionality to allocate a buffer to hold IMA
   measurements at kexec 'load', 

 - ima functionality to suspend and resume measurements as needed during
   buffer copy at kexec 'execute',

 - ima functionality for mapping the measurement list 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',

 - and removal of deprecated functions.

The modifications 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.

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/

Tushar Sugandhi (10):
  ima: implement function to allocate buffer at kexec load
  ima: implement function to populate buffer at kexec execute
  ima: allocate buffer at kexec load to hold ima measurements
  ima: implement functions to suspend and resume measurements
  kexec: implement functions to map and unmap segment to kimage
  ima: update buffer at kexec execute with ima measurements
  ima: remove function ima_dump_measurement_list
  ima: implement and register a reboot notifier function to update kexec
    buffer
  ima: suspend measurements while the kexec buffer is being copied
  kexec: update kexec_file_load syscall to call ima_kexec_post_load

 include/linux/ima.h                |   3 +
 include/linux/kexec.h              |  13 ++
 kernel/kexec_core.c                |  72 +++++++++-
 kernel/kexec_file.c                |   7 +
 kernel/kexec_internal.h            |   1 +
 security/integrity/ima/ima.h       |   4 +
 security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
 security/integrity/ima/ima_queue.c |  32 +++++
 8 files changed, 295 insertions(+), 48 deletions(-)

-- 
2.25.1

[PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

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'.

Some devices can be configured to call kexec 'load' first, and followed
by kexec 'execute' after some time. (as opposed to calling 'load' and
'execute' in one single kexec command).  In such scenario, if new IMA
measurements are added between kexec 'load' and kexec 'execute', the
TPM PCRs are extended with the IMA events between 'load' and 'execute';
but those IMA events are not carried over to the new kernel after kexec
soft reboot.  This results in mismatch between TPM PCR quotes and the
actual IMA measurements list after the device boots into the new kexec
image.  This mismatch results in the remote attestation failing for that
device.

This patch series proposes a solution to solve this problem by allocating
the necessary buffer at kexec 'load' time, and populating the buffer
with the IMA measurements at kexec 'execute' time. 

The solution includes:
 - addition of new functionality to allocate a buffer to hold IMA
   measurements at kexec 'load', 

 - ima functionality to suspend and resume measurements as needed during
   buffer copy at kexec 'execute',

 - ima functionality for mapping the measurement list 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',

 - and removal of deprecated functions.

The modifications 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.

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/

Tushar Sugandhi (10):
  ima: implement function to allocate buffer at kexec load
  ima: implement function to populate buffer at kexec execute
  ima: allocate buffer at kexec load to hold ima measurements
  ima: implement functions to suspend and resume measurements
  kexec: implement functions to map and unmap segment to kimage
  ima: update buffer at kexec execute with ima measurements
  ima: remove function ima_dump_measurement_list
  ima: implement and register a reboot notifier function to update kexec
    buffer
  ima: suspend measurements while the kexec buffer is being copied
  kexec: update kexec_file_load syscall to call ima_kexec_post_load

 include/linux/ima.h                |   3 +
 include/linux/kexec.h              |  13 ++
 kernel/kexec_core.c                |  72 +++++++++-
 kernel/kexec_file.c                |   7 +
 kernel/kexec_internal.h            |   1 +
 security/integrity/ima/ima.h       |   4 +
 security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
 security/integrity/ima/ima_queue.c |  32 +++++
 8 files changed, 295 insertions(+), 48 deletions(-)

-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Tushar Sugandhi]

IMA does not provide a mechanism to allocate memory for IMA log storage
during kexec operation.  The function should handle the scenario where
the kexec load is called multiple times. 

Implement a function to allocate buffer of size kexec_segment_size at
kexec load.  If the buffer was already allocated, free that buffer and
reallocate.  Finally, initialize ima_khdr struct. 

The patch operates under the assumption that the segment size does not
change between kexec load and execute.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 47 ++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 419dc405c831..48a683874044 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -15,6 +15,53 @@
 #include "ima.h"
 
 #ifdef CONFIG_IMA_KEXEC
+struct seq_file ima_kexec_file;
+struct ima_kexec_hdr ima_khdr;
+static size_t kexec_segment_size;
+
+void ima_clear_kexec_file(void)
+{
+	vfree(ima_kexec_file.buf);
+	ima_kexec_file.buf = NULL;
+	ima_kexec_file.size = 0;
+	ima_kexec_file.read_pos = 0;
+	ima_kexec_file.count = 0;
+}
+
+static int ima_allocate_buf_at_kexec_load(void)
+{
+	if ((kexec_segment_size == 0) ||
+	    (kexec_segment_size == ULONG_MAX) ||
+	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
+		pr_err("%s: Invalid segment size for kexec: %zu\n",
+			__func__, kexec_segment_size);
+		return -EINVAL;
+	}
+
+	/* if kexec load was called before, clear the existing buffer
+	 *  before allocating a new one
+	 */
+	if (ima_kexec_file.buf)
+		ima_clear_kexec_file();
+
+	/* segment size can't change between kexec load and execute */
+	ima_kexec_file.buf = vmalloc(kexec_segment_size);
+	if (!ima_kexec_file.buf) {
+		pr_err("%s: No memory for ima kexec measurement buffer\n",
+			__func__);
+		return -ENOMEM;
+	}
+
+	ima_kexec_file.size = kexec_segment_size;
+	ima_kexec_file.read_pos = 0;
+	ima_kexec_file.count = sizeof(ima_khdr);	/* reserved space */
+
+	memset(&ima_khdr, 0, sizeof(ima_khdr));
+	ima_khdr.version = 1;
+
+	return 0;
+}
+
 static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 				     unsigned long segment_size)
 {
-- 
2.25.1

[PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Tushar Sugandhi]

IMA does not provide a mechanism to allocate memory for IMA log storage
during kexec operation.  The function should handle the scenario where
the kexec load is called multiple times. 

Implement a function to allocate buffer of size kexec_segment_size at
kexec load.  If the buffer was already allocated, free that buffer and
reallocate.  Finally, initialize ima_khdr struct. 

The patch operates under the assumption that the segment size does not
change between kexec load and execute.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 47 ++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 419dc405c831..48a683874044 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -15,6 +15,53 @@
 #include "ima.h"
 
 #ifdef CONFIG_IMA_KEXEC
+struct seq_file ima_kexec_file;
+struct ima_kexec_hdr ima_khdr;
+static size_t kexec_segment_size;
+
+void ima_clear_kexec_file(void)
+{
+	vfree(ima_kexec_file.buf);
+	ima_kexec_file.buf = NULL;
+	ima_kexec_file.size = 0;
+	ima_kexec_file.read_pos = 0;
+	ima_kexec_file.count = 0;
+}
+
+static int ima_allocate_buf_at_kexec_load(void)
+{
+	if ((kexec_segment_size == 0) ||
+	    (kexec_segment_size == ULONG_MAX) ||
+	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
+		pr_err("%s: Invalid segment size for kexec: %zu\n",
+			__func__, kexec_segment_size);
+		return -EINVAL;
+	}
+
+	/* if kexec load was called before, clear the existing buffer
+	 *  before allocating a new one
+	 */
+	if (ima_kexec_file.buf)
+		ima_clear_kexec_file();
+
+	/* segment size can't change between kexec load and execute */
+	ima_kexec_file.buf = vmalloc(kexec_segment_size);
+	if (!ima_kexec_file.buf) {
+		pr_err("%s: No memory for ima kexec measurement buffer\n",
+			__func__);
+		return -ENOMEM;
+	}
+
+	ima_kexec_file.size = kexec_segment_size;
+	ima_kexec_file.read_pos = 0;
+	ima_kexec_file.count = sizeof(ima_khdr);	/* reserved space */
+
+	memset(&ima_khdr, 0, sizeof(ima_khdr));
+	ima_khdr.version = 1;
+
+	return 0;
+}
+
 static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 				     unsigned long segment_size)
 {
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 02/10] ima: implement function to populate buffer at kexec execute

[Tushar Sugandhi]

There is no existing IMA functionality to just populate the buffer at
kexec execute with IMA measurements.

Implement a function to iterate over ima_measurements and populate the
ima_kexec_file buffer.  After the loop, populate ima_khdr with buffer
details (version, buffer size, number of measurements).  Copy the ima_khdr
data into ima_kexec_file.buf and update buffer_size and buffer.

The patch assumes that the ima_kexec_file.size is sufficient to hold all
the measurements.  It returns an error and does not handle scenarios where
additional space might be needed.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 48a683874044..858b67689701 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
 	return 0;
 }
 
+static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
+{
+	struct ima_queue_entry *qe;
+	int ret = 0;
+
+	/*
+	 * Ensure the kexec buffer is large enough to hold ima_khdr
+	 */
+	if (ima_kexec_file.size < sizeof(ima_khdr)) {
+		pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
+			__func__);
+		ima_clear_kexec_file();
+		return -ENOMEM;
+	}
+
+	list_for_each_entry_rcu(qe, &ima_measurements, later) {
+		if (ima_kexec_file.count < ima_kexec_file.size) {
+			ima_khdr.count++;
+			ima_measurements_show(&ima_kexec_file, qe);
+		} else {
+			ret = -ENOMEM;
+			pr_err("%s: Kexec ima_measurements buffer too small\n",
+				__func__);
+			break;
+		}
+	}
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * fill in reserved space with some buffer details
+	 * (eg. version, buffer size, number of measurements)
+	 */
+	ima_khdr.buffer_size = ima_kexec_file.count;
+	if (ima_canonical_fmt) {
+		ima_khdr.version = cpu_to_le16(ima_khdr.version);
+		ima_khdr.count = cpu_to_le64(ima_khdr.count);
+		ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
+	}
+
+	memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
+	*buffer_size = ima_kexec_file.count;
+	*buffer = ima_kexec_file.buf;
+
+out:
+	if (ret < 0)
+		ima_clear_kexec_file();
+
+	return ret;
+}
+
+
 static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 				     unsigned long segment_size)
 {
-- 
2.25.1

[PATCH 02/10] ima: implement function to populate buffer at kexec execute

[Tushar Sugandhi]

There is no existing IMA functionality to just populate the buffer at
kexec execute with IMA measurements.

Implement a function to iterate over ima_measurements and populate the
ima_kexec_file buffer.  After the loop, populate ima_khdr with buffer
details (version, buffer size, number of measurements).  Copy the ima_khdr
data into ima_kexec_file.buf and update buffer_size and buffer.

The patch assumes that the ima_kexec_file.size is sufficient to hold all
the measurements.  It returns an error and does not handle scenarios where
additional space might be needed.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 48a683874044..858b67689701 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
 	return 0;
 }
 
+static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
+{
+	struct ima_queue_entry *qe;
+	int ret = 0;
+
+	/*
+	 * Ensure the kexec buffer is large enough to hold ima_khdr
+	 */
+	if (ima_kexec_file.size < sizeof(ima_khdr)) {
+		pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
+			__func__);
+		ima_clear_kexec_file();
+		return -ENOMEM;
+	}
+
+	list_for_each_entry_rcu(qe, &ima_measurements, later) {
+		if (ima_kexec_file.count < ima_kexec_file.size) {
+			ima_khdr.count++;
+			ima_measurements_show(&ima_kexec_file, qe);
+		} else {
+			ret = -ENOMEM;
+			pr_err("%s: Kexec ima_measurements buffer too small\n",
+				__func__);
+			break;
+		}
+	}
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * fill in reserved space with some buffer details
+	 * (eg. version, buffer size, number of measurements)
+	 */
+	ima_khdr.buffer_size = ima_kexec_file.count;
+	if (ima_canonical_fmt) {
+		ima_khdr.version = cpu_to_le16(ima_khdr.version);
+		ima_khdr.count = cpu_to_le64(ima_khdr.count);
+		ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
+	}
+
+	memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
+	*buffer_size = ima_kexec_file.count;
+	*buffer = ima_kexec_file.buf;
+
+out:
+	if (ret < 0)
+		ima_clear_kexec_file();
+
+	return ret;
+}
+
+
 static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 				     unsigned long segment_size)
 {
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Tushar Sugandhi]

The IMA subsystem needs a dedicated mechanism to reserve extra memory for
measurements added between the kexec 'load' and kexec 'execute'.

Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
taking ima binary runtime measurements size, size of ima_kexec_hdr, and
IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
buffer.

This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
sufficient to handle the additional measurements.  This should be as per
the system requirements and based on the number of additional measurements
expected during the window from kexec 'load' to kexec 'execute'.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c29db699c996..2ffda9449b9b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 
 #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
 
+#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
+
 /* current content of the policy */
 extern int ima_policy_flag;
 
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 858b67689701..7deb8df31485 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -188,31 +188,30 @@ 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_segment_size;
 	int ret;
 
 	/*
-	 * Reserve an extra half page of memory for additional measurements
-	 * added during the kexec load.
+	 * Reserve extra memory for measurements added in the window from
+	 * kexec 'load' to kexec 'execute'.
 	 */
-	binary_runtime_size = ima_get_binary_runtime_size();
+	binary_runtime_size = ima_get_binary_runtime_size() +
+			      sizeof(struct ima_kexec_hdr) +
+			      IMA_KEXEC_EXTRA_SIZE;
+
 	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");
 		return;
 	}
 
-	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
-				  kexec_segment_size);
-	if (!kexec_buffer) {
-		pr_err("Not enough memory for the kexec measurement buffer.\n");
+	ret = ima_allocate_buf_at_kexec_load();
+	if (ret < 0)
 		return;
-	}
 
 	kbuf.buffer = kexec_buffer;
 	kbuf.bufsz = kexec_buffer_size;
-- 
2.25.1

[PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Tushar Sugandhi]

The IMA subsystem needs a dedicated mechanism to reserve extra memory for
measurements added between the kexec 'load' and kexec 'execute'.

Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
taking ima binary runtime measurements size, size of ima_kexec_hdr, and
IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
buffer.

This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
sufficient to handle the additional measurements.  This should be as per
the system requirements and based on the number of additional measurements
expected during the window from kexec 'load' to kexec 'execute'.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c29db699c996..2ffda9449b9b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 
 #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
 
+#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
+
 /* current content of the policy */
 extern int ima_policy_flag;
 
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 858b67689701..7deb8df31485 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -188,31 +188,30 @@ 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_segment_size;
 	int ret;
 
 	/*
-	 * Reserve an extra half page of memory for additional measurements
-	 * added during the kexec load.
+	 * Reserve extra memory for measurements added in the window from
+	 * kexec 'load' to kexec 'execute'.
 	 */
-	binary_runtime_size = ima_get_binary_runtime_size();
+	binary_runtime_size = ima_get_binary_runtime_size() +
+			      sizeof(struct ima_kexec_hdr) +
+			      IMA_KEXEC_EXTRA_SIZE;
+
 	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");
 		return;
 	}
 
-	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
-				  kexec_segment_size);
-	if (!kexec_buffer) {
-		pr_err("Not enough memory for the kexec measurement buffer.\n");
+	ret = ima_allocate_buf_at_kexec_load();
+	if (ret < 0)
 		return;
-	}
 
 	kbuf.buffer = kexec_buffer;
 	kbuf.bufsz = kexec_buffer_size;
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 04/10] ima: implement functions to suspend and resume measurements

[Tushar Sugandhi]

The IMA subsystem needs to provide a mechanism to suspend or resume
measurements.  This functionality is needed to temporarily halt the
recording of IMA measurements during certain system operations.

Add functions ima_measurements_suspend() and ima_measurements_resume()
functions to set and reset the suspend_ima_measurements variable,
respectively, to suspend/resume IMA measurements.  Use
ima_extend_list_mutex to ensure that the operations are thread-safe.

Note: Use ima_measurements_suspend() and ima_measurements_resume() in
pairs to avoid indefinitely suspending IMA measurements.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_queue.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 2ffda9449b9b..7127c5cf1909 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -163,6 +163,8 @@ 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);
+void ima_measurements_suspend(void);
+void ima_measurements_resume(void);
 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_queue.c b/security/integrity/ima/ima_queue.c
index 532da87ce519..cb9abc02a304 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -44,6 +44,11 @@ struct ima_h_table ima_htable = {
  */
 static DEFINE_MUTEX(ima_extend_list_mutex);
 
+/*
+ * Used internally by the kernel to suspend-resume ima measurements.
+ */
+static atomic_t suspend_ima_measurements;
+
 /* lookup up the digest value in the hash table, and return the entry */
 static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 						       int pcr)
@@ -148,6 +153,20 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
 	return result;
 }
 
+void ima_measurements_suspend(void)
+{
+	mutex_lock(&ima_extend_list_mutex);
+	atomic_set(&suspend_ima_measurements, 1);
+	mutex_unlock(&ima_extend_list_mutex);
+}
+
+void ima_measurements_resume(void)
+{
+	mutex_lock(&ima_extend_list_mutex);
+	atomic_set(&suspend_ima_measurements, 0);
+	mutex_unlock(&ima_extend_list_mutex);
+}
+
 /*
  * Add template entry to the measurement list and hash table, and
  * extend the pcr.
-- 
2.25.1

[PATCH 04/10] ima: implement functions to suspend and resume measurements

[Tushar Sugandhi]

The IMA subsystem needs to provide a mechanism to suspend or resume
measurements.  This functionality is needed to temporarily halt the
recording of IMA measurements during certain system operations.

Add functions ima_measurements_suspend() and ima_measurements_resume()
functions to set and reset the suspend_ima_measurements variable,
respectively, to suspend/resume IMA measurements.  Use
ima_extend_list_mutex to ensure that the operations are thread-safe.

Note: Use ima_measurements_suspend() and ima_measurements_resume() in
pairs to avoid indefinitely suspending IMA measurements.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_queue.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 2ffda9449b9b..7127c5cf1909 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -163,6 +163,8 @@ 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);
+void ima_measurements_suspend(void);
+void ima_measurements_resume(void);
 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_queue.c b/security/integrity/ima/ima_queue.c
index 532da87ce519..cb9abc02a304 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -44,6 +44,11 @@ struct ima_h_table ima_htable = {
  */
 static DEFINE_MUTEX(ima_extend_list_mutex);
 
+/*
+ * Used internally by the kernel to suspend-resume ima measurements.
+ */
+static atomic_t suspend_ima_measurements;
+
 /* lookup up the digest value in the hash table, and return the entry */
 static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 						       int pcr)
@@ -148,6 +153,20 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
 	return result;
 }
 
+void ima_measurements_suspend(void)
+{
+	mutex_lock(&ima_extend_list_mutex);
+	atomic_set(&suspend_ima_measurements, 1);
+	mutex_unlock(&ima_extend_list_mutex);
+}
+
+void ima_measurements_resume(void)
+{
+	mutex_lock(&ima_extend_list_mutex);
+	atomic_set(&suspend_ima_measurements, 0);
+	mutex_unlock(&ima_extend_list_mutex);
+}
+
 /*
  * Add template entry to the measurement list and hash table, and
  * extend the pcr.
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Tushar Sugandhi]

Currently, there's no mechanism to map and unmap segments to the kimage
structure.  This functionality is needed when dealing with memory segments
in the context of a kexec operation.

The patch adds two new functions: kimage_map_segment() and
kimage_unmap_segment().

Implement kimage_map_segment() which takes a kimage pointer, an address,
and a size.  Ensures that the entire segment is being mapped by comparing
the given address and size to each segment in the kimage's segment array.
Collect the source pages that correspond to the given address range,
allocate an array of pointers to these pages, and map them to a contiguous
range of virtual addresses.  If the mapping operation is successful, the
function returns the start of this range.  Otherwise, it frees the page
pointer array and returns NULL.

Implement kimage_unmap_segment() that takes a pointer to a segment buffer
and unmaps it using vunmap().

Finally, move for_each_kimage_entry() macro to kexec.h.

Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
avoid memory leaks and ensure that all mapped segments are properly
unmapped when they're no longer needed.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 include/linux/kexec.h | 13 ++++++++
 kernel/kexec_core.c   | 72 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 22b5cd24f581..e00b8101b53b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -490,6 +490,15 @@ static inline int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, g
 static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages) { }
 #endif
 
+#define for_each_kimage_entry(image, ptr, entry) \
+	for (ptr = &image->head; (entry = *ptr) && !(entry & IND_DONE); \
+		ptr = (entry & IND_INDIRECTION) ? \
+			boot_phys_to_virt((entry & PAGE_MASK)) : ptr + 1)
+
+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;
@@ -497,6 +506,10 @@ 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 3d578c6fefee..424e303fce25 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -594,11 +594,6 @@ void kimage_terminate(struct kimage *image)
 	*image->entry = IND_DONE;
 }
 
-#define for_each_kimage_entry(image, ptr, entry) \
-	for (ptr = &image->head; (entry = *ptr) && !(entry & IND_DONE); \
-		ptr = (entry & IND_INDIRECTION) ? \
-			boot_phys_to_virt((entry & PAGE_MASK)) : ptr + 1)
-
 static void kimage_free_entry(kimage_entry_t entry)
 {
 	struct page *page;
@@ -921,6 +916,73 @@ 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;
+	struct page **src_pages;
+	int i, npages;
+	kimage_entry_t *ptr, entry;
+	void *vaddr = NULL;
+
+	/*
+	 * Make sure that we are mapping a whole segment.
+	 */
+	for (i = 0; i < image->nr_segments; i++) {
+		if (addr == image->segment[i].mem &&
+		    size == image->segment[i].memsz) {
+			break;
+		}
+	}
+
+	if (i == image->nr_segments) {
+		pr_err("%s: No segment matching [%lx, %lx)\n", __func__,
+		       addr, eaddr);
+		return NULL;
+	}
+
+	/*
+	 * Collect the source pages and map them in a contiguous VA range.
+	 */
+	npages = PFN_UP(eaddr) - PFN_DOWN(addr);
+	src_pages = kmalloc(sizeof(*src_pages) * npages, GFP_KERNEL);
+	if (!src_pages) {
+		pr_err("%s: Could not allocate ima pages array.\n", __func__);
+		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++] = phys_to_page(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);
+	if (!vaddr) {
+		pr_err("%s: Could not map imap buffer.\n", __func__);
+		kfree(src_pages);
+	}
+	return vaddr;
+}
+
+void kimage_unmap_segment(void *segment_buffer)
+{
+	vunmap(segment_buffer);
+}
+
 struct kexec_load_limit {
 	/* Mutex protects the limit count. */
 	struct mutex mutex;
-- 
2.25.1

[PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Tushar Sugandhi]

Currently, there's no mechanism to map and unmap segments to the kimage
structure.  This functionality is needed when dealing with memory segments
in the context of a kexec operation.

The patch adds two new functions: kimage_map_segment() and
kimage_unmap_segment().

Implement kimage_map_segment() which takes a kimage pointer, an address,
and a size.  Ensures that the entire segment is being mapped by comparing
the given address and size to each segment in the kimage's segment array.
Collect the source pages that correspond to the given address range,
allocate an array of pointers to these pages, and map them to a contiguous
range of virtual addresses.  If the mapping operation is successful, the
function returns the start of this range.  Otherwise, it frees the page
pointer array and returns NULL.

Implement kimage_unmap_segment() that takes a pointer to a segment buffer
and unmaps it using vunmap().

Finally, move for_each_kimage_entry() macro to kexec.h.

Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
avoid memory leaks and ensure that all mapped segments are properly
unmapped when they're no longer needed.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 include/linux/kexec.h | 13 ++++++++
 kernel/kexec_core.c   | 72 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 22b5cd24f581..e00b8101b53b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -490,6 +490,15 @@ static inline int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, g
 static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages) { }
 #endif
 
+#define for_each_kimage_entry(image, ptr, entry) \
+	for (ptr = &image->head; (entry = *ptr) && !(entry & IND_DONE); \
+		ptr = (entry & IND_INDIRECTION) ? \
+			boot_phys_to_virt((entry & PAGE_MASK)) : ptr + 1)
+
+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;
@@ -497,6 +506,10 @@ 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 3d578c6fefee..424e303fce25 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -594,11 +594,6 @@ void kimage_terminate(struct kimage *image)
 	*image->entry = IND_DONE;
 }
 
-#define for_each_kimage_entry(image, ptr, entry) \
-	for (ptr = &image->head; (entry = *ptr) && !(entry & IND_DONE); \
-		ptr = (entry & IND_INDIRECTION) ? \
-			boot_phys_to_virt((entry & PAGE_MASK)) : ptr + 1)
-
 static void kimage_free_entry(kimage_entry_t entry)
 {
 	struct page *page;
@@ -921,6 +916,73 @@ 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;
+	struct page **src_pages;
+	int i, npages;
+	kimage_entry_t *ptr, entry;
+	void *vaddr = NULL;
+
+	/*
+	 * Make sure that we are mapping a whole segment.
+	 */
+	for (i = 0; i < image->nr_segments; i++) {
+		if (addr == image->segment[i].mem &&
+		    size == image->segment[i].memsz) {
+			break;
+		}
+	}
+
+	if (i == image->nr_segments) {
+		pr_err("%s: No segment matching [%lx, %lx)\n", __func__,
+		       addr, eaddr);
+		return NULL;
+	}
+
+	/*
+	 * Collect the source pages and map them in a contiguous VA range.
+	 */
+	npages = PFN_UP(eaddr) - PFN_DOWN(addr);
+	src_pages = kmalloc(sizeof(*src_pages) * npages, GFP_KERNEL);
+	if (!src_pages) {
+		pr_err("%s: Could not allocate ima pages array.\n", __func__);
+		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++] = phys_to_page(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);
+	if (!vaddr) {
+		pr_err("%s: Could not map imap buffer.\n", __func__);
+		kfree(src_pages);
+	}
+	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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Tushar Sugandhi]

In the current implementation, the measurement list is not updated in the
buffer during the window between kexec load and execute.  This leads to
measurement loss in the buffer when transitioning from the old to the new
kernel.  This patch provides a way to update the measurement list in the
buffer during a kexec execution.  Suspending the measurements during the
buffer update ensures the buffer doesn't get corrupted, or goes out of
sync with TPM PCRs. Thus it ensures the integrity of measurements is
maintained across kernel transitions during a kexec.

Introduce a new variable ima_kexec_buffer that is used to hold the address
of the IMA kexec buffer.

Implement a function ima_update_kexec_buffer() that is called during
kexec execute, allowing the IMA to update the measurement list with the
events between kexec load and execute.  First check if a kexec is in
progress and if the IMA kexec buffer is initialized.  If these conditions
are met, suspend IMA measurements and check if the new buffer size obtained
from ima_get_binary_runtime_size() is larger than the current buffer size.
If the new buffer size is too large, output an error message, and resume
the measurements.

If the new buffer size fits, populate the new buffer with the current
measurements using ima_populate_buf_at_kexec_execute() and copy it into
ima_kexec_buffer.

Unmap ima_kexec_buffer segment from the image.  Resuming the measurements
is not needed in case of successful measurements since the control is being
passed to the new kernel anyways through kexec execute.  However, the
measurements should be resumed if there are any errors flagged in the
function.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 50 ++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 7deb8df31485..224d88ccfe85 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -18,6 +18,7 @@
 struct seq_file ima_kexec_file;
 struct ima_kexec_hdr ima_khdr;
 static size_t kexec_segment_size;
+static void *ima_kexec_buffer;
 
 void ima_clear_kexec_file(void)
 {
@@ -230,6 +231,55 @@ void ima_add_kexec_buffer(struct kimage *image)
 	pr_debug("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)
+{
+	void *new_buffer = NULL;
+	size_t new_buffer_size, cur_buffer_size;
+	bool resume = false;
+
+	if (!kexec_in_progress) {
+		pr_info("%s: No kexec in progress.\n", __func__);
+		return NOTIFY_OK;
+	}
+
+	if (!ima_kexec_buffer) {
+		pr_err("%s: Kexec buffer not set.\n", __func__);
+		return NOTIFY_OK;
+	}
+
+	ima_measurements_suspend();
+
+	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
+	new_buffer_size = ima_get_binary_runtime_size();
+	if (new_buffer_size > cur_buffer_size) {
+		pr_err("%s: Measurement list grew too large.\n", __func__);
+		resume = true;
+		goto out;
+	}
+
+	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
+
+	if (!new_buffer) {
+		pr_err("%s: Dump measurements failed.\n", __func__);
+		resume = true;
+		goto out;
+	}
+	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
+out:
+	kimage_unmap_segment(ima_kexec_buffer);
+	ima_kexec_buffer = NULL;
+
+	if (resume)
+		ima_measurements_resume();
+
+	return NOTIFY_OK;
+}
+
 #endif /* IMA_KEXEC */
 
 /*
-- 
2.25.1

[PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Tushar Sugandhi]

In the current implementation, the measurement list is not updated in the
buffer during the window between kexec load and execute.  This leads to
measurement loss in the buffer when transitioning from the old to the new
kernel.  This patch provides a way to update the measurement list in the
buffer during a kexec execution.  Suspending the measurements during the
buffer update ensures the buffer doesn't get corrupted, or goes out of
sync with TPM PCRs. Thus it ensures the integrity of measurements is
maintained across kernel transitions during a kexec.

Introduce a new variable ima_kexec_buffer that is used to hold the address
of the IMA kexec buffer.

Implement a function ima_update_kexec_buffer() that is called during
kexec execute, allowing the IMA to update the measurement list with the
events between kexec load and execute.  First check if a kexec is in
progress and if the IMA kexec buffer is initialized.  If these conditions
are met, suspend IMA measurements and check if the new buffer size obtained
from ima_get_binary_runtime_size() is larger than the current buffer size.
If the new buffer size is too large, output an error message, and resume
the measurements.

If the new buffer size fits, populate the new buffer with the current
measurements using ima_populate_buf_at_kexec_execute() and copy it into
ima_kexec_buffer.

Unmap ima_kexec_buffer segment from the image.  Resuming the measurements
is not needed in case of successful measurements since the control is being
passed to the new kernel anyways through kexec execute.  However, the
measurements should be resumed if there are any errors flagged in the
function.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 50 ++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 7deb8df31485..224d88ccfe85 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -18,6 +18,7 @@
 struct seq_file ima_kexec_file;
 struct ima_kexec_hdr ima_khdr;
 static size_t kexec_segment_size;
+static void *ima_kexec_buffer;
 
 void ima_clear_kexec_file(void)
 {
@@ -230,6 +231,55 @@ void ima_add_kexec_buffer(struct kimage *image)
 	pr_debug("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)
+{
+	void *new_buffer = NULL;
+	size_t new_buffer_size, cur_buffer_size;
+	bool resume = false;
+
+	if (!kexec_in_progress) {
+		pr_info("%s: No kexec in progress.\n", __func__);
+		return NOTIFY_OK;
+	}
+
+	if (!ima_kexec_buffer) {
+		pr_err("%s: Kexec buffer not set.\n", __func__);
+		return NOTIFY_OK;
+	}
+
+	ima_measurements_suspend();
+
+	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
+	new_buffer_size = ima_get_binary_runtime_size();
+	if (new_buffer_size > cur_buffer_size) {
+		pr_err("%s: Measurement list grew too large.\n", __func__);
+		resume = true;
+		goto out;
+	}
+
+	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
+
+	if (!new_buffer) {
+		pr_err("%s: Dump measurements failed.\n", __func__);
+		resume = true;
+		goto out;
+	}
+	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
+out:
+	kimage_unmap_segment(ima_kexec_buffer);
+	ima_kexec_buffer = NULL;
+
+	if (resume)
+		ima_measurements_resume();
+
+	return NOTIFY_OK;
+}
+
 #endif /* IMA_KEXEC */
 
 /*
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 07/10] ima: remove function ima_dump_measurement_list

[Tushar Sugandhi]

The ima_dump_measurement_list function was designed to iterate over the
IMA measurement list and store each entry into a buffer.  The buffer,
along with its size, would be returned to the caller.  However, the
function is no longer required in the IMA subsystem.  It previously served
to dump the measurement list during the kexec 'load' operation, but this
functionality has been replaced by an alternative approach in this patch
series.

Remove the unused ima_dump_measurement_list function from the IMA
subsystem, to ensure a cleaner and more maintainable code.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 58 ------------------------------
 1 file changed, 58 deletions(-)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 224d88ccfe85..424930085c18 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -115,64 +115,6 @@ static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **
 }
 
 
-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;
-
-	/* segment size can't change between kexec load and execute */
-	file.buf = vmalloc(segment_size);
-	if (!file.buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	file.size = segment_size;
-	file.read_pos = 0;
-	file.count = sizeof(khdr);	/* reserved space */
-
-	memset(&khdr, 0, sizeof(khdr));
-	khdr.version = 1;
-	list_for_each_entry_rcu(qe, &ima_measurements, later) {
-		if (file.count < file.size) {
-			khdr.count++;
-			ima_measurements_show(&file, qe);
-		} else {
-			ret = -EINVAL;
-			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;
-	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));
-
-	print_hex_dump_debug("ima dump: ", DUMP_PREFIX_NONE, 16, 1,
-			     file.buf, file.count < 100 ? file.count : 100,
-			     true);
-
-	*buffer_size = file.count;
-	*buffer = file.buf;
-out:
-	if (ret == -EINVAL)
-		vfree(file.buf);
-	return ret;
-}
-
 /*
  * Called during kexec_file_load so that IMA can add a segment to the kexec
  * image for the measurement list for the next kernel.
-- 
2.25.1

[PATCH 07/10] ima: remove function ima_dump_measurement_list

[Tushar Sugandhi]

The ima_dump_measurement_list function was designed to iterate over the
IMA measurement list and store each entry into a buffer.  The buffer,
along with its size, would be returned to the caller.  However, the
function is no longer required in the IMA subsystem.  It previously served
to dump the measurement list during the kexec 'load' operation, but this
functionality has been replaced by an alternative approach in this patch
series.

Remove the unused ima_dump_measurement_list function from the IMA
subsystem, to ensure a cleaner and more maintainable code.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 58 ------------------------------
 1 file changed, 58 deletions(-)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 224d88ccfe85..424930085c18 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -115,64 +115,6 @@ static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **
 }
 
 
-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;
-
-	/* segment size can't change between kexec load and execute */
-	file.buf = vmalloc(segment_size);
-	if (!file.buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	file.size = segment_size;
-	file.read_pos = 0;
-	file.count = sizeof(khdr);	/* reserved space */
-
-	memset(&khdr, 0, sizeof(khdr));
-	khdr.version = 1;
-	list_for_each_entry_rcu(qe, &ima_measurements, later) {
-		if (file.count < file.size) {
-			khdr.count++;
-			ima_measurements_show(&file, qe);
-		} else {
-			ret = -EINVAL;
-			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;
-	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));
-
-	print_hex_dump_debug("ima dump: ", DUMP_PREFIX_NONE, 16, 1,
-			     file.buf, file.count < 100 ? file.count : 100,
-			     true);
-
-	*buffer_size = file.count;
-	*buffer = file.buf;
-out:
-	if (ret == -EINVAL)
-		vfree(file.buf);
-	return ret;
-}
-
 /*
  * Called during kexec_file_load so that IMA can add a segment to the kexec
  * image for the measurement list for the next kernel.
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 08/10] ima: implement and register a reboot notifier function to update kexec buffer

[Tushar Sugandhi]

The IMA subsystem needs to ensure that the measurement list is up to date
during a kexec operation, i.e., when the kernel is rebooted without going
through the full system reboot process. Currently, there is no mechanism
to update the measurement list when the system is soft booted using kexec.

Add a notifier function ima_update_kexec_buffer that is called during a
kexec soft reboot.  Implement ima_kexec_post_load, which maps the IMA
buffer after a kexec load and registers the reboot notifier.

Define a new notifier block update_buffer_nb, with ima_update_kexec_buffer
as its notifier function.  Register the notifier function in
ima_kexec_post_load if it hasn't been already, indicated by the
ima_kexec_update_registered flag.

When a kexec soft reboot is triggered, ima_update_kexec_buffer will be
executed to update the IMA buffer.  This ensures that the events between
kexec 'load' and 'execute' are captured and integrity of measurements
remains intact across kexec reboots.

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

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 86b57757c7b1..006db20f852d 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -49,6 +49,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 424930085c18..363c107dc4a5 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -12,6 +12,8 @@
 #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
@@ -19,6 +21,7 @@ struct seq_file ima_kexec_file;
 struct ima_kexec_hdr ima_khdr;
 static size_t kexec_segment_size;
 static void *ima_kexec_buffer;
+static bool ima_kexec_update_registered;
 
 void ima_clear_kexec_file(void)
 {
@@ -222,6 +225,38 @@ static int ima_update_kexec_buffer(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 
+struct notifier_block update_buffer_nb = {
+	.notifier_call = ima_update_kexec_buffer,
+};
+
+/*
+ * Create a mapping for the source pages that contain the IMA buffer
+ * so we can update it later.
+ */
+void ima_kexec_post_load(struct kimage *image)
+{
+	if (ima_kexec_buffer) {
+		kimage_unmap_segment(ima_kexec_buffer);
+		ima_kexec_buffer = NULL;
+	}
+
+	if (!image->ima_buffer_addr)
+		return;
+
+	ima_kexec_buffer = kimage_map_segment(image,
+					      image->ima_buffer_addr,
+					      image->ima_buffer_size);
+	if (!ima_kexec_buffer) {
+		pr_err("%s: Could not map measurements buffer.\n", __func__);
+		return;
+	}
+
+	if (!ima_kexec_update_registered) {
+		register_reboot_notifier(&update_buffer_nb);
+		ima_kexec_update_registered = true;
+	}
+}
+
 #endif /* IMA_KEXEC */
 
 /*
-- 
2.25.1

[PATCH 08/10] ima: implement and register a reboot notifier function to update kexec buffer

[Tushar Sugandhi]

The IMA subsystem needs to ensure that the measurement list is up to date
during a kexec operation, i.e., when the kernel is rebooted without going
through the full system reboot process. Currently, there is no mechanism
to update the measurement list when the system is soft booted using kexec.

Add a notifier function ima_update_kexec_buffer that is called during a
kexec soft reboot.  Implement ima_kexec_post_load, which maps the IMA
buffer after a kexec load and registers the reboot notifier.

Define a new notifier block update_buffer_nb, with ima_update_kexec_buffer
as its notifier function.  Register the notifier function in
ima_kexec_post_load if it hasn't been already, indicated by the
ima_kexec_update_registered flag.

When a kexec soft reboot is triggered, ima_update_kexec_buffer will be
executed to update the IMA buffer.  This ensures that the events between
kexec 'load' and 'execute' are captured and integrity of measurements
remains intact across kexec reboots.

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

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 86b57757c7b1..006db20f852d 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -49,6 +49,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 424930085c18..363c107dc4a5 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -12,6 +12,8 @@
 #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
@@ -19,6 +21,7 @@ struct seq_file ima_kexec_file;
 struct ima_kexec_hdr ima_khdr;
 static size_t kexec_segment_size;
 static void *ima_kexec_buffer;
+static bool ima_kexec_update_registered;
 
 void ima_clear_kexec_file(void)
 {
@@ -222,6 +225,38 @@ static int ima_update_kexec_buffer(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 
+struct notifier_block update_buffer_nb = {
+	.notifier_call = ima_update_kexec_buffer,
+};
+
+/*
+ * Create a mapping for the source pages that contain the IMA buffer
+ * so we can update it later.
+ */
+void ima_kexec_post_load(struct kimage *image)
+{
+	if (ima_kexec_buffer) {
+		kimage_unmap_segment(ima_kexec_buffer);
+		ima_kexec_buffer = NULL;
+	}
+
+	if (!image->ima_buffer_addr)
+		return;
+
+	ima_kexec_buffer = kimage_map_segment(image,
+					      image->ima_buffer_addr,
+					      image->ima_buffer_size);
+	if (!ima_kexec_buffer) {
+		pr_err("%s: Could not map measurements buffer.\n", __func__);
+		return;
+	}
+
+	if (!ima_kexec_update_registered) {
+		register_reboot_notifier(&update_buffer_nb);
+		ima_kexec_update_registered = true;
+	}
+}
+
 #endif /* IMA_KEXEC */
 
 /*
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 09/10] ima: suspend measurements while the kexec buffer is being copied

[Tushar Sugandhi]

If the new measurements are added to the list while the kexec buffer is
being copied during kexec execute, the buffer may get corrupted, or it can
go out of sync with TPM PCRs.  This could potentially lead to breaking the
integrity of the measurements after the kexec soft reboot to the new
kernel.

Introduce a check in the ima_add_template_entry function not to measure
events and return from the function early when the suspend_ima_measurements
flag is set.

This ensures the consistency of the IMA measurement list during the copying
of the kexec buffer.  When the suspend_ima_measurements flag is set, any
new measurements will be ignored until the flag is unset.  This allows the
buffer to be safely copied without worrying about concurrent modifications
to the measurement list.  This is crucial for maintaining the integrity of
the measurements during a kexec soft reboot.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_queue.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index cb9abc02a304..5946a26a2849 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -195,6 +195,19 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 		}
 	}
 
+	/*
+	 * suspend_ima_measurements will be set if the system is
+	 * undergoing kexec soft boot to a new kernel.
+	 * suspending measurements in this short window ensures the
+	 * consistency of the IMA measurement list during copying
+	 * of the kexec buffer.
+	 */
+	if (atomic_read(&suspend_ima_measurements)) {
+		audit_cause = "measurements_suspended";
+		audit_info = 0;
+		goto out;
+	}
+
 	result = ima_add_digest_entry(entry,
 				      !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE));
 	if (result < 0) {
-- 
2.25.1

[PATCH 09/10] ima: suspend measurements while the kexec buffer is being copied

[Tushar Sugandhi]

If the new measurements are added to the list while the kexec buffer is
being copied during kexec execute, the buffer may get corrupted, or it can
go out of sync with TPM PCRs.  This could potentially lead to breaking the
integrity of the measurements after the kexec soft reboot to the new
kernel.

Introduce a check in the ima_add_template_entry function not to measure
events and return from the function early when the suspend_ima_measurements
flag is set.

This ensures the consistency of the IMA measurement list during the copying
of the kexec buffer.  When the suspend_ima_measurements flag is set, any
new measurements will be ignored until the flag is unset.  This allows the
buffer to be safely copied without worrying about concurrent modifications
to the measurement list.  This is crucial for maintaining the integrity of
the measurements during a kexec soft reboot.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_queue.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index cb9abc02a304..5946a26a2849 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -195,6 +195,19 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 		}
 	}
 
+	/*
+	 * suspend_ima_measurements will be set if the system is
+	 * undergoing kexec soft boot to a new kernel.
+	 * suspending measurements in this short window ensures the
+	 * consistency of the IMA measurement list during copying
+	 * of the kexec buffer.
+	 */
+	if (atomic_read(&suspend_ima_measurements)) {
+		audit_cause = "measurements_suspended";
+		audit_info = 0;
+		goto out;
+	}
+
 	result = ima_add_digest_entry(entry,
 				      !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE));
 	if (result < 0) {
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[Tushar Sugandhi]

The kexec_file_load syscall is used to load a new kernel for kexec.
The syscall needs to update its function to call ima_kexec_post_load, which
was implemented in a previous patch.  ima_kexec_post_load takes care of
mapping the measurement list for the next kernel and registering a reboot
notifier if it's not already registered.

Modify the kexec_file_load syscall to call ima_kexec_post_load after the
image has been loaded and prepared for kexec.  This ensures that the IMA
measurement list will be available to the next kernel after a kexec reboot.
This also ensures the measurements taken in the window between kexec load
and execute are captured and passed to the next kernel. 

Declare the kimage_file_post_load function in the kernel/kexec_internal.h, 
so it can be properly used in the syscall.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 kernel/kexec_file.c     | 7 +++++++
 kernel/kexec_internal.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f989f5f1933b..efe28e77280c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
 }
 #endif
 
+void kimage_file_post_load(struct kimage *image)
+{
+	ima_kexec_post_load(image);
+}
+
 /*
  * In file mode list of segments is prepared by kernel. Copy relevant
  * data from user space, do error checking, prepare segment list
@@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 
 	kimage_terminate(image);
 
+	kimage_file_post_load(image);
+
 	ret = machine_kexec_post_load(image);
 	if (ret)
 		goto out;
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 74da1409cd14..98dd5fcafaf0 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
 
 #ifdef CONFIG_KEXEC_FILE
 #include <linux/purgatory.h>
+void kimage_file_post_load(struct kimage *image);
 void kimage_file_post_load_cleanup(struct kimage *image);
 extern char kexec_purgatory[];
 extern size_t kexec_purgatory_size;
-- 
2.25.1

[PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[Tushar Sugandhi]

The kexec_file_load syscall is used to load a new kernel for kexec.
The syscall needs to update its function to call ima_kexec_post_load, which
was implemented in a previous patch.  ima_kexec_post_load takes care of
mapping the measurement list for the next kernel and registering a reboot
notifier if it's not already registered.

Modify the kexec_file_load syscall to call ima_kexec_post_load after the
image has been loaded and prepared for kexec.  This ensures that the IMA
measurement list will be available to the next kernel after a kexec reboot.
This also ensures the measurements taken in the window between kexec load
and execute are captured and passed to the next kernel. 

Declare the kimage_file_post_load function in the kernel/kexec_internal.h, 
so it can be properly used in the syscall.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 kernel/kexec_file.c     | 7 +++++++
 kernel/kexec_internal.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f989f5f1933b..efe28e77280c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
 }
 #endif
 
+void kimage_file_post_load(struct kimage *image)
+{
+	ima_kexec_post_load(image);
+}
+
 /*
  * In file mode list of segments is prepared by kernel. Copy relevant
  * data from user space, do error checking, prepare segment list
@@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 
 	kimage_terminate(image);
 
+	kimage_file_post_load(image);
+
 	ret = machine_kexec_post_load(image);
 	if (ret)
 		goto out;
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 74da1409cd14..98dd5fcafaf0 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
 
 #ifdef CONFIG_KEXEC_FILE
 #include <linux/purgatory.h>
+void kimage_file_post_load(struct kimage *image);
 void kimage_file_post_load_cleanup(struct kimage *image);
 extern char kexec_purgatory[];
 extern size_t kexec_purgatory_size;
-- 
2.25.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Dave Young]

[Add Eric in cc]

On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
>
> 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'.
>
> Some devices can be configured to call kexec 'load' first, and followed
> by kexec 'execute' after some time. (as opposed to calling 'load' and
> 'execute' in one single kexec command).  In such scenario, if new IMA
> measurements are added between kexec 'load' and kexec 'execute', the
> TPM PCRs are extended with the IMA events between 'load' and 'execute';
> but those IMA events are not carried over to the new kernel after kexec
> soft reboot.  This results in mismatch between TPM PCR quotes and the
> actual IMA measurements list after the device boots into the new kexec
> image.  This mismatch results in the remote attestation failing for that
> device.
>
> This patch series proposes a solution to solve this problem by allocating
> the necessary buffer at kexec 'load' time, and populating the buffer
> with the IMA measurements at kexec 'execute' time.
>
> The solution includes:
>  - addition of new functionality to allocate a buffer to hold IMA
>    measurements at kexec 'load',
>
>  - ima functionality to suspend and resume measurements as needed during
>    buffer copy at kexec 'execute',
>
>  - ima functionality for mapping the measurement list 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',
>
>  - and removal of deprecated functions.
>
> The modifications 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.
>
> 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/
>
> Tushar Sugandhi (10):
>   ima: implement function to allocate buffer at kexec load
>   ima: implement function to populate buffer at kexec execute
>   ima: allocate buffer at kexec load to hold ima measurements
>   ima: implement functions to suspend and resume measurements
>   kexec: implement functions to map and unmap segment to kimage
>   ima: update buffer at kexec execute with ima measurements
>   ima: remove function ima_dump_measurement_list
>   ima: implement and register a reboot notifier function to update kexec
>     buffer
>   ima: suspend measurements while the kexec buffer is being copied
>   kexec: update kexec_file_load syscall to call ima_kexec_post_load
>
>  include/linux/ima.h                |   3 +
>  include/linux/kexec.h              |  13 ++
>  kernel/kexec_core.c                |  72 +++++++++-
>  kernel/kexec_file.c                |   7 +
>  kernel/kexec_internal.h            |   1 +
>  security/integrity/ima/ima.h       |   4 +
>  security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>  security/integrity/ima/ima_queue.c |  32 +++++
>  8 files changed, 295 insertions(+), 48 deletions(-)
>
> --
> 2.25.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Dave Young]

[Add Eric in cc]

On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
>
> 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'.
>
> Some devices can be configured to call kexec 'load' first, and followed
> by kexec 'execute' after some time. (as opposed to calling 'load' and
> 'execute' in one single kexec command).  In such scenario, if new IMA
> measurements are added between kexec 'load' and kexec 'execute', the
> TPM PCRs are extended with the IMA events between 'load' and 'execute';
> but those IMA events are not carried over to the new kernel after kexec
> soft reboot.  This results in mismatch between TPM PCR quotes and the
> actual IMA measurements list after the device boots into the new kexec
> image.  This mismatch results in the remote attestation failing for that
> device.
>
> This patch series proposes a solution to solve this problem by allocating
> the necessary buffer at kexec 'load' time, and populating the buffer
> with the IMA measurements at kexec 'execute' time.
>
> The solution includes:
>  - addition of new functionality to allocate a buffer to hold IMA
>    measurements at kexec 'load',
>
>  - ima functionality to suspend and resume measurements as needed during
>    buffer copy at kexec 'execute',
>
>  - ima functionality for mapping the measurement list 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',
>
>  - and removal of deprecated functions.
>
> The modifications 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.
>
> 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/
>
> Tushar Sugandhi (10):
>   ima: implement function to allocate buffer at kexec load
>   ima: implement function to populate buffer at kexec execute
>   ima: allocate buffer at kexec load to hold ima measurements
>   ima: implement functions to suspend and resume measurements
>   kexec: implement functions to map and unmap segment to kimage
>   ima: update buffer at kexec execute with ima measurements
>   ima: remove function ima_dump_measurement_list
>   ima: implement and register a reboot notifier function to update kexec
>     buffer
>   ima: suspend measurements while the kexec buffer is being copied
>   kexec: update kexec_file_load syscall to call ima_kexec_post_load
>
>  include/linux/ima.h                |   3 +
>  include/linux/kexec.h              |  13 ++
>  kernel/kexec_core.c                |  72 +++++++++-
>  kernel/kexec_file.c                |   7 +
>  kernel/kexec_internal.h            |   1 +
>  security/integrity/ima/ima.h       |   4 +
>  security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>  security/integrity/ima/ima_queue.c |  32 +++++
>  8 files changed, 295 insertions(+), 48 deletions(-)
>
> --
> 2.25.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[RuiRui Yang]

On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
>
> The kexec_file_load syscall is used to load a new kernel for kexec.
> The syscall needs to update its function to call ima_kexec_post_load, which
> was implemented in a previous patch.  ima_kexec_post_load takes care of
> mapping the measurement list for the next kernel and registering a reboot
> notifier if it's not already registered.
>
> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
> image has been loaded and prepared for kexec.  This ensures that the IMA
> measurement list will be available to the next kernel after a kexec reboot.
> This also ensures the measurements taken in the window between kexec load
> and execute are captured and passed to the next kernel.
>
> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
> so it can be properly used in the syscall.
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>  kernel/kexec_file.c     | 7 +++++++
>  kernel/kexec_internal.h | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f989f5f1933b..efe28e77280c 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
>  }
>  #endif
>
> +void kimage_file_post_load(struct kimage *image)
> +{
> +       ima_kexec_post_load(image);
> +}
> +
>  /*
>   * In file mode list of segments is prepared by kernel. Copy relevant
>   * data from user space, do error checking, prepare segment list
> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>
>         kimage_terminate(image);
>
> +       kimage_file_post_load(image);

I think it should be only done for the reboot case,  please just
exclude the kdump case here..

> +
>         ret = machine_kexec_post_load(image);
>         if (ret)
>                 goto out;
> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
> index 74da1409cd14..98dd5fcafaf0 100644
> --- a/kernel/kexec_internal.h
> +++ b/kernel/kexec_internal.h
> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
>
>  #ifdef CONFIG_KEXEC_FILE
>  #include <linux/purgatory.h>
> +void kimage_file_post_load(struct kimage *image);
>  void kimage_file_post_load_cleanup(struct kimage *image);
>  extern char kexec_purgatory[];
>  extern size_t kexec_purgatory_size;
> --
> 2.25.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[RuiRui Yang]

On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
>
> The kexec_file_load syscall is used to load a new kernel for kexec.
> The syscall needs to update its function to call ima_kexec_post_load, which
> was implemented in a previous patch.  ima_kexec_post_load takes care of
> mapping the measurement list for the next kernel and registering a reboot
> notifier if it's not already registered.
>
> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
> image has been loaded and prepared for kexec.  This ensures that the IMA
> measurement list will be available to the next kernel after a kexec reboot.
> This also ensures the measurements taken in the window between kexec load
> and execute are captured and passed to the next kernel.
>
> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
> so it can be properly used in the syscall.
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>  kernel/kexec_file.c     | 7 +++++++
>  kernel/kexec_internal.h | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f989f5f1933b..efe28e77280c 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
>  }
>  #endif
>
> +void kimage_file_post_load(struct kimage *image)
> +{
> +       ima_kexec_post_load(image);
> +}
> +
>  /*
>   * In file mode list of segments is prepared by kernel. Copy relevant
>   * data from user space, do error checking, prepare segment list
> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>
>         kimage_terminate(image);
>
> +       kimage_file_post_load(image);

I think it should be only done for the reboot case,  please just
exclude the kdump case here..

> +
>         ret = machine_kexec_post_load(image);
>         if (ret)
>                 goto out;
> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
> index 74da1409cd14..98dd5fcafaf0 100644
> --- a/kernel/kexec_internal.h
> +++ b/kernel/kexec_internal.h
> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
>
>  #ifdef CONFIG_KEXEC_FILE
>  #include <linux/purgatory.h>
> +void kimage_file_post_load(struct kimage *image);
>  void kimage_file_post_load_cleanup(struct kimage *image);
>  extern char kexec_purgatory[];
>  extern size_t kexec_purgatory_size;
> --
> 2.25.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Stefan Berger]

On 7/3/23 17:57, Tushar Sugandhi wrote:
> Currently, there's no mechanism to map and unmap segments to the kimage
> structure.  This functionality is needed when dealing with memory segments
> in the context of a kexec operation.
> 
> The patch adds two new functions: kimage_map_segment() and
> kimage_unmap_segment().
> 
> Implement kimage_map_segment() which takes a kimage pointer, an address,
> and a size.  Ensures that the entire segment is being mapped by comparing
> the given address and size to each segment in the kimage's segment array.
> Collect the source pages that correspond to the given address range,
> allocate an array of pointers to these pages, and map them to a contiguous
> range of virtual addresses.  If the mapping operation is successful, the
> function returns the start of this range.  Otherwise, it frees the page
> pointer array and returns NULL.
> 
> Implement kimage_unmap_segment() that takes a pointer to a segment buffer
> and unmaps it using vunmap().
> 
> Finally, move for_each_kimage_entry() macro to kexec.h.
> 
> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
> avoid memory leaks and ensure that all mapped segments are properly
> unmapped when they're no longer needed.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

> +
> +	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++] = phys_to_page(src_page_addr);

Since phys_to_page is not defined on many/most architectures I change it for ppc64 and have successfully used the following:

+                               src_pages[i++] = virt_to_page(__va(src_page_addr))


After several kexecs the following check still works:

# evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
Matched per TPM bank calculated digest(s).


    Stefan

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Stefan Berger]

On 7/3/23 17:57, Tushar Sugandhi wrote:
> Currently, there's no mechanism to map and unmap segments to the kimage
> structure.  This functionality is needed when dealing with memory segments
> in the context of a kexec operation.
> 
> The patch adds two new functions: kimage_map_segment() and
> kimage_unmap_segment().
> 
> Implement kimage_map_segment() which takes a kimage pointer, an address,
> and a size.  Ensures that the entire segment is being mapped by comparing
> the given address and size to each segment in the kimage's segment array.
> Collect the source pages that correspond to the given address range,
> allocate an array of pointers to these pages, and map them to a contiguous
> range of virtual addresses.  If the mapping operation is successful, the
> function returns the start of this range.  Otherwise, it frees the page
> pointer array and returns NULL.
> 
> Implement kimage_unmap_segment() that takes a pointer to a segment buffer
> and unmaps it using vunmap().
> 
> Finally, move for_each_kimage_entry() macro to kexec.h.
> 
> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
> avoid memory leaks and ensure that all mapped segments are properly
> unmapped when they're no longer needed.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

> +
> +	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++] = phys_to_page(src_page_addr);

Since phys_to_page is not defined on many/most architectures I change it for ppc64 and have successfully used the following:

+                               src_pages[i++] = virt_to_page(__va(src_page_addr))


After several kexecs the following check still works:

# evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
Matched per TPM bank calculated digest(s).


    Stefan


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> IMA does not provide a mechanism to allocate memory for IMA log storage
> during kexec operation.

The IMA measurement list is currently being carried across kexec, so
obviously a buffer is being allocated for it.  IMA not allocating
memory for the measurment list is not the problem statement.  Please
concisely provide the problem statement, explaining why IMA needs to
allocate the buffer.

> The function should handle the scenario where
> the kexec load is called multiple times.

Currently the buffer is being freed with the kexec 'unload'.  With this
patch IMA is allocating a buffer for the measurement list, which needs
to be freed independently of the kexec 'unload'.

> Implement a function to allocate buffer of size kexec_segment_size at
> kexec load.  If the buffer was already allocated, free that buffer and
> reallocate.  Finally, initialihze ima_khdr struct. 
> 
> The patch operates under the assumption that the segment size does not
> change between kexec load and execute.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

-- 
thanks,

Mimib

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> IMA does not provide a mechanism to allocate memory for IMA log storage
> during kexec operation.

The IMA measurement list is currently being carried across kexec, so
obviously a buffer is being allocated for it.  IMA not allocating
memory for the measurment list is not the problem statement.  Please
concisely provide the problem statement, explaining why IMA needs to
allocate the buffer.

> The function should handle the scenario where
> the kexec load is called multiple times.

Currently the buffer is being freed with the kexec 'unload'.  With this
patch IMA is allocating a buffer for the measurement list, which needs
to be freed independently of the kexec 'unload'.

> Implement a function to allocate buffer of size kexec_segment_size at
> kexec load.  If the buffer was already allocated, free that buffer and
> reallocate.  Finally, initialihze ima_khdr struct. 
> 
> The patch operates under the assumption that the segment size does not
> change between kexec load and execute.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

-- 
thanks,

Mimib





_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 02/10] ima: implement function to populate buffer at kexec execute

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> There is no existing IMA functionality to just populate the buffer at
> kexec execute with IMA measurements.

The same function that copies the measurement list at kexec 'load',
could be re-used at kexec 'exec'.   Why is a new function that is very
similar to the existing ima_dump_measurement_list() needed?

> 
> Implement a function to iterate over ima_measurements and populate the
> ima_kexec_file buffer.  After the loop, populate ima_khdr with buffer
> details (version, buffer size, number of measurements).  Copy the ima_khdr
> data into ima_kexec_file.buf and update buffer_size and buffer.
> 
> 
> The patch assumes that the ima_kexec_file.size is sufficient to hold all
> the measurements.  It returns an error and does not handle scenarios where
> additional space might be needed.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>  security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 48a683874044..858b67689701 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
>  	return 0;
>  }
>  
> +static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
> +{
> +	struct ima_queue_entry *qe;
> +	int ret = 0;
> +
> +	/*
> +	 * Ensure the kexec buffer is large enough to hold ima_khdr
> +	 */
> +	if (ima_kexec_file.size < sizeof(ima_khdr)) {
> +		pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
> +			__func__);
> +		ima_clear_kexec_file();
> +		return -ENOMEM;
> +	}
> +
> +	list_for_each_entry_rcu(qe, &ima_measurements, later) {
> +		if (ima_kexec_file.count < ima_kexec_file.size) {
> +			ima_khdr.count++;
> +			ima_measurements_show(&ima_kexec_file, qe);
> +		} else {
> +			ret = -ENOMEM;
> +			pr_err("%s: Kexec ima_measurements buffer too small\n",
> +				__func__);
> +			break;
> +		}
> +	}
> +	if (ret < 0)
> +		goto out;
> +
> +	/*
> +	 * fill in reserved space with some buffer details
> +	 * (eg. version, buffer size, number of measurements)
> +	 */
> +	ima_khdr.buffer_size = ima_kexec_file.count;
> +	if (ima_canonical_fmt) {
> +		ima_khdr.version = cpu_to_le16(ima_khdr.version);
> +		ima_khdr.count = cpu_to_le64(ima_khdr.count);
> +		ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
> +	}
> +
> +	memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
> +	*buffer_size = ima_kexec_file.count;
> +	*buffer = ima_kexec_file.buf;
> +
> +out:
> +	if (ret < 0)
> +		ima_clear_kexec_file();
> +
> +	return ret;
> +}
> +
> b+
>  static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>  				     unsigned long segment_size)
>  {

-- 
thanks,

Mimi

Re: [PATCH 02/10] ima: implement function to populate buffer at kexec execute

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> There is no existing IMA functionality to just populate the buffer at
> kexec execute with IMA measurements.

The same function that copies the measurement list at kexec 'load',
could be re-used at kexec 'exec'.   Why is a new function that is very
similar to the existing ima_dump_measurement_list() needed?

> 
> Implement a function to iterate over ima_measurements and populate the
> ima_kexec_file buffer.  After the loop, populate ima_khdr with buffer
> details (version, buffer size, number of measurements).  Copy the ima_khdr
> data into ima_kexec_file.buf and update buffer_size and buffer.
> 
> 
> The patch assumes that the ima_kexec_file.size is sufficient to hold all
> the measurements.  It returns an error and does not handle scenarios where
> additional space might be needed.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>  security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 48a683874044..858b67689701 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
>  	return 0;
>  }
>  
> +static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
> +{
> +	struct ima_queue_entry *qe;
> +	int ret = 0;
> +
> +	/*
> +	 * Ensure the kexec buffer is large enough to hold ima_khdr
> +	 */
> +	if (ima_kexec_file.size < sizeof(ima_khdr)) {
> +		pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
> +			__func__);
> +		ima_clear_kexec_file();
> +		return -ENOMEM;
> +	}
> +
> +	list_for_each_entry_rcu(qe, &ima_measurements, later) {
> +		if (ima_kexec_file.count < ima_kexec_file.size) {
> +			ima_khdr.count++;
> +			ima_measurements_show(&ima_kexec_file, qe);
> +		} else {
> +			ret = -ENOMEM;
> +			pr_err("%s: Kexec ima_measurements buffer too small\n",
> +				__func__);
> +			break;
> +		}
> +	}
> +	if (ret < 0)
> +		goto out;
> +
> +	/*
> +	 * fill in reserved space with some buffer details
> +	 * (eg. version, buffer size, number of measurements)
> +	 */
> +	ima_khdr.buffer_size = ima_kexec_file.count;
> +	if (ima_canonical_fmt) {
> +		ima_khdr.version = cpu_to_le16(ima_khdr.version);
> +		ima_khdr.count = cpu_to_le64(ima_khdr.count);
> +		ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
> +	}
> +
> +	memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
> +	*buffer_size = ima_kexec_file.count;
> +	*buffer = ima_kexec_file.buf;
> +
> +out:
> +	if (ret < 0)
> +		ima_clear_kexec_file();
> +
> +	return ret;
> +}
> +
> b+
>  static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>  				     unsigned long segment_size)
>  {

-- 
thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Mimi Zohar]

Hi Tushar,

The function to "ima: allocate buffer at kexec load to hold ima
measurements" already exists.  Please update the Subject line to
indicate increasing the IMA kexec buffer size.

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> The IMA subsystem needs a dedicated mechanism to reserve extra memory for
> measurements added between the kexec 'load' and kexec 'execute'.

What is a "dedicated mechanism?".  Either start the sentence with
"reserve ..." or completely remove it.

> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
> buffer.

The size of the ima_kexec_hdr kind of gets lost in the amount of
additional memory being allocated, but sure it's a nice clean up.

> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
> sufficient to handle the additional measurements.  This should be as per
> the system requirements and based on the number of additional measurements
> expected during the window from kexec 'load' to kexec 'execute'.

Should the extra amount of memory be hard coded?

> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>  security/integrity/ima/ima.h       |  2 ++
>  security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c29db699c996..2ffda9449b9b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>  
>  #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>  
> +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
> 

Not all IMA policies require extra memory.  Define and use a new IMA
Kconfig to set the amount of extra memory.

>  /* current content of the policy */
>  extern int ima_policy_flag;
>  
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 858b67689701..7deb8df31485 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -188,31 +188,30 @@ 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_segment_size;
>  	int ret;
>  
>  	/*
> -	 * Reserve an extra half page of memory for additional measurements
> -	 * added during the kexec load.
> +	 * Reserve extra memory for measurements added in the window from
> +	 * kexec 'load' to kexec 'execute'.
>  	 */
> -	binary_runtime_size = ima_get_binary_runtime_size();
> +	binary_runtime_size = ima_get_binary_runtime_size() +
> +			      sizeof(struct ima_kexec_hdr) +
> +			      IMA_KEXEC_EXTRA_SIZE;
> +
>  	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");
>  		return;
>  	}
>  
> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> -				  kexec_segment_size);

Removing the call to ima_dump_measurement_list here is not bisect safe.

> -	if (!kexec_buffer) {
> -		pr_err("Not enough memory for the kexec measurement buffer.\n");
> +	ret = ima_allocate_buf_at_kexec_load();
> +	if (ret < 0)
>  		return;
> -	}
>  
>  	kbuf.buffer = kexec_buffer;
>  	kbuf.bufsz = kexec_buffer_size;

-- 
thanks,

Mimi

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Mimi Zohar]

Hi Tushar,

The function to "ima: allocate buffer at kexec load to hold ima
measurements" already exists.  Please update the Subject line to
indicate increasing the IMA kexec buffer size.

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> The IMA subsystem needs a dedicated mechanism to reserve extra memory for
> measurements added between the kexec 'load' and kexec 'execute'.

What is a "dedicated mechanism?".  Either start the sentence with
"reserve ..." or completely remove it.

> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
> buffer.

The size of the ima_kexec_hdr kind of gets lost in the amount of
additional memory being allocated, but sure it's a nice clean up.

> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
> sufficient to handle the additional measurements.  This should be as per
> the system requirements and based on the number of additional measurements
> expected during the window from kexec 'load' to kexec 'execute'.

Should the extra amount of memory be hard coded?

> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>  security/integrity/ima/ima.h       |  2 ++
>  security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c29db699c996..2ffda9449b9b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>  
>  #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>  
> +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
> 

Not all IMA policies require extra memory.  Define and use a new IMA
Kconfig to set the amount of extra memory.

>  /* current content of the policy */
>  extern int ima_policy_flag;
>  
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 858b67689701..7deb8df31485 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -188,31 +188,30 @@ 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_segment_size;
>  	int ret;
>  
>  	/*
> -	 * Reserve an extra half page of memory for additional measurements
> -	 * added during the kexec load.
> +	 * Reserve extra memory for measurements added in the window from
> +	 * kexec 'load' to kexec 'execute'.
>  	 */
> -	binary_runtime_size = ima_get_binary_runtime_size();
> +	binary_runtime_size = ima_get_binary_runtime_size() +
> +			      sizeof(struct ima_kexec_hdr) +
> +			      IMA_KEXEC_EXTRA_SIZE;
> +
>  	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");
>  		return;
>  	}
>  
> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> -				  kexec_segment_size);

Removing the call to ima_dump_measurement_list here is not bisect safe.

> -	if (!kexec_buffer) {
> -		pr_err("Not enough memory for the kexec measurement buffer.\n");
> +	ret = ima_allocate_buf_at_kexec_load();
> +	if (ret < 0)
>  		return;
> -	}
>  
>  	kbuf.buffer = kexec_buffer;
>  	kbuf.bufsz = kexec_buffer_size;

-- 
thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 07/10] ima: remove function ima_dump_measurement_list

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> The ima_dump_measurement_list function was designed to iterate over the
> IMA measurement list and store each entry into a buffer.  The buffer,
> along with its size, would be returned to the caller.  However, the
> function is no longer required in the IMA subsystem.  It previously served
> to dump the measurement list during the kexec 'load' operation, but this
> functionality has been replaced by an alternative approach in this patch
> series.
> 
> Remove the unused ima_dump_measurement_list function from the IMA
> subsystem, to ensure a cleaner and more maintainable code.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

Have you compiled the code and tested after applying each patch? 
Removing the caller before removing the code should have resulted in a
warning.

-- 
thanks,

Mimi

Re: [PATCH 07/10] ima: remove function ima_dump_measurement_list

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> The ima_dump_measurement_list function was designed to iterate over the
> IMA measurement list and store each entry into a buffer.  The buffer,
> along with its size, would be returned to the caller.  However, the
> function is no longer required in the IMA subsystem.  It previously served
> to dump the measurement list during the kexec 'load' operation, but this
> functionality has been replaced by an alternative approach in this patch
> series.
> 
> Remove the unused ima_dump_measurement_list function from the IMA
> subsystem, to ensure a cleaner and more maintainable code.
> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

Have you compiled the code and tested after applying each patch? 
Removing the caller before removing the code should have resulted in a
warning.

-- 
thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:

> +/*
> + * 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)
> +{
> +	void *new_buffer = NULL;
> +	size_t new_buffer_size, cur_buffer_size;
> +	bool resume = false;
> +
> +	if (!kexec_in_progress) {
> +		pr_info("%s: No kexec in progress.\n", __func__);
> +		return NOTIFY_OK;
> +	}
> +
> +	if (!ima_kexec_buffer) {
> +		pr_err("%s: Kexec buffer not set.\n", __func__);
> +		return NOTIFY_OK;
> +	}
> +
> +	ima_measurements_suspend();
> +
> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
> +	new_buffer_size = ima_get_binary_runtime_size();
> +	if (new_buffer_size > cur_buffer_size) {
> +		pr_err("%s: Measurement list grew too large.\n", __func__);
> +		resume = true;
> +		goto out;
> +	}

This changes the current behavior of carrying as many measurements
across kexec as possible.  True the measurement list won't verify
against the TPM PCRs, but not copying the measurements leaves the
impression there weren't any previous measurements.

This also explains the reason for allocating an IMA buffer (patch 1/10)
and not writing the measurements directly into the kexec buffer.

> +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
> +
> +	if (!new_buffer) {
> +		pr_err("%s: Dump measurements failed.\n", __func__);
> +		resume = true;
> +		goto out;
> +	}
> +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
> +out:
> +	kimage_unmap_segment(ima_kexec_buffer);
> +	ima_kexec_buffer = NULL;
> +
> +	if (resume)
> +		ima_measurements_resume();
> +
> +	return NOTIFY_OK;
> +}
> +
>  #endif /* IMA_KEXEC */
>  
>  /*

-- 
thanks,

Mimi

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Mimi Zohar]

Hi Tushar,

On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:

> +/*
> + * 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)
> +{
> +	void *new_buffer = NULL;
> +	size_t new_buffer_size, cur_buffer_size;
> +	bool resume = false;
> +
> +	if (!kexec_in_progress) {
> +		pr_info("%s: No kexec in progress.\n", __func__);
> +		return NOTIFY_OK;
> +	}
> +
> +	if (!ima_kexec_buffer) {
> +		pr_err("%s: Kexec buffer not set.\n", __func__);
> +		return NOTIFY_OK;
> +	}
> +
> +	ima_measurements_suspend();
> +
> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
> +	new_buffer_size = ima_get_binary_runtime_size();
> +	if (new_buffer_size > cur_buffer_size) {
> +		pr_err("%s: Measurement list grew too large.\n", __func__);
> +		resume = true;
> +		goto out;
> +	}

This changes the current behavior of carrying as many measurements
across kexec as possible.  True the measurement list won't verify
against the TPM PCRs, but not copying the measurements leaves the
impression there weren't any previous measurements.

This also explains the reason for allocating an IMA buffer (patch 1/10)
and not writing the measurements directly into the kexec buffer.

> +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
> +
> +	if (!new_buffer) {
> +		pr_err("%s: Dump measurements failed.\n", __func__);
> +		resume = true;
> +		goto out;
> +	}
> +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
> +out:
> +	kimage_unmap_segment(ima_kexec_buffer);
> +	ima_kexec_buffer = NULL;
> +
> +	if (resume)
> +		ima_measurements_resume();
> +
> +	return NOTIFY_OK;
> +}
> +
>  #endif /* IMA_KEXEC */
>  
>  /*

-- 
thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Mimi Zohar]

On Mon, 2023-07-03 at 14:56 -0700, Tushar Sugandhi wrote:
> 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'.
> 
> Some devices can be configured to call kexec 'load' first, and followed
> by kexec 'execute' after some time. (as opposed to calling 'load' and
> 'execute' in one single kexec command).  In such scenario, if new IMA
> measurements are added between kexec 'load' and kexec 'execute', the
> TPM PCRs are extended with the IMA events between 'load' and 'execute';
> but those IMA events are not carried over to the new kernel after kexec
> soft reboot.  This results in mismatch between TPM PCR quotes and the
> actual IMA measurements list after the device boots into the new kexec
> image.  This mismatch results in the remote attestation failing for that
> device.
> 
> This patch series proposes a solution to solve this problem by allocating
> the necessary buffer at kexec 'load' time, and populating the buffer
> with the IMA measurements at kexec 'execute' time. 

Thanks, Tushar.   Depending on the IMA policy, the above problem
statement is correct, but not all policies are affected by it.  It's
also unclear why so much needs to change.  Instead of copying the
measurement list at kexec 'load',  using the existing method and simply
copying them at kexec 'exec' would suffice.

Also as mentioned in comment on 3/10, the ordering of this patch set is
not bisect safe.  If the same method of copying the measurement list
was used, changing from copying at  kexec 'load'  to kexec 'exec' could
be done in the same patch.

Mimi

> 
> The solution includes:
>  - addition of new functionality to allocate a buffer to hold IMA
>    measurements at kexec 'load', 
> 
>  - ima functionality to suspend and resume measurements as needed during
>    buffer copy at kexec 'execute',
> 
>  - ima functionality for mapping the measurement list 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',
> 
>  - and removal of deprecated functions.
> 
> The modifications 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.
> 
> 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/
> 
> Tushar Sugandhi (10):
>   ima: implement function to allocate buffer at kexec load
>   ima: implement function to populate buffer at kexec execute
>   ima: allocate buffer at kexec load to hold ima measurements
>   ima: implement functions to suspend and resume measurements
>   kexec: implement functions to map and unmap segment to kimage
>   ima: update buffer at kexec execute with ima measurements
>   ima: remove function ima_dump_measurement_list
>   ima: implement and register a reboot notifier function to update kexec
>     buffer
>   ima: suspend measurements while the kexec buffer is being copied
>   kexec: update kexec_file_load syscall to call ima_kexec_post_load
> 
>  include/linux/ima.h                |   3 +
>  include/linux/kexec.h              |  13 ++
>  kernel/kexec_core.c                |  72 +++++++++-
>  kernel/kexec_file.c                |   7 +
>  kernel/kexec_internal.h            |   1 +
>  security/integrity/ima/ima.h       |   4 +
>  security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>  security/integrity/ima/ima_queue.c |  32 +++++
>  8 files changed, 295 insertions(+), 48 deletions(-)
> 

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Mimi Zohar]

On Mon, 2023-07-03 at 14:56 -0700, Tushar Sugandhi wrote:
> 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'.
> 
> Some devices can be configured to call kexec 'load' first, and followed
> by kexec 'execute' after some time. (as opposed to calling 'load' and
> 'execute' in one single kexec command).  In such scenario, if new IMA
> measurements are added between kexec 'load' and kexec 'execute', the
> TPM PCRs are extended with the IMA events between 'load' and 'execute';
> but those IMA events are not carried over to the new kernel after kexec
> soft reboot.  This results in mismatch between TPM PCR quotes and the
> actual IMA measurements list after the device boots into the new kexec
> image.  This mismatch results in the remote attestation failing for that
> device.
> 
> This patch series proposes a solution to solve this problem by allocating
> the necessary buffer at kexec 'load' time, and populating the buffer
> with the IMA measurements at kexec 'execute' time. 

Thanks, Tushar.   Depending on the IMA policy, the above problem
statement is correct, but not all policies are affected by it.  It's
also unclear why so much needs to change.  Instead of copying the
measurement list at kexec 'load',  using the existing method and simply
copying them at kexec 'exec' would suffice.

Also as mentioned in comment on 3/10, the ordering of this patch set is
not bisect safe.  If the same method of copying the measurement list
was used, changing from copying at  kexec 'load'  to kexec 'exec' could
be done in the same patch.

Mimi

> 
> The solution includes:
>  - addition of new functionality to allocate a buffer to hold IMA
>    measurements at kexec 'load', 
> 
>  - ima functionality to suspend and resume measurements as needed during
>    buffer copy at kexec 'execute',
> 
>  - ima functionality for mapping the measurement list 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',
> 
>  - and removal of deprecated functions.
> 
> The modifications 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.
> 
> 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/
> 
> Tushar Sugandhi (10):
>   ima: implement function to allocate buffer at kexec load
>   ima: implement function to populate buffer at kexec execute
>   ima: allocate buffer at kexec load to hold ima measurements
>   ima: implement functions to suspend and resume measurements
>   kexec: implement functions to map and unmap segment to kimage
>   ima: update buffer at kexec execute with ima measurements
>   ima: remove function ima_dump_measurement_list
>   ima: implement and register a reboot notifier function to update kexec
>     buffer
>   ima: suspend measurements while the kexec buffer is being copied
>   kexec: update kexec_file_load syscall to call ima_kexec_post_load
> 
>  include/linux/ima.h                |   3 +
>  include/linux/kexec.h              |  13 ++
>  kernel/kexec_core.c                |  72 +++++++++-
>  kernel/kexec_file.c                |   7 +
>  kernel/kexec_internal.h            |   1 +
>  security/integrity/ima/ima.h       |   4 +
>  security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>  security/integrity/ima/ima_queue.c |  32 +++++
>  8 files changed, 295 insertions(+), 48 deletions(-)
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Mimi Zohar]

On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote:
> Hi Tushar,
> 
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> 
> > +/*
> > + * 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)
> > +{
> > +	void *new_buffer = NULL;
> > +	size_t new_buffer_size, cur_buffer_size;
> > +	bool resume = false;
> > +
> > +	if (!kexec_in_progress) {
> > +		pr_info("%s: No kexec in progress.\n", __func__);
> > +		return NOTIFY_OK;
> > +	}
> > +
> > +	if (!ima_kexec_buffer) {
> > +		pr_err("%s: Kexec buffer not set.\n", __func__);
> > +		return NOTIFY_OK;
> > +	}
> > +
> > +	ima_measurements_suspend();
> > +
> > +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
> > +	new_buffer_size = ima_get_binary_runtime_size();
> > +	if (new_buffer_size > cur_buffer_size) {
> > +		pr_err("%s: Measurement list grew too large.\n", __func__);
> > +		resume = true;
> > +		goto out;
> > +	}
> 
> This changes the current behavior of carrying as many measurements
> across kexec as possible.  True the measurement list won't verify
> against the TPM PCRs, but not copying the measurements leaves the
> impression there weren't any previous measurements.
> 
> This also explains the reason for allocating an IMA buffer (patch 1/10)
> and not writing the measurements directly into the kexec buffer.

If not carrying even a partial measurement list across kexec is
desired, then in addition to the "boot_aggregate" record, define a new
record containing the TPM pcrcounter.  With this information,
attestation servers will at least be able to detect if the measurement
list was truncated.

thanks,

Mimi

> 
> > +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
> > +
> > +	if (!new_buffer) {
> > +		pr_err("%s: Dump measurements failed.\n", __func__);
> > +		resume = true;
> > +		goto out;
> > +	}
> > +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
> > +out:
> > +	kimage_unmap_segment(ima_kexec_buffer);
> > +	ima_kexec_buffer = NULL;
> > +
> > +	if (resume)
> > +		ima_measurements_resume();
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> >  #endif /* IMA_KEXEC */
> >  
> >  /*
> 

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Mimi Zohar]

On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote:
> Hi Tushar,
> 
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> 
> > +/*
> > + * 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)
> > +{
> > +	void *new_buffer = NULL;
> > +	size_t new_buffer_size, cur_buffer_size;
> > +	bool resume = false;
> > +
> > +	if (!kexec_in_progress) {
> > +		pr_info("%s: No kexec in progress.\n", __func__);
> > +		return NOTIFY_OK;
> > +	}
> > +
> > +	if (!ima_kexec_buffer) {
> > +		pr_err("%s: Kexec buffer not set.\n", __func__);
> > +		return NOTIFY_OK;
> > +	}
> > +
> > +	ima_measurements_suspend();
> > +
> > +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
> > +	new_buffer_size = ima_get_binary_runtime_size();
> > +	if (new_buffer_size > cur_buffer_size) {
> > +		pr_err("%s: Measurement list grew too large.\n", __func__);
> > +		resume = true;
> > +		goto out;
> > +	}
> 
> This changes the current behavior of carrying as many measurements
> across kexec as possible.  True the measurement list won't verify
> against the TPM PCRs, but not copying the measurements leaves the
> impression there weren't any previous measurements.
> 
> This also explains the reason for allocating an IMA buffer (patch 1/10)
> and not writing the measurements directly into the kexec buffer.

If not carrying even a partial measurement list across kexec is
desired, then in addition to the "boot_aggregate" record, define a new
record containing the TPM pcrcounter.  With this information,
attestation servers will at least be able to detect if the measurement
list was truncated.

thanks,

Mimi

> 
> > +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
> > +
> > +	if (!new_buffer) {
> > +		pr_err("%s: Dump measurements failed.\n", __func__);
> > +		resume = true;
> > +		goto out;
> > +	}
> > +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
> > +out:
> > +	kimage_unmap_segment(ima_kexec_buffer);
> > +	ima_kexec_buffer = NULL;
> > +
> > +	if (resume)
> > +		ima_measurements_resume();
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> >  #endif /* IMA_KEXEC */
> >  
> >  /*
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

Thanks for reviewing this series Mimi. Appreciate it.

Adding Eric to cc.

On 7/7/23 08:55, Mimi Zohar wrote:
> On Mon, 2023-07-03 at 14:56 -0700, Tushar Sugandhi wrote:
>> 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'.
>>
>> Some devices can be configured to call kexec 'load' first, and followed
>> by kexec 'execute' after some time. (as opposed to calling 'load' and
>> 'execute' in one single kexec command).  In such scenario, if new IMA
>> measurements are added between kexec 'load' and kexec 'execute', the
>> TPM PCRs are extended with the IMA events between 'load' and 'execute';
>> but those IMA events are not carried over to the new kernel after kexec
>> soft reboot.  This results in mismatch between TPM PCR quotes and the
>> actual IMA measurements list after the device boots into the new kexec
>> image.  This mismatch results in the remote attestation failing for that
>> device.
>>
>> This patch series proposes a solution to solve this problem by allocating
>> the necessary buffer at kexec 'load' time, and populating the buffer
>> with the IMA measurements at kexec 'execute' time.
> Thanks, Tushar.   Depending on the IMA policy, the above problem
> statement is correct, but not all policies are affected by it.  It's
> also unclear why so much needs to change.  Instead of copying the
> measurement list at kexec 'load',  using the existing method and simply
> copying them at kexec 'exec' would suffice.
My understanding is the buffer must be allocated at kexec ‘load’ time.
The segment size cannot change between kexec ‘load’ and kexec ‘execute’.
Not sure if this is a technical limitation of IMA, or KEXEC.

Could you/someone from kexec side let me know?

If my current understanding is not correct, then I agree, simply copying
the measurements at kexec ‘execute’ using ima_dump_measurement_list should
suffice.

>
> Also as mentioned in comment on 3/10, the ordering of this patch set is
> not bisect safe.  If the same method of copying the measurement list
> was used, changing from copying at  kexec 'load'  to kexec 'exec' could
> be done in the same patch.
>
> Mimi
Ok. Based on my above response, if there is no such technical limitation
to allocate and copy at kexec ‘execute’ – I will simply move the call
to ima_dump_measurement_list from kexec 'load' to 'exec'.
And that can be done inthe same patch as you mentioned.

~Tushar
>
>> The solution includes:
>>   - addition of new functionality to allocate a buffer to hold IMA
>>     measurements at kexec 'load',
>>
>>   - ima functionality to suspend and resume measurements as needed during
>>     buffer copy at kexec 'execute',
>>
>>   - ima functionality for mapping the measurement list 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',
>>
>>   - and removal of deprecated functions.
>>
>> The modifications 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.
>>
>> 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/
>>
>> Tushar Sugandhi (10):
>>    ima: implement function to allocate buffer at kexec load
>>    ima: implement function to populate buffer at kexec execute
>>    ima: allocate buffer at kexec load to hold ima measurements
>>    ima: implement functions to suspend and resume measurements
>>    kexec: implement functions to map and unmap segment to kimage
>>    ima: update buffer at kexec execute with ima measurements
>>    ima: remove function ima_dump_measurement_list
>>    ima: implement and register a reboot notifier function to update kexec
>>      buffer
>>    ima: suspend measurements while the kexec buffer is being copied
>>    kexec: update kexec_file_load syscall to call ima_kexec_post_load
>>
>>   include/linux/ima.h                |   3 +
>>   include/linux/kexec.h              |  13 ++
>>   kernel/kexec_core.c                |  72 +++++++++-
>>   kernel/kexec_file.c                |   7 +
>>   kernel/kexec_internal.h            |   1 +
>>   security/integrity/ima/ima.h       |   4 +
>>   security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>>   security/integrity/ima/ima_queue.c |  32 +++++
>>   8 files changed, 295 insertions(+), 48 deletions(-)
>>

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

Thanks for reviewing this series Mimi. Appreciate it.

Adding Eric to cc.

On 7/7/23 08:55, Mimi Zohar wrote:
> On Mon, 2023-07-03 at 14:56 -0700, Tushar Sugandhi wrote:
>> 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'.
>>
>> Some devices can be configured to call kexec 'load' first, and followed
>> by kexec 'execute' after some time. (as opposed to calling 'load' and
>> 'execute' in one single kexec command).  In such scenario, if new IMA
>> measurements are added between kexec 'load' and kexec 'execute', the
>> TPM PCRs are extended with the IMA events between 'load' and 'execute';
>> but those IMA events are not carried over to the new kernel after kexec
>> soft reboot.  This results in mismatch between TPM PCR quotes and the
>> actual IMA measurements list after the device boots into the new kexec
>> image.  This mismatch results in the remote attestation failing for that
>> device.
>>
>> This patch series proposes a solution to solve this problem by allocating
>> the necessary buffer at kexec 'load' time, and populating the buffer
>> with the IMA measurements at kexec 'execute' time.
> Thanks, Tushar.   Depending on the IMA policy, the above problem
> statement is correct, but not all policies are affected by it.  It's
> also unclear why so much needs to change.  Instead of copying the
> measurement list at kexec 'load',  using the existing method and simply
> copying them at kexec 'exec' would suffice.
My understanding is the buffer must be allocated at kexec ‘load’ time.
The segment size cannot change between kexec ‘load’ and kexec ‘execute’.
Not sure if this is a technical limitation of IMA, or KEXEC.

Could you/someone from kexec side let me know?

If my current understanding is not correct, then I agree, simply copying
the measurements at kexec ‘execute’ using ima_dump_measurement_list should
suffice.

>
> Also as mentioned in comment on 3/10, the ordering of this patch set is
> not bisect safe.  If the same method of copying the measurement list
> was used, changing from copying at  kexec 'load'  to kexec 'exec' could
> be done in the same patch.
>
> Mimi
Ok. Based on my above response, if there is no such technical limitation
to allocate and copy at kexec ‘execute’ – I will simply move the call
to ima_dump_measurement_list from kexec 'load' to 'exec'.
And that can be done inthe same patch as you mentioned.

~Tushar
>
>> The solution includes:
>>   - addition of new functionality to allocate a buffer to hold IMA
>>     measurements at kexec 'load',
>>
>>   - ima functionality to suspend and resume measurements as needed during
>>     buffer copy at kexec 'execute',
>>
>>   - ima functionality for mapping the measurement list 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',
>>
>>   - and removal of deprecated functions.
>>
>> The modifications 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.
>>
>> 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/
>>
>> Tushar Sugandhi (10):
>>    ima: implement function to allocate buffer at kexec load
>>    ima: implement function to populate buffer at kexec execute
>>    ima: allocate buffer at kexec load to hold ima measurements
>>    ima: implement functions to suspend and resume measurements
>>    kexec: implement functions to map and unmap segment to kimage
>>    ima: update buffer at kexec execute with ima measurements
>>    ima: remove function ima_dump_measurement_list
>>    ima: implement and register a reboot notifier function to update kexec
>>      buffer
>>    ima: suspend measurements while the kexec buffer is being copied
>>    kexec: update kexec_file_load syscall to call ima_kexec_post_load
>>
>>   include/linux/ima.h                |   3 +
>>   include/linux/kexec.h              |  13 ++
>>   kernel/kexec_core.c                |  72 +++++++++-
>>   kernel/kexec_file.c                |   7 +
>>   kernel/kexec_internal.h            |   1 +
>>   security/integrity/ima/ima.h       |   4 +
>>   security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>>   security/integrity/ima/ima_queue.c |  32 +++++
>>   8 files changed, 295 insertions(+), 48 deletions(-)
>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

Thanks Dave for taking a look at this.

I will keep Eric looped in on the next versions of this
patch series.

~Tushar

On 7/7/23 01:18, Dave Young wrote:
> [Add Eric in cc]
>
> On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
>> 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'.
>>
>> Some devices can be configured to call kexec 'load' first, and followed
>> by kexec 'execute' after some time. (as opposed to calling 'load' and
>> 'execute' in one single kexec command).  In such scenario, if new IMA
>> measurements are added between kexec 'load' and kexec 'execute', the
>> TPM PCRs are extended with the IMA events between 'load' and 'execute';
>> but those IMA events are not carried over to the new kernel after kexec
>> soft reboot.  This results in mismatch between TPM PCR quotes and the
>> actual IMA measurements list after the device boots into the new kexec
>> image.  This mismatch results in the remote attestation failing for that
>> device.
>>
>> This patch series proposes a solution to solve this problem by allocating
>> the necessary buffer at kexec 'load' time, and populating the buffer
>> with the IMA measurements at kexec 'execute' time.
>>
>> The solution includes:
>>   - addition of new functionality to allocate a buffer to hold IMA
>>     measurements at kexec 'load',
>>
>>   - ima functionality to suspend and resume measurements as needed during
>>     buffer copy at kexec 'execute',
>>
>>   - ima functionality for mapping the measurement list 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',
>>
>>   - and removal of deprecated functions.
>>
>> The modifications 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.
>>
>> 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/
>>
>> Tushar Sugandhi (10):
>>    ima: implement function to allocate buffer at kexec load
>>    ima: implement function to populate buffer at kexec execute
>>    ima: allocate buffer at kexec load to hold ima measurements
>>    ima: implement functions to suspend and resume measurements
>>    kexec: implement functions to map and unmap segment to kimage
>>    ima: update buffer at kexec execute with ima measurements
>>    ima: remove function ima_dump_measurement_list
>>    ima: implement and register a reboot notifier function to update kexec
>>      buffer
>>    ima: suspend measurements while the kexec buffer is being copied
>>    kexec: update kexec_file_load syscall to call ima_kexec_post_load
>>
>>   include/linux/ima.h                |   3 +
>>   include/linux/kexec.h              |  13 ++
>>   kernel/kexec_core.c                |  72 +++++++++-
>>   kernel/kexec_file.c                |   7 +
>>   kernel/kexec_internal.h            |   1 +
>>   security/integrity/ima/ima.h       |   4 +
>>   security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>>   security/integrity/ima/ima_queue.c |  32 +++++
>>   8 files changed, 295 insertions(+), 48 deletions(-)
>>
>> --
>> 2.25.1
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
>>

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

Thanks Dave for taking a look at this.

I will keep Eric looped in on the next versions of this
patch series.

~Tushar

On 7/7/23 01:18, Dave Young wrote:
> [Add Eric in cc]
>
> On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
>> 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'.
>>
>> Some devices can be configured to call kexec 'load' first, and followed
>> by kexec 'execute' after some time. (as opposed to calling 'load' and
>> 'execute' in one single kexec command).  In such scenario, if new IMA
>> measurements are added between kexec 'load' and kexec 'execute', the
>> TPM PCRs are extended with the IMA events between 'load' and 'execute';
>> but those IMA events are not carried over to the new kernel after kexec
>> soft reboot.  This results in mismatch between TPM PCR quotes and the
>> actual IMA measurements list after the device boots into the new kexec
>> image.  This mismatch results in the remote attestation failing for that
>> device.
>>
>> This patch series proposes a solution to solve this problem by allocating
>> the necessary buffer at kexec 'load' time, and populating the buffer
>> with the IMA measurements at kexec 'execute' time.
>>
>> The solution includes:
>>   - addition of new functionality to allocate a buffer to hold IMA
>>     measurements at kexec 'load',
>>
>>   - ima functionality to suspend and resume measurements as needed during
>>     buffer copy at kexec 'execute',
>>
>>   - ima functionality for mapping the measurement list 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',
>>
>>   - and removal of deprecated functions.
>>
>> The modifications 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.
>>
>> 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/
>>
>> Tushar Sugandhi (10):
>>    ima: implement function to allocate buffer at kexec load
>>    ima: implement function to populate buffer at kexec execute
>>    ima: allocate buffer at kexec load to hold ima measurements
>>    ima: implement functions to suspend and resume measurements
>>    kexec: implement functions to map and unmap segment to kimage
>>    ima: update buffer at kexec execute with ima measurements
>>    ima: remove function ima_dump_measurement_list
>>    ima: implement and register a reboot notifier function to update kexec
>>      buffer
>>    ima: suspend measurements while the kexec buffer is being copied
>>    kexec: update kexec_file_load syscall to call ima_kexec_post_load
>>
>>   include/linux/ima.h                |   3 +
>>   include/linux/kexec.h              |  13 ++
>>   kernel/kexec_core.c                |  72 +++++++++-
>>   kernel/kexec_file.c                |   7 +
>>   kernel/kexec_internal.h            |   1 +
>>   security/integrity/ima/ima.h       |   4 +
>>   security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>>   security/integrity/ima/ima_queue.c |  32 +++++
>>   8 files changed, 295 insertions(+), 48 deletions(-)
>>
>> --
>> 2.25.1
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 06:00, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> IMA does not provide a mechanism to allocate memory for IMA log storage
>> during kexec operation.
> The IMA measurement list is currently being carried across kexec, so
> obviously a buffer is being allocated for it.  IMA not allocating
> memory for the measurment list is not the problem statement.  Please
> concisely provide the problem statement, explaining why IMA needs to
> allocate the buffer.
>
I meant IMA does not provide separate functions to allocate buffer and
populate measurements.  Both operations are wrapped in an atomic
ima_dump_measurement_list().

As I mentioned in the comment in the cover letter, if there is no such
technical limitation to allocate the buffer and copy the measurements at
kexec ‘execute’ – I will make the necessary code changes and update the
above line in the patch description accordingly.
>> The function should handle the scenario where
>> the kexec load is called multiple times.
> Currently the buffer is being freed with the kexec 'unload'.  With this
> patch IMA is allocating a buffer for the measurement list, which needs
> to be freed independently of the kexec 'unload'.
If we end up allocating the buffer at kexec ‘execute’ (which results in
soft boot to next Kernel) – is it technically possible that
kexec ‘unload’ being called after calling kexec ‘execute’?

If not, should I still free the buffer at kexec ‘unload’ in this
scenario?

~Tushar


>> Implement a function to allocate buffer of size kexec_segment_size at
>> kexec load.  If the buffer was already allocated, free that buffer and
>> reallocate.  Finally, initialihze ima_khdr struct.
>>
>> The patch operates under the assumption that the segment size does not
>> change between kexec load and execute.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 06:00, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> IMA does not provide a mechanism to allocate memory for IMA log storage
>> during kexec operation.
> The IMA measurement list is currently being carried across kexec, so
> obviously a buffer is being allocated for it.  IMA not allocating
> memory for the measurment list is not the problem statement.  Please
> concisely provide the problem statement, explaining why IMA needs to
> allocate the buffer.
>
I meant IMA does not provide separate functions to allocate buffer and
populate measurements.  Both operations are wrapped in an atomic
ima_dump_measurement_list().

As I mentioned in the comment in the cover letter, if there is no such
technical limitation to allocate the buffer and copy the measurements at
kexec ‘execute’ – I will make the necessary code changes and update the
above line in the patch description accordingly.
>> The function should handle the scenario where
>> the kexec load is called multiple times.
> Currently the buffer is being freed with the kexec 'unload'.  With this
> patch IMA is allocating a buffer for the measurement list, which needs
> to be freed independently of the kexec 'unload'.
If we end up allocating the buffer at kexec ‘execute’ (which results in
soft boot to next Kernel) – is it technically possible that
kexec ‘unload’ being called after calling kexec ‘execute’?

If not, should I still free the buffer at kexec ‘unload’ in this
scenario?

~Tushar


>> Implement a function to allocate buffer of size kexec_segment_size at
>> kexec load.  If the buffer was already allocated, free that buffer and
>> reallocate.  Finally, initialihze ima_khdr struct.
>>
>> The patch operates under the assumption that the segment size does not
>> change between kexec load and execute.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 02/10] ima: implement function to populate buffer at kexec execute

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 06:00, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> There is no existing IMA functionality to just populate the buffer at
>> kexec execute with IMA measurements.
> The same function that copies the measurement list at kexec 'load',
> could be re-used at kexec 'exec'.   Why is a new function that is very
> similar to the existing ima_dump_measurement_list() needed?
The current implementation of ima_dump_measurement_list() does both -
(1) allocation of buffer and (2) copying the measurements at kexec ‘load’.

The goal is to copy the measurements at kexec ‘execute’.

As I mentioned earlier, my understanding is the buffer must be allocated
at kexec ‘load’ time.  And the segment size cannot change between kexec
‘load’ and kexec ‘execute’.

That’s why I believe the two new functions are needed.
So I split the functionality in ima_dump_measurement_list() into
two functions –
  (1) ima_allocate_buf_at_kexec_load() in patch 01/10 and
  (2) ima_populate_buf_at_kexec_execute()  in patch 02/10

and then deprecated ima_dump_measurement_list() in patch 7/10.

If I call ima_dump_measurement_list() both at kexec ‘load’
as well as kexec ‘execute’, it will result in double memory allocation.

As you said, if the functionality can be achieved by simply moving
ima_dump_measurement_list() from kexec ‘load’ to kexec ‘execute’ –
then you are right, we don’t need the two separate functions.

~Tushar

>
>> Implement a function to iterate over ima_measurements and populate the
>> ima_kexec_file buffer.  After the loop, populate ima_khdr with buffer
>> details (version, buffer size, number of measurements).  Copy the ima_khdr
>> data into ima_kexec_file.buf and update buffer_size and buffer.
>>
>>
>> The patch assumes that the ima_kexec_file.size is sufficient to hold all
>> the measurements.  It returns an error and does not handle scenarios where
>> additional space might be needed.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 48a683874044..858b67689701 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
>>   	return 0;
>>   }
>>   
>> +static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
>> +{
>> +	struct ima_queue_entry *qe;
>> +	int ret = 0;
>> +
>> +	/*
>> +	 * Ensure the kexec buffer is large enough to hold ima_khdr
>> +	 */
>> +	if (ima_kexec_file.size < sizeof(ima_khdr)) {
>> +		pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
>> +			__func__);
>> +		ima_clear_kexec_file();
>> +		return -ENOMEM;
>> +	}
>> +
>> +	list_for_each_entry_rcu(qe, &ima_measurements, later) {
>> +		if (ima_kexec_file.count < ima_kexec_file.size) {
>> +			ima_khdr.count++;
>> +			ima_measurements_show(&ima_kexec_file, qe);
>> +		} else {
>> +			ret = -ENOMEM;
>> +			pr_err("%s: Kexec ima_measurements buffer too small\n",
>> +				__func__);
>> +			break;
>> +		}
>> +	}
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	/*
>> +	 * fill in reserved space with some buffer details
>> +	 * (eg. version, buffer size, number of measurements)
>> +	 */
>> +	ima_khdr.buffer_size = ima_kexec_file.count;
>> +	if (ima_canonical_fmt) {
>> +		ima_khdr.version = cpu_to_le16(ima_khdr.version);
>> +		ima_khdr.count = cpu_to_le64(ima_khdr.count);
>> +		ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
>> +	}
>> +
>> +	memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
>> +	*buffer_size = ima_kexec_file.count;
>> +	*buffer = ima_kexec_file.buf;
>> +
>> +out:
>> +	if (ret < 0)
>> +		ima_clear_kexec_file();
>> +
>> +	return ret;
>> +}
>> +
>> b+
>>   static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>>   				     unsigned long segment_size)
>>   {

Re: [PATCH 02/10] ima: implement function to populate buffer at kexec execute

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 06:00, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> There is no existing IMA functionality to just populate the buffer at
>> kexec execute with IMA measurements.
> The same function that copies the measurement list at kexec 'load',
> could be re-used at kexec 'exec'.   Why is a new function that is very
> similar to the existing ima_dump_measurement_list() needed?
The current implementation of ima_dump_measurement_list() does both -
(1) allocation of buffer and (2) copying the measurements at kexec ‘load’.

The goal is to copy the measurements at kexec ‘execute’.

As I mentioned earlier, my understanding is the buffer must be allocated
at kexec ‘load’ time.  And the segment size cannot change between kexec
‘load’ and kexec ‘execute’.

That’s why I believe the two new functions are needed.
So I split the functionality in ima_dump_measurement_list() into
two functions –
  (1) ima_allocate_buf_at_kexec_load() in patch 01/10 and
  (2) ima_populate_buf_at_kexec_execute()  in patch 02/10

and then deprecated ima_dump_measurement_list() in patch 7/10.

If I call ima_dump_measurement_list() both at kexec ‘load’
as well as kexec ‘execute’, it will result in double memory allocation.

As you said, if the functionality can be achieved by simply moving
ima_dump_measurement_list() from kexec ‘load’ to kexec ‘execute’ –
then you are right, we don’t need the two separate functions.

~Tushar

>
>> Implement a function to iterate over ima_measurements and populate the
>> ima_kexec_file buffer.  After the loop, populate ima_khdr with buffer
>> details (version, buffer size, number of measurements).  Copy the ima_khdr
>> data into ima_kexec_file.buf and update buffer_size and buffer.
>>
>>
>> The patch assumes that the ima_kexec_file.size is sufficient to hold all
>> the measurements.  It returns an error and does not handle scenarios where
>> additional space might be needed.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima_kexec.c | 52 ++++++++++++++++++++++++++++++
>>   1 file changed, 52 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 48a683874044..858b67689701 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -62,6 +62,58 @@ static int ima_allocate_buf_at_kexec_load(void)
>>   	return 0;
>>   }
>>   
>> +static int ima_populate_buf_at_kexec_execute(unsigned long *buffer_size, void **buffer)
>> +{
>> +	struct ima_queue_entry *qe;
>> +	int ret = 0;
>> +
>> +	/*
>> +	 * Ensure the kexec buffer is large enough to hold ima_khdr
>> +	 */
>> +	if (ima_kexec_file.size < sizeof(ima_khdr)) {
>> +		pr_err("%s: Kexec buffer size too low to hold ima_khdr\n",
>> +			__func__);
>> +		ima_clear_kexec_file();
>> +		return -ENOMEM;
>> +	}
>> +
>> +	list_for_each_entry_rcu(qe, &ima_measurements, later) {
>> +		if (ima_kexec_file.count < ima_kexec_file.size) {
>> +			ima_khdr.count++;
>> +			ima_measurements_show(&ima_kexec_file, qe);
>> +		} else {
>> +			ret = -ENOMEM;
>> +			pr_err("%s: Kexec ima_measurements buffer too small\n",
>> +				__func__);
>> +			break;
>> +		}
>> +	}
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	/*
>> +	 * fill in reserved space with some buffer details
>> +	 * (eg. version, buffer size, number of measurements)
>> +	 */
>> +	ima_khdr.buffer_size = ima_kexec_file.count;
>> +	if (ima_canonical_fmt) {
>> +		ima_khdr.version = cpu_to_le16(ima_khdr.version);
>> +		ima_khdr.count = cpu_to_le64(ima_khdr.count);
>> +		ima_khdr.buffer_size = cpu_to_le64(ima_khdr.buffer_size);
>> +	}
>> +
>> +	memcpy(ima_kexec_file.buf, &ima_khdr, sizeof(ima_khdr));
>> +	*buffer_size = ima_kexec_file.count;
>> +	*buffer = ima_kexec_file.buf;
>> +
>> +out:
>> +	if (ret < 0)
>> +		ima_clear_kexec_file();
>> +
>> +	return ret;
>> +}
>> +
>> b+
>>   static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>>   				     unsigned long segment_size)
>>   {

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 06:01, Mimi Zohar wrote:
> Hi Tushar,
>
> The function to "ima: allocate buffer at kexec load to hold ima
> measurements" already exists.  Please update the Subject line to
> indicate increasing the IMA kexec buffer size.
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> The IMA subsystem needs a dedicated mechanism to reserve extra memory for
>> measurements added between the kexec 'load' and kexec 'execute'.
> What is a "dedicated mechanism?".  Either start the sentence with
> "reserve ..." or completely remove it.
>
Will do.
>> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
>> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
>> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
>> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
>> buffer.
> The size of the ima_kexec_hdr kind of gets lost in the amount of
> additional memory being allocated, but sure it's a nice clean up.
Thanks. :)
>> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
>> sufficient to handle the additional measurements.  This should be as per
>> the system requirements and based on the number of additional measurements
>> expected during the window from kexec 'load' to kexec 'execute'.
> Should the extra amount of memory be hard coded?
>
You are right.  As you suggested below, defining the extra amount in
Kconfig is a cleaner approach.

>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima.h       |  2 ++
>>   security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index c29db699c996..2ffda9449b9b 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>>   
>>   #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>>   
>> +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
>>
> Not all IMA policies require extra memory.  Define and use a new IMA
> Kconfig to set the amount of extra memory.
Sure.  Will do.
>>   /* current content of the policy */
>>   extern int ima_policy_flag;
>>   
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 858b67689701..7deb8df31485 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -188,31 +188,30 @@ 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_segment_size;
>>   	int ret;
>>   
>>   	/*
>> -	 * Reserve an extra half page of memory for additional measurements
>> -	 * added during the kexec load.
>> +	 * Reserve extra memory for measurements added in the window from
>> +	 * kexec 'load' to kexec 'execute'.
>>   	 */
>> -	binary_runtime_size = ima_get_binary_runtime_size();
>> +	binary_runtime_size = ima_get_binary_runtime_size() +
>> +			      sizeof(struct ima_kexec_hdr) +
>> +			      IMA_KEXEC_EXTRA_SIZE;
>> +
>>   	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");
>>   		return;
>>   	}
>>   
>> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
>> -				  kexec_segment_size);
> Removing the call to ima_dump_measurement_list here is not bisect safe.
Agreed.  Will take care of this based on if we end up using just
ima_dump_measurement_list(), or combination of
ima_allocate_buf_at_kexec_load() and ima_populate_buf_at_kexec_execute().

~Tushar
>> -	if (!kexec_buffer) {
>> -		pr_err("Not enough memory for the kexec measurement buffer.\n");
>> +	ret = ima_allocate_buf_at_kexec_load();
>> +	if (ret < 0)
>>   		return;
>> -	}
>>   
>>   	kbuf.buffer = kexec_buffer;
>>   	kbuf.bufsz = kexec_buffer_size;

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 06:01, Mimi Zohar wrote:
> Hi Tushar,
>
> The function to "ima: allocate buffer at kexec load to hold ima
> measurements" already exists.  Please update the Subject line to
> indicate increasing the IMA kexec buffer size.
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> The IMA subsystem needs a dedicated mechanism to reserve extra memory for
>> measurements added between the kexec 'load' and kexec 'execute'.
> What is a "dedicated mechanism?".  Either start the sentence with
> "reserve ..." or completely remove it.
>
Will do.
>> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
>> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
>> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
>> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
>> buffer.
> The size of the ima_kexec_hdr kind of gets lost in the amount of
> additional memory being allocated, but sure it's a nice clean up.
Thanks. :)
>> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
>> sufficient to handle the additional measurements.  This should be as per
>> the system requirements and based on the number of additional measurements
>> expected during the window from kexec 'load' to kexec 'execute'.
> Should the extra amount of memory be hard coded?
>
You are right.  As you suggested below, defining the extra amount in
Kconfig is a cleaner approach.

>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima.h       |  2 ++
>>   security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index c29db699c996..2ffda9449b9b 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>>   
>>   #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>>   
>> +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
>>
> Not all IMA policies require extra memory.  Define and use a new IMA
> Kconfig to set the amount of extra memory.
Sure.  Will do.
>>   /* current content of the policy */
>>   extern int ima_policy_flag;
>>   
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 858b67689701..7deb8df31485 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -188,31 +188,30 @@ 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_segment_size;
>>   	int ret;
>>   
>>   	/*
>> -	 * Reserve an extra half page of memory for additional measurements
>> -	 * added during the kexec load.
>> +	 * Reserve extra memory for measurements added in the window from
>> +	 * kexec 'load' to kexec 'execute'.
>>   	 */
>> -	binary_runtime_size = ima_get_binary_runtime_size();
>> +	binary_runtime_size = ima_get_binary_runtime_size() +
>> +			      sizeof(struct ima_kexec_hdr) +
>> +			      IMA_KEXEC_EXTRA_SIZE;
>> +
>>   	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");
>>   		return;
>>   	}
>>   
>> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
>> -				  kexec_segment_size);
> Removing the call to ima_dump_measurement_list here is not bisect safe.
Agreed.  Will take care of this based on if we end up using just
ima_dump_measurement_list(), or combination of
ima_allocate_buf_at_kexec_load() and ima_populate_buf_at_kexec_execute().

~Tushar
>> -	if (!kexec_buffer) {
>> -		pr_err("Not enough memory for the kexec measurement buffer.\n");
>> +	ret = ima_allocate_buf_at_kexec_load();
>> +	if (ret < 0)
>>   		return;
>> -	}
>>   
>>   	kbuf.buffer = kexec_buffer;
>>   	kbuf.bufsz = kexec_buffer_size;

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 05:28, Stefan Berger wrote:
>
>
> On 7/3/23 17:57, Tushar Sugandhi wrote:
>> Currently, there's no mechanism to map and unmap segments to the kimage
>> structure.  This functionality is needed when dealing with memory 
>> segments
>> in the context of a kexec operation.
>>
>> The patch adds two new functions: kimage_map_segment() and
>> kimage_unmap_segment().
>>
>> Implement kimage_map_segment() which takes a kimage pointer, an address,
>> and a size.  Ensures that the entire segment is being mapped by 
>> comparing
>> the given address and size to each segment in the kimage's segment 
>> array.
>> Collect the source pages that correspond to the given address range,
>> allocate an array of pointers to these pages, and map them to a 
>> contiguous
>> range of virtual addresses.  If the mapping operation is successful, the
>> function returns the start of this range.  Otherwise, it frees the page
>> pointer array and returns NULL.
>>
>> Implement kimage_unmap_segment() that takes a pointer to a segment 
>> buffer
>> and unmaps it using vunmap().
>>
>> Finally, move for_each_kimage_entry() macro to kexec.h.
>>
>> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
>> avoid memory leaks and ensure that all mapped segments are properly
>> unmapped when they're no longer needed.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>
>> +
>> +    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++] = phys_to_page(src_page_addr);
>
> Since phys_to_page is not defined on many/most architectures I change 
> it for ppc64 and have successfully used the following:
>
> +                               src_pages[i++] = 
> virt_to_page(__va(src_page_addr))
>
>
> After several kexecs the following check still works:
>
> # evmctl ima_measurement --ignore-violations 
> /sys/kernel/security/ima/binary_runtime_measurements
> Matched per TPM bank calculated digest(s).
>
>
>    Stefan
Thank you so much Stefan for reviewing this series, and catching this
issue.  Are you suggesting I should use virt_to_page on all architectures
unconditionally, or use it only when phys_to_page is not available?

~Tushar

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 05:28, Stefan Berger wrote:
>
>
> On 7/3/23 17:57, Tushar Sugandhi wrote:
>> Currently, there's no mechanism to map and unmap segments to the kimage
>> structure.  This functionality is needed when dealing with memory 
>> segments
>> in the context of a kexec operation.
>>
>> The patch adds two new functions: kimage_map_segment() and
>> kimage_unmap_segment().
>>
>> Implement kimage_map_segment() which takes a kimage pointer, an address,
>> and a size.  Ensures that the entire segment is being mapped by 
>> comparing
>> the given address and size to each segment in the kimage's segment 
>> array.
>> Collect the source pages that correspond to the given address range,
>> allocate an array of pointers to these pages, and map them to a 
>> contiguous
>> range of virtual addresses.  If the mapping operation is successful, the
>> function returns the start of this range.  Otherwise, it frees the page
>> pointer array and returns NULL.
>>
>> Implement kimage_unmap_segment() that takes a pointer to a segment 
>> buffer
>> and unmaps it using vunmap().
>>
>> Finally, move for_each_kimage_entry() macro to kexec.h.
>>
>> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
>> avoid memory leaks and ensure that all mapped segments are properly
>> unmapped when they're no longer needed.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>
>> +
>> +    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++] = phys_to_page(src_page_addr);
>
> Since phys_to_page is not defined on many/most architectures I change 
> it for ppc64 and have successfully used the following:
>
> +                               src_pages[i++] = 
> virt_to_page(__va(src_page_addr))
>
>
> After several kexecs the following check still works:
>
> # evmctl ima_measurement --ignore-violations 
> /sys/kernel/security/ima/binary_runtime_measurements
> Matched per TPM bank calculated digest(s).
>
>
>    Stefan
Thank you so much Stefan for reviewing this series, and catching this
issue.  Are you suggesting I should use virt_to_page on all architectures
unconditionally, or use it only when phys_to_page is not available?

~Tushar

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 08:01, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>
>> +/*
>> + * 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)
>> +{
>> +	void *new_buffer = NULL;
>> +	size_t new_buffer_size, cur_buffer_size;
>> +	bool resume = false;
>> +
>> +	if (!kexec_in_progress) {
>> +		pr_info("%s: No kexec in progress.\n", __func__);
>> +		return NOTIFY_OK;
>> +	}
>> +
>> +	if (!ima_kexec_buffer) {
>> +		pr_err("%s: Kexec buffer not set.\n", __func__);
>> +		return NOTIFY_OK;
>> +	}
>> +
>> +	ima_measurements_suspend();
>> +
>> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
>> +	new_buffer_size = ima_get_binary_runtime_size();
>> +	if (new_buffer_size > cur_buffer_size) {
>> +		pr_err("%s: Measurement list grew too large.\n", __func__);
>> +		resume = true;
>> +		goto out;
>> +	}
> This changes the current behavior of carrying as many measurements
> across kexec as possible.  True the measurement list won't verify
> against the TPM PCRs, but not copying the measurements leaves the
> impression there weren't any previous measurements.
>
> This also explains the reason for allocating an IMA buffer (patch 1/10)
> and not writing the measurements directly into the kexec buffer.
Thanks.

I will update this logic depending if we decide to use
ima_dump_measurement_list() at kexec ‘execute’, or combination of
ima_allocate_buf_at_kexec_load() and ima_populate_buf_at_kexec_execute()
at kexec ‘load’ and kexec ‘execute’ respectively.

~Tushar

>> +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
>> +
>> +	if (!new_buffer) {
>> +		pr_err("%s: Dump measurements failed.\n", __func__);
>> +		resume = true;
>> +		goto out;
>> +	}
>> +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
>> +out:
>> +	kimage_unmap_segment(ima_kexec_buffer);
>> +	ima_kexec_buffer = NULL;
>> +
>> +	if (resume)
>> +		ima_measurements_resume();
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>>   #endif /* IMA_KEXEC */
>>   
>>   /*

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 08:01, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>
>> +/*
>> + * 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)
>> +{
>> +	void *new_buffer = NULL;
>> +	size_t new_buffer_size, cur_buffer_size;
>> +	bool resume = false;
>> +
>> +	if (!kexec_in_progress) {
>> +		pr_info("%s: No kexec in progress.\n", __func__);
>> +		return NOTIFY_OK;
>> +	}
>> +
>> +	if (!ima_kexec_buffer) {
>> +		pr_err("%s: Kexec buffer not set.\n", __func__);
>> +		return NOTIFY_OK;
>> +	}
>> +
>> +	ima_measurements_suspend();
>> +
>> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
>> +	new_buffer_size = ima_get_binary_runtime_size();
>> +	if (new_buffer_size > cur_buffer_size) {
>> +		pr_err("%s: Measurement list grew too large.\n", __func__);
>> +		resume = true;
>> +		goto out;
>> +	}
> This changes the current behavior of carrying as many measurements
> across kexec as possible.  True the measurement list won't verify
> against the TPM PCRs, but not copying the measurements leaves the
> impression there weren't any previous measurements.
>
> This also explains the reason for allocating an IMA buffer (patch 1/10)
> and not writing the measurements directly into the kexec buffer.
Thanks.

I will update this logic depending if we decide to use
ima_dump_measurement_list() at kexec ‘execute’, or combination of
ima_allocate_buf_at_kexec_load() and ima_populate_buf_at_kexec_execute()
at kexec ‘load’ and kexec ‘execute’ respectively.

~Tushar

>> +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
>> +
>> +	if (!new_buffer) {
>> +		pr_err("%s: Dump measurements failed.\n", __func__);
>> +		resume = true;
>> +		goto out;
>> +	}
>> +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
>> +out:
>> +	kimage_unmap_segment(ima_kexec_buffer);
>> +	ima_kexec_buffer = NULL;
>> +
>> +	if (resume)
>> +		ima_measurements_resume();
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>>   #endif /* IMA_KEXEC */
>>   
>>   /*

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 12:49, Mimi Zohar wrote:
> On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote:
>> Hi Tushar,
>>
>> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>>
>>> +/*
>>> + * 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)
>>> +{
>>> +	void *new_buffer = NULL;
>>> +	size_t new_buffer_size, cur_buffer_size;
>>> +	bool resume = false;
>>> +
>>> +	if (!kexec_in_progress) {
>>> +		pr_info("%s: No kexec in progress.\n", __func__);
>>> +		return NOTIFY_OK;
>>> +	}
>>> +
>>> +	if (!ima_kexec_buffer) {
>>> +		pr_err("%s: Kexec buffer not set.\n", __func__);
>>> +		return NOTIFY_OK;
>>> +	}
>>> +
>>> +	ima_measurements_suspend();
>>> +
>>> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
>>> +	new_buffer_size = ima_get_binary_runtime_size();
>>> +	if (new_buffer_size > cur_buffer_size) {
>>> +		pr_err("%s: Measurement list grew too large.\n", __func__);
>>> +		resume = true;
>>> +		goto out;
>>> +	}
>> This changes the current behavior of carrying as many measurements
>> across kexec as possible.  True the measurement list won't verify
>> against the TPM PCRs, but not copying the measurements leaves the
>> impression there weren't any previous measurements.
>>
>> This also explains the reason for allocating an IMA buffer (patch 1/10)
>> and not writing the measurements directly into the kexec buffer.
> If not carrying even a partial measurement list across kexec is
> desired, then in addition to the "boot_aggregate" record, define a new
> record containing the TPM pcrcounter.  With this information,
> attestation servers will at least be able to detect if the measurement
> list was truncated.
>
> thanks,
>
> Mimi
Sure.  Recording TPM pcrcounter at boot aggregate and
Kexec 'load' should provide the necessary information to the
attestation servers.  We can implement this if needed, based on how
rest of the series evolves.

~Tushar
>>> +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
>>> +
>>> +	if (!new_buffer) {
>>> +		pr_err("%s: Dump measurements failed.\n", __func__);
>>> +		resume = true;
>>> +		goto out;
>>> +	}
>>> +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
>>> +out:
>>> +	kimage_unmap_segment(ima_kexec_buffer);
>>> +	ima_kexec_buffer = NULL;
>>> +
>>> +	if (resume)
>>> +		ima_measurements_resume();
>>> +
>>> +	return NOTIFY_OK;
>>> +}
>>> +
>>>   #endif /* IMA_KEXEC */
>>>   
>>>   /*

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Tushar Sugandhi]

Adding Eric to cc.

On 7/7/23 12:49, Mimi Zohar wrote:
> On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote:
>> Hi Tushar,
>>
>> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>>
>>> +/*
>>> + * 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)
>>> +{
>>> +	void *new_buffer = NULL;
>>> +	size_t new_buffer_size, cur_buffer_size;
>>> +	bool resume = false;
>>> +
>>> +	if (!kexec_in_progress) {
>>> +		pr_info("%s: No kexec in progress.\n", __func__);
>>> +		return NOTIFY_OK;
>>> +	}
>>> +
>>> +	if (!ima_kexec_buffer) {
>>> +		pr_err("%s: Kexec buffer not set.\n", __func__);
>>> +		return NOTIFY_OK;
>>> +	}
>>> +
>>> +	ima_measurements_suspend();
>>> +
>>> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
>>> +	new_buffer_size = ima_get_binary_runtime_size();
>>> +	if (new_buffer_size > cur_buffer_size) {
>>> +		pr_err("%s: Measurement list grew too large.\n", __func__);
>>> +		resume = true;
>>> +		goto out;
>>> +	}
>> This changes the current behavior of carrying as many measurements
>> across kexec as possible.  True the measurement list won't verify
>> against the TPM PCRs, but not copying the measurements leaves the
>> impression there weren't any previous measurements.
>>
>> This also explains the reason for allocating an IMA buffer (patch 1/10)
>> and not writing the measurements directly into the kexec buffer.
> If not carrying even a partial measurement list across kexec is
> desired, then in addition to the "boot_aggregate" record, define a new
> record containing the TPM pcrcounter.  With this information,
> attestation servers will at least be able to detect if the measurement
> list was truncated.
>
> thanks,
>
> Mimi
Sure.  Recording TPM pcrcounter at boot aggregate and
Kexec 'load' should provide the necessary information to the
attestation servers.  We can implement this if needed, based on how
rest of the series evolves.

~Tushar
>>> +	ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer);
>>> +
>>> +	if (!new_buffer) {
>>> +		pr_err("%s: Dump measurements failed.\n", __func__);
>>> +		resume = true;
>>> +		goto out;
>>> +	}
>>> +	memcpy(ima_kexec_buffer, new_buffer, new_buffer_size);
>>> +out:
>>> +	kimage_unmap_segment(ima_kexec_buffer);
>>> +	ima_kexec_buffer = NULL;
>>> +
>>> +	if (resume)
>>> +		ima_measurements_resume();
>>> +
>>> +	return NOTIFY_OK;
>>> +}
>>> +
>>>   #endif /* IMA_KEXEC */
>>>   
>>>   /*

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 07/10] ima: remove function ima_dump_measurement_list

[Tushar Sugandhi]

Adding Eric to cc.


On 7/7/23 06:55, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> The ima_dump_measurement_list function was designed to iterate over the
>> IMA measurement list and store each entry into a buffer.  The buffer,
>> along with its size, would be returned to the caller.  However, the
>> function is no longer required in the IMA subsystem.  It previously served
>> to dump the measurement list during the kexec 'load' operation, but this
>> functionality has been replaced by an alternative approach in this patch
>> series.
>>
>> Remove the unused ima_dump_measurement_list function from the IMA
>> subsystem, to ensure a cleaner and more maintainable code.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Have you compiled the code and tested after applying each patch?
> Removing the caller before removing the code should have resulted in a
> warning.
>
Yes, I compiled each patch separately, and I did see this warning.
But removing ima_dump_measurement_list after implementing and
calling the alternate ima_allocate_buf_at_kexec_load() and
ima_populate_buf_at_kexec_execute() felt cleaner in terms of organizing
the patches.

I can remove the definition when I deprecate the usage, if we end up
using the alternate two functions.

~Tushar

Re: [PATCH 07/10] ima: remove function ima_dump_measurement_list

[Tushar Sugandhi]

Adding Eric to cc.


On 7/7/23 06:55, Mimi Zohar wrote:
> Hi Tushar,
>
> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>> The ima_dump_measurement_list function was designed to iterate over the
>> IMA measurement list and store each entry into a buffer.  The buffer,
>> along with its size, would be returned to the caller.  However, the
>> function is no longer required in the IMA subsystem.  It previously served
>> to dump the measurement list during the kexec 'load' operation, but this
>> functionality has been replaced by an alternative approach in this patch
>> series.
>>
>> Remove the unused ima_dump_measurement_list function from the IMA
>> subsystem, to ensure a cleaner and more maintainable code.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Have you compiled the code and tested after applying each patch?
> Removing the caller before removing the code should have resulted in a
> warning.
>
Yes, I compiled each patch separately, and I did see this warning.
But removing ima_dump_measurement_list after implementing and
calling the alternate ima_allocate_buf_at_kexec_load() and
ima_populate_buf_at_kexec_execute() felt cleaner in terms of organizing
the patches.

I can remove the definition when I deprecate the usage, if we end up
using the alternate two functions.

~Tushar

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[Tushar Sugandhi]

On 7/7/23 01:20, RuiRui Yang wrote:
> On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
>> The kexec_file_load syscall is used to load a new kernel for kexec.
>> The syscall needs to update its function to call ima_kexec_post_load, which
>> was implemented in a previous patch.  ima_kexec_post_load takes care of
>> mapping the measurement list for the next kernel and registering a reboot
>> notifier if it's not already registered.
>>
>> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
>> image has been loaded and prepared for kexec.  This ensures that the IMA
>> measurement list will be available to the next kernel after a kexec reboot.
>> This also ensures the measurements taken in the window between kexec load
>> and execute are captured and passed to the next kernel.
>>
>> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
>> so it can be properly used in the syscall.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   kernel/kexec_file.c     | 7 +++++++
>>   kernel/kexec_internal.h | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index f989f5f1933b..efe28e77280c 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
>>   }
>>   #endif
>>
>> +void kimage_file_post_load(struct kimage *image)
>> +{
>> +       ima_kexec_post_load(image);
>> +}
>> +
>>   /*
>>    * In file mode list of segments is prepared by kernel. Copy relevant
>>    * data from user space, do error checking, prepare segment list
>> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>>
>>          kimage_terminate(image);
>>
>> +       kimage_file_post_load(image);
> I think it should be only done for the reboot case,  please just
> exclude the kdump case here..
>
Thanks for the feedback RuiRui.  Appreciate it.

Conceptually I agree with you that this needs to be done only for reboot.
I need to figure out how to do it implementation wise.

If you can give me pointers/suggestions, that would help.

~Tushar
>> +
>>          ret = machine_kexec_post_load(image);
>>          if (ret)
>>                  goto out;
>> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
>> index 74da1409cd14..98dd5fcafaf0 100644
>> --- a/kernel/kexec_internal.h
>> +++ b/kernel/kexec_internal.h
>> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
>>
>>   #ifdef CONFIG_KEXEC_FILE
>>   #include <linux/purgatory.h>
>> +void kimage_file_post_load(struct kimage *image);
>>   void kimage_file_post_load_cleanup(struct kimage *image);
>>   extern char kexec_purgatory[];
>>   extern size_t kexec_purgatory_size;
>> --
>> 2.25.1
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
>>

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[Tushar Sugandhi]

On 7/7/23 01:20, RuiRui Yang wrote:
> On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
>> The kexec_file_load syscall is used to load a new kernel for kexec.
>> The syscall needs to update its function to call ima_kexec_post_load, which
>> was implemented in a previous patch.  ima_kexec_post_load takes care of
>> mapping the measurement list for the next kernel and registering a reboot
>> notifier if it's not already registered.
>>
>> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
>> image has been loaded and prepared for kexec.  This ensures that the IMA
>> measurement list will be available to the next kernel after a kexec reboot.
>> This also ensures the measurements taken in the window between kexec load
>> and execute are captured and passed to the next kernel.
>>
>> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
>> so it can be properly used in the syscall.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   kernel/kexec_file.c     | 7 +++++++
>>   kernel/kexec_internal.h | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index f989f5f1933b..efe28e77280c 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
>>   }
>>   #endif
>>
>> +void kimage_file_post_load(struct kimage *image)
>> +{
>> +       ima_kexec_post_load(image);
>> +}
>> +
>>   /*
>>    * In file mode list of segments is prepared by kernel. Copy relevant
>>    * data from user space, do error checking, prepare segment list
>> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>>
>>          kimage_terminate(image);
>>
>> +       kimage_file_post_load(image);
> I think it should be only done for the reboot case,  please just
> exclude the kdump case here..
>
Thanks for the feedback RuiRui.  Appreciate it.

Conceptually I agree with you that this needs to be done only for reboot.
I need to figure out how to do it implementation wise.

If you can give me pointers/suggestions, that would help.

~Tushar
>> +
>>          ret = machine_kexec_post_load(image);
>>          if (ret)
>>                  goto out;
>> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
>> index 74da1409cd14..98dd5fcafaf0 100644
>> --- a/kernel/kexec_internal.h
>> +++ b/kernel/kexec_internal.h
>> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
>>
>>   #ifdef CONFIG_KEXEC_FILE
>>   #include <linux/purgatory.h>
>> +void kimage_file_post_load(struct kimage *image);
>>   void kimage_file_post_load_cleanup(struct kimage *image);
>>   extern char kexec_purgatory[];
>>   extern size_t kexec_purgatory_size;
>> --
>> 2.25.1
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Stefan Berger]

On 7/11/23 14:41, Tushar Sugandhi wrote:
> Adding Eric to cc.
> 
> On 7/7/23 05:28, Stefan Berger wrote:
>>
>>
>> On 7/3/23 17:57, Tushar Sugandhi wrote:
>>> Currently, there's no mechanism to map and unmap segments to the kimage
>>> structure.  This functionality is needed when dealing with memory segments
>>> in the context of a kexec operation.
>>>
>>> The patch adds two new functions: kimage_map_segment() and
>>> kimage_unmap_segment().
>>>
>>> Implement kimage_map_segment() which takes a kimage pointer, an address,
>>> and a size.  Ensures that the entire segment is being mapped by comparing
>>> the given address and size to each segment in the kimage's segment array.
>>> Collect the source pages that correspond to the given address range,
>>> allocate an array of pointers to these pages, and map them to a contiguous
>>> range of virtual addresses.  If the mapping operation is successful, the
>>> function returns the start of this range.  Otherwise, it frees the page
>>> pointer array and returns NULL.
>>>
>>> Implement kimage_unmap_segment() that takes a pointer to a segment buffer
>>> and unmaps it using vunmap().
>>>
>>> Finally, move for_each_kimage_entry() macro to kexec.h.
>>>
>>> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
>>> avoid memory leaks and ensure that all mapped segments are properly
>>> unmapped when they're no longer needed.
>>>
>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>
>>> +
>>> +    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++] = phys_to_page(src_page_addr);
>>
>> Since phys_to_page is not defined on many/most architectures I change it for ppc64 and have successfully used the following:
>>
>> +                               src_pages[i++] = virt_to_page(__va(src_page_addr))
>>
>>
>> After several kexecs the following check still works:
>>
>> # evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
>> Matched per TPM bank calculated digest(s).
>>
>>
>>    Stefan
> Thank you so much Stefan for reviewing this series, and catching this
> issue.  Are you suggesting I should use virt_to_page on all architectures
> unconditionally, or use it only when phys_to_page is not available?

I would try to used it on all architectures.

    Stefan

> 
> ~Tushar
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Stefan Berger]

On 7/11/23 14:41, Tushar Sugandhi wrote:
> Adding Eric to cc.
> 
> On 7/7/23 05:28, Stefan Berger wrote:
>>
>>
>> On 7/3/23 17:57, Tushar Sugandhi wrote:
>>> Currently, there's no mechanism to map and unmap segments to the kimage
>>> structure.  This functionality is needed when dealing with memory segments
>>> in the context of a kexec operation.
>>>
>>> The patch adds two new functions: kimage_map_segment() and
>>> kimage_unmap_segment().
>>>
>>> Implement kimage_map_segment() which takes a kimage pointer, an address,
>>> and a size.  Ensures that the entire segment is being mapped by comparing
>>> the given address and size to each segment in the kimage's segment array.
>>> Collect the source pages that correspond to the given address range,
>>> allocate an array of pointers to these pages, and map them to a contiguous
>>> range of virtual addresses.  If the mapping operation is successful, the
>>> function returns the start of this range.  Otherwise, it frees the page
>>> pointer array and returns NULL.
>>>
>>> Implement kimage_unmap_segment() that takes a pointer to a segment buffer
>>> and unmaps it using vunmap().
>>>
>>> Finally, move for_each_kimage_entry() macro to kexec.h.
>>>
>>> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
>>> avoid memory leaks and ensure that all mapped segments are properly
>>> unmapped when they're no longer needed.
>>>
>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>
>>> +
>>> +    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++] = phys_to_page(src_page_addr);
>>
>> Since phys_to_page is not defined on many/most architectures I change it for ppc64 and have successfully used the following:
>>
>> +                               src_pages[i++] = virt_to_page(__va(src_page_addr))
>>
>>
>> After several kexecs the following check still works:
>>
>> # evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements
>> Matched per TPM bank calculated digest(s).
>>
>>
>>    Stefan
> Thank you so much Stefan for reviewing this series, and catching this
> issue.  Are you suggesting I should use virt_to_page on all architectures
> unconditionally, or use it only when phys_to_page is not available?

I would try to used it on all architectures.

    Stefan

> 
> ~Tushar
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Stefan Berger]

On 7/3/23 17:57, Tushar Sugandhi wrote:
> The IMA subsystem needs a dedicated mechanism to reserve extra memory for
> measurements added between the kexec 'load' and kexec 'execute'.
> 
> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
> buffer.
> 
> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
> sufficient to handle the additional measurements.  This should be as per
> the system requirements and based on the number of additional measurements
> expected during the window from kexec 'load' to kexec 'execute'.

This could be the most problematic part if the 'execute' happens much later
than the 'load' with lots of measurement activity in between. I am wondering
whether not doing anything at 'load' time and doing the whole work at 'execute' time
wouldn't be the right thing to do ?

Otherwise, if we wanted the work to be split as you suggest, could you
- krealloc the src_pages (now in kimage_map_segment) to add space for a few more pages needed for the additional measurements
- add those few more pages to src_pages
- vunmap the previous mapping
- vmap the extended src_pages array
- update ima_kexec_file.buf with the diff between the new and old vmap'ed addresses
- append to the existing log

This presumably would help resolve this potential issue.

The src_pages is currently not kfree'd -- may be a memory leak.

Regards,
    Stefan

> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>   security/integrity/ima/ima.h       |  2 ++
>   security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>   2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c29db699c996..2ffda9449b9b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>   
>   #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>   
> +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
> +
>   /* current content of the policy */
>   extern int ima_policy_flag;
>   
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 858b67689701..7deb8df31485 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -188,31 +188,30 @@ 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_segment_size;
>   	int ret;
>   
>   	/*
> -	 * Reserve an extra half page of memory for additional measurements
> -	 * added during the kexec load.
> +	 * Reserve extra memory for measurements added in the window from
> +	 * kexec 'load' to kexec 'execute'.
>   	 */
> -	binary_runtime_size = ima_get_binary_runtime_size();
> +	binary_runtime_size = ima_get_binary_runtime_size() +
> +			      sizeof(struct ima_kexec_hdr) +
> +			      IMA_KEXEC_EXTRA_SIZE;
> +
>   	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");
>   		return;
>   	}
>   
> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> -				  kexec_segment_size);
> -	if (!kexec_buffer) {
> -		pr_err("Not enough memory for the kexec measurement buffer.\n");
> +	ret = ima_allocate_buf_at_kexec_load();
> +	if (ret < 0)
>   		return;
> -	}
>   
>   	kbuf.buffer = kexec_buffer;
>   	kbuf.bufsz = kexec_buffer_size;

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Stefan Berger]

On 7/3/23 17:57, Tushar Sugandhi wrote:
> The IMA subsystem needs a dedicated mechanism to reserve extra memory for
> measurements added between the kexec 'load' and kexec 'execute'.
> 
> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to align
> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
> buffer.
> 
> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
> sufficient to handle the additional measurements.  This should be as per
> the system requirements and based on the number of additional measurements
> expected during the window from kexec 'load' to kexec 'execute'.

This could be the most problematic part if the 'execute' happens much later
than the 'load' with lots of measurement activity in between. I am wondering
whether not doing anything at 'load' time and doing the whole work at 'execute' time
wouldn't be the right thing to do ?

Otherwise, if we wanted the work to be split as you suggest, could you
- krealloc the src_pages (now in kimage_map_segment) to add space for a few more pages needed for the additional measurements
- add those few more pages to src_pages
- vunmap the previous mapping
- vmap the extended src_pages array
- update ima_kexec_file.buf with the diff between the new and old vmap'ed addresses
- append to the existing log

This presumably would help resolve this potential issue.

The src_pages is currently not kfree'd -- may be a memory leak.

Regards,
    Stefan

> 
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> ---
>   security/integrity/ima/ima.h       |  2 ++
>   security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>   2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c29db699c996..2ffda9449b9b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>   
>   #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>   
> +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
> +
>   /* current content of the policy */
>   extern int ima_policy_flag;
>   
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 858b67689701..7deb8df31485 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -188,31 +188,30 @@ 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_segment_size;
>   	int ret;
>   
>   	/*
> -	 * Reserve an extra half page of memory for additional measurements
> -	 * added during the kexec load.
> +	 * Reserve extra memory for measurements added in the window from
> +	 * kexec 'load' to kexec 'execute'.
>   	 */
> -	binary_runtime_size = ima_get_binary_runtime_size();
> +	binary_runtime_size = ima_get_binary_runtime_size() +
> +			      sizeof(struct ima_kexec_hdr) +
> +			      IMA_KEXEC_EXTRA_SIZE;
> +
>   	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");
>   		return;
>   	}
>   
> -	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> -				  kexec_segment_size);
> -	if (!kexec_buffer) {
> -		pr_err("Not enough memory for the kexec measurement buffer.\n");
> +	ret = ima_allocate_buf_at_kexec_load();
> +	if (ret < 0)
>   		return;
> -	}
>   
>   	kbuf.buffer = kexec_buffer;
>   	kbuf.bufsz = kexec_buffer_size;

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Mimi Zohar]

On Tue, 2023-07-11 at 10:59 -0700, Tushar Sugandhi wrote:
> Adding Eric to cc.
> 
> On 7/7/23 06:00, Mimi Zohar wrote:
> > Hi Tushar,
> >
> > On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> >> IMA does not provide a mechanism to allocate memory for IMA log storage
> >> during kexec operation.
> > The IMA measurement list is currently being carried across kexec, so
> > obviously a buffer is being allocated for it.  IMA not allocating
> > memory for the measurment list is not the problem statement.  Please
> > concisely provide the problem statement, explaining why IMA needs to
> > allocate the buffer.
> >
> I meant IMA does not provide separate functions to allocate buffer and
> populate measurements.  Both operations are wrapped in an atomic
> ima_dump_measurement_list().

Ok.

> As I mentioned in the comment in the cover letter, if there is no such
> technical limitation to allocate the buffer and copy the measurements at
> kexec ‘execute’ – I will make the necessary code changes and update the
> above line in the patch description accordingly.

The "normal" way of making this type of change would be to split the
existing ima_dump_measurement_list() function.  Copying the measurement
list would still be named ima_dump_measurement_list().  The other could
be named ima_alloc_kexec_buf().  Both functions initially would be
called.

Eric, besides updating the buffer at kexec execute, is there anything
else that needs to be done (e.g. updating digests)?

> >> The function should handle the scenario where
> >> the kexec load is called multiple times.
> > Currently the buffer is being freed with the kexec 'unload'.  With this
> > patch IMA is allocating a buffer for the measurement list, which needs
> > to be freed independently of the kexec 'unload'.

> If we end up allocating the buffer at kexec ‘execute’ (which results in
> soft boot to next Kernel) – is it technically possible that
> kexec ‘unload’ being called after calling kexec ‘execute’?

> If not, should I still free the buffer at kexec ‘unload’ in this
> scenario?

The question is how to access the buffer once kexec_add_buffer() is
called.

Mimi

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Mimi Zohar]

On Tue, 2023-07-11 at 10:59 -0700, Tushar Sugandhi wrote:
> Adding Eric to cc.
> 
> On 7/7/23 06:00, Mimi Zohar wrote:
> > Hi Tushar,
> >
> > On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> >> IMA does not provide a mechanism to allocate memory for IMA log storage
> >> during kexec operation.
> > The IMA measurement list is currently being carried across kexec, so
> > obviously a buffer is being allocated for it.  IMA not allocating
> > memory for the measurment list is not the problem statement.  Please
> > concisely provide the problem statement, explaining why IMA needs to
> > allocate the buffer.
> >
> I meant IMA does not provide separate functions to allocate buffer and
> populate measurements.  Both operations are wrapped in an atomic
> ima_dump_measurement_list().

Ok.

> As I mentioned in the comment in the cover letter, if there is no such
> technical limitation to allocate the buffer and copy the measurements at
> kexec ‘execute’ – I will make the necessary code changes and update the
> above line in the patch description accordingly.

The "normal" way of making this type of change would be to split the
existing ima_dump_measurement_list() function.  Copying the measurement
list would still be named ima_dump_measurement_list().  The other could
be named ima_alloc_kexec_buf().  Both functions initially would be
called.

Eric, besides updating the buffer at kexec execute, is there anything
else that needs to be done (e.g. updating digests)?

> >> The function should handle the scenario where
> >> the kexec load is called multiple times.
> > Currently the buffer is being freed with the kexec 'unload'.  With this
> > patch IMA is allocating a buffer for the measurement list, which needs
> > to be freed independently of the kexec 'unload'.

> If we end up allocating the buffer at kexec ‘execute’ (which results in
> soft boot to next Kernel) – is it technically possible that
> kexec ‘unload’ being called after calling kexec ‘execute’?

> If not, should I still free the buffer at kexec ‘unload’ in this
> scenario?

The question is how to access the buffer once kexec_add_buffer() is
called.

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[RuiRui Yang]

On Wed, 12 Jul 2023 at 03:15, Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
>
>
> On 7/7/23 01:20, RuiRui Yang wrote:
> > On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
> > <tusharsu@linux.microsoft.com> wrote:
> >> The kexec_file_load syscall is used to load a new kernel for kexec.
> >> The syscall needs to update its function to call ima_kexec_post_load, which
> >> was implemented in a previous patch.  ima_kexec_post_load takes care of
> >> mapping the measurement list for the next kernel and registering a reboot
> >> notifier if it's not already registered.
> >>
> >> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
> >> image has been loaded and prepared for kexec.  This ensures that the IMA
> >> measurement list will be available to the next kernel after a kexec reboot.
> >> This also ensures the measurements taken in the window between kexec load
> >> and execute are captured and passed to the next kernel.
> >>
> >> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
> >> so it can be properly used in the syscall.
> >>
> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> >> ---
> >>   kernel/kexec_file.c     | 7 +++++++
> >>   kernel/kexec_internal.h | 1 +
> >>   2 files changed, 8 insertions(+)
> >>
> >> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> >> index f989f5f1933b..efe28e77280c 100644
> >> --- a/kernel/kexec_file.c
> >> +++ b/kernel/kexec_file.c
> >> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
> >>   }
> >>   #endif
> >>
> >> +void kimage_file_post_load(struct kimage *image)
> >> +{
> >> +       ima_kexec_post_load(image);
> >> +}
> >> +
> >>   /*
> >>    * In file mode list of segments is prepared by kernel. Copy relevant
> >>    * data from user space, do error checking, prepare segment list
> >> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> >>
> >>          kimage_terminate(image);
> >>
> >> +       kimage_file_post_load(image);
> > I think it should be only done for the reboot case,  please just
> > exclude the kdump case here..
> >
> Thanks for the feedback RuiRui.  Appreciate it.
>
> Conceptually I agree with you that this needs to be done only for reboot.
> I need to figure out how to do it implementation wise.
>
> If you can give me pointers/suggestions, that would help.

Hi Tushar,

You can check the flags argument in the function
if (flags & KEXEC_FILE_ON_CRASH) is true then this is a kdump kernel
loading, just skip the kimage_file_post_load in that case?

>
> ~Tushar
> >> +
> >>          ret = machine_kexec_post_load(image);
> >>          if (ret)
> >>                  goto out;
> >> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
> >> index 74da1409cd14..98dd5fcafaf0 100644
> >> --- a/kernel/kexec_internal.h
> >> +++ b/kernel/kexec_internal.h
> >> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
> >>
> >>   #ifdef CONFIG_KEXEC_FILE
> >>   #include <linux/purgatory.h>
> >> +void kimage_file_post_load(struct kimage *image);
> >>   void kimage_file_post_load_cleanup(struct kimage *image);
> >>   extern char kexec_purgatory[];
> >>   extern size_t kexec_purgatory_size;
> >> --
> >> 2.25.1
> >>
> >>
> >> _______________________________________________
> >> kexec mailing list
> >> kexec@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/kexec
> >>
>

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[RuiRui Yang]

On Wed, 12 Jul 2023 at 03:15, Tushar Sugandhi
<tusharsu@linux.microsoft.com> wrote:
>
>
> On 7/7/23 01:20, RuiRui Yang wrote:
> > On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
> > <tusharsu@linux.microsoft.com> wrote:
> >> The kexec_file_load syscall is used to load a new kernel for kexec.
> >> The syscall needs to update its function to call ima_kexec_post_load, which
> >> was implemented in a previous patch.  ima_kexec_post_load takes care of
> >> mapping the measurement list for the next kernel and registering a reboot
> >> notifier if it's not already registered.
> >>
> >> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
> >> image has been loaded and prepared for kexec.  This ensures that the IMA
> >> measurement list will be available to the next kernel after a kexec reboot.
> >> This also ensures the measurements taken in the window between kexec load
> >> and execute are captured and passed to the next kernel.
> >>
> >> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
> >> so it can be properly used in the syscall.
> >>
> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> >> ---
> >>   kernel/kexec_file.c     | 7 +++++++
> >>   kernel/kexec_internal.h | 1 +
> >>   2 files changed, 8 insertions(+)
> >>
> >> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> >> index f989f5f1933b..efe28e77280c 100644
> >> --- a/kernel/kexec_file.c
> >> +++ b/kernel/kexec_file.c
> >> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
> >>   }
> >>   #endif
> >>
> >> +void kimage_file_post_load(struct kimage *image)
> >> +{
> >> +       ima_kexec_post_load(image);
> >> +}
> >> +
> >>   /*
> >>    * In file mode list of segments is prepared by kernel. Copy relevant
> >>    * data from user space, do error checking, prepare segment list
> >> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> >>
> >>          kimage_terminate(image);
> >>
> >> +       kimage_file_post_load(image);
> > I think it should be only done for the reboot case,  please just
> > exclude the kdump case here..
> >
> Thanks for the feedback RuiRui.  Appreciate it.
>
> Conceptually I agree with you that this needs to be done only for reboot.
> I need to figure out how to do it implementation wise.
>
> If you can give me pointers/suggestions, that would help.

Hi Tushar,

You can check the flags argument in the function
if (flags & KEXEC_FILE_ON_CRASH) is true then this is a kdump kernel
loading, just skip the kimage_file_post_load in that case?

>
> ~Tushar
> >> +
> >>          ret = machine_kexec_post_load(image);
> >>          if (ret)
> >>                  goto out;
> >> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
> >> index 74da1409cd14..98dd5fcafaf0 100644
> >> --- a/kernel/kexec_internal.h
> >> +++ b/kernel/kexec_internal.h
> >> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
> >>
> >>   #ifdef CONFIG_KEXEC_FILE
> >>   #include <linux/purgatory.h>
> >> +void kimage_file_post_load(struct kimage *image);
> >>   void kimage_file_post_load_cleanup(struct kimage *image);
> >>   extern char kexec_purgatory[];
> >>   extern size_t kexec_purgatory_size;
> >> --
> >> 2.25.1
> >>
> >>
> >> _______________________________________________
> >> kexec mailing list
> >> kexec@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/kexec
> >>
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Mimi Zohar]

On Tue, 2023-07-11 at 12:08 -0700, Tushar Sugandhi wrote:
> Adding Eric to cc.
> 
> On 7/7/23 12:49, Mimi Zohar wrote:
> > On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote:
> >> Hi Tushar,
> >>
> >> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> >>
> >>> +/*
> >>> + * 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)
> >>> +{
> >>> +	void *new_buffer = NULL;
> >>> +	size_t new_buffer_size, cur_buffer_size;
> >>> +	bool resume = false;
> >>> +
> >>> +	if (!kexec_in_progress) {
> >>> +		pr_info("%s: No kexec in progress.\n", __func__);
> >>> +		return NOTIFY_OK;
> >>> +	}
> >>> +
> >>> +	if (!ima_kexec_buffer) {
> >>> +		pr_err("%s: Kexec buffer not set.\n", __func__);
> >>> +		return NOTIFY_OK;
> >>> +	}
> >>> +
> >>> +	ima_measurements_suspend();
> >>> +
> >>> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
> >>> +	new_buffer_size = ima_get_binary_runtime_size();
> >>> +	if (new_buffer_size > cur_buffer_size) {
> >>> +		pr_err("%s: Measurement list grew too large.\n", __func__);
> >>> +		resume = true;
> >>> +		goto out;
> >>> +	}
> >> This changes the current behavior of carrying as many measurements
> >> across kexec as possible.  True the measurement list won't verify
> >> against the TPM PCRs, but not copying the measurements leaves the
> >> impression there weren't any previous measurements.
> >>
> >> This also explains the reason for allocating an IMA buffer (patch 1/10)
> >> and not writing the measurements directly into the kexec buffer.
> > If not carrying even a partial measurement list across kexec is
> > desired, then in addition to the "boot_aggregate" record, define a new
> > record containing the TPM pcrcounter.  With this information,
> > attestation servers will at least be able to detect if the measurement
> > list was truncated.

> Sure.  Recording TPM pcrcounter at boot aggregate and
> Kexec 'load' should provide the necessary information to the
> attestation servers.  We can implement this if needed, based on how
> rest of the series evolves.

Recording the TPM pcrcounter should be done independently of this patch
set.  This patch set would have a dependency on it.

-- 
thanks,

Mimi

Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements

[Mimi Zohar]

On Tue, 2023-07-11 at 12:08 -0700, Tushar Sugandhi wrote:
> Adding Eric to cc.
> 
> On 7/7/23 12:49, Mimi Zohar wrote:
> > On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote:
> >> Hi Tushar,
> >>
> >> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
> >>
> >>> +/*
> >>> + * 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)
> >>> +{
> >>> +	void *new_buffer = NULL;
> >>> +	size_t new_buffer_size, cur_buffer_size;
> >>> +	bool resume = false;
> >>> +
> >>> +	if (!kexec_in_progress) {
> >>> +		pr_info("%s: No kexec in progress.\n", __func__);
> >>> +		return NOTIFY_OK;
> >>> +	}
> >>> +
> >>> +	if (!ima_kexec_buffer) {
> >>> +		pr_err("%s: Kexec buffer not set.\n", __func__);
> >>> +		return NOTIFY_OK;
> >>> +	}
> >>> +
> >>> +	ima_measurements_suspend();
> >>> +
> >>> +	cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr);
> >>> +	new_buffer_size = ima_get_binary_runtime_size();
> >>> +	if (new_buffer_size > cur_buffer_size) {
> >>> +		pr_err("%s: Measurement list grew too large.\n", __func__);
> >>> +		resume = true;
> >>> +		goto out;
> >>> +	}
> >> This changes the current behavior of carrying as many measurements
> >> across kexec as possible.  True the measurement list won't verify
> >> against the TPM PCRs, but not copying the measurements leaves the
> >> impression there weren't any previous measurements.
> >>
> >> This also explains the reason for allocating an IMA buffer (patch 1/10)
> >> and not writing the measurements directly into the kexec buffer.
> > If not carrying even a partial measurement list across kexec is
> > desired, then in addition to the "boot_aggregate" record, define a new
> > record containing the TPM pcrcounter.  With this information,
> > attestation servers will at least be able to detect if the measurement
> > list was truncated.

> Sure.  Recording TPM pcrcounter at boot aggregate and
> Kexec 'load' should provide the necessary information to the
> attestation servers.  We can implement this if needed, based on how
> rest of the series evolves.

Recording the TPM pcrcounter should be done independently of this patch
set.  This patch set would have a dependency on it.

-- 
thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[Tushar Sugandhi]

On 7/11/23 18:28, RuiRui Yang wrote:
> On Wed, 12 Jul 2023 at 03:15, Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
>>
>> On 7/7/23 01:20, RuiRui Yang wrote:
>>> On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
>>> <tusharsu@linux.microsoft.com> wrote:
>>>> The kexec_file_load syscall is used to load a new kernel for kexec.
>>>> The syscall needs to update its function to call ima_kexec_post_load, which
>>>> was implemented in a previous patch.  ima_kexec_post_load takes care of
>>>> mapping the measurement list for the next kernel and registering a reboot
>>>> notifier if it's not already registered.
>>>>
>>>> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
>>>> image has been loaded and prepared for kexec.  This ensures that the IMA
>>>> measurement list will be available to the next kernel after a kexec reboot.
>>>> This also ensures the measurements taken in the window between kexec load
>>>> and execute are captured and passed to the next kernel.
>>>>
>>>> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
>>>> so it can be properly used in the syscall.
>>>>
>>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>> ---
>>>>    kernel/kexec_file.c     | 7 +++++++
>>>>    kernel/kexec_internal.h | 1 +
>>>>    2 files changed, 8 insertions(+)
>>>>
>>>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>>>> index f989f5f1933b..efe28e77280c 100644
>>>> --- a/kernel/kexec_file.c
>>>> +++ b/kernel/kexec_file.c
>>>> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
>>>>    }
>>>>    #endif
>>>>
>>>> +void kimage_file_post_load(struct kimage *image)
>>>> +{
>>>> +       ima_kexec_post_load(image);
>>>> +}
>>>> +
>>>>    /*
>>>>     * In file mode list of segments is prepared by kernel. Copy relevant
>>>>     * data from user space, do error checking, prepare segment list
>>>> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>>>>
>>>>           kimage_terminate(image);
>>>>
>>>> +       kimage_file_post_load(image);
>>> I think it should be only done for the reboot case,  please just
>>> exclude the kdump case here..
>>>
>> Thanks for the feedback RuiRui.  Appreciate it.
>>
>> Conceptually I agree with you that this needs to be done only for reboot.
>> I need to figure out how to do it implementation wise.
>>
>> If you can give me pointers/suggestions, that would help.
> Hi Tushar,
>
> You can check the flags argument in the function
> if (flags & KEXEC_FILE_ON_CRASH) is true then this is a kdump kernel
> loading, just skip the kimage_file_post_load in that case?
Great.  Thanks for the pointer.  Will do.
~Tushar
>> ~Tushar
>>>> +
>>>>           ret = machine_kexec_post_load(image);
>>>>           if (ret)
>>>>                   goto out;
>>>> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
>>>> index 74da1409cd14..98dd5fcafaf0 100644
>>>> --- a/kernel/kexec_internal.h
>>>> +++ b/kernel/kexec_internal.h
>>>> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
>>>>
>>>>    #ifdef CONFIG_KEXEC_FILE
>>>>    #include <linux/purgatory.h>
>>>> +void kimage_file_post_load(struct kimage *image);
>>>>    void kimage_file_post_load_cleanup(struct kimage *image);
>>>>    extern char kexec_purgatory[];
>>>>    extern size_t kexec_purgatory_size;
>>>> --
>>>> 2.25.1
>>>>
>>>>
>>>> _______________________________________________
>>>> kexec mailing list
>>>> kexec@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>>

Re: [PATCH 10/10] kexec: update kexec_file_load syscall to call ima_kexec_post_load

[Tushar Sugandhi]

On 7/11/23 18:28, RuiRui Yang wrote:
> On Wed, 12 Jul 2023 at 03:15, Tushar Sugandhi
> <tusharsu@linux.microsoft.com> wrote:
>>
>> On 7/7/23 01:20, RuiRui Yang wrote:
>>> On Tue, 4 Jul 2023 at 05:58, Tushar Sugandhi
>>> <tusharsu@linux.microsoft.com> wrote:
>>>> The kexec_file_load syscall is used to load a new kernel for kexec.
>>>> The syscall needs to update its function to call ima_kexec_post_load, which
>>>> was implemented in a previous patch.  ima_kexec_post_load takes care of
>>>> mapping the measurement list for the next kernel and registering a reboot
>>>> notifier if it's not already registered.
>>>>
>>>> Modify the kexec_file_load syscall to call ima_kexec_post_load after the
>>>> image has been loaded and prepared for kexec.  This ensures that the IMA
>>>> measurement list will be available to the next kernel after a kexec reboot.
>>>> This also ensures the measurements taken in the window between kexec load
>>>> and execute are captured and passed to the next kernel.
>>>>
>>>> Declare the kimage_file_post_load function in the kernel/kexec_internal.h,
>>>> so it can be properly used in the syscall.
>>>>
>>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>> ---
>>>>    kernel/kexec_file.c     | 7 +++++++
>>>>    kernel/kexec_internal.h | 1 +
>>>>    2 files changed, 8 insertions(+)
>>>>
>>>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>>>> index f989f5f1933b..efe28e77280c 100644
>>>> --- a/kernel/kexec_file.c
>>>> +++ b/kernel/kexec_file.c
>>>> @@ -184,6 +184,11 @@ kimage_validate_signature(struct kimage *image)
>>>>    }
>>>>    #endif
>>>>
>>>> +void kimage_file_post_load(struct kimage *image)
>>>> +{
>>>> +       ima_kexec_post_load(image);
>>>> +}
>>>> +
>>>>    /*
>>>>     * In file mode list of segments is prepared by kernel. Copy relevant
>>>>     * data from user space, do error checking, prepare segment list
>>>> @@ -399,6 +404,8 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>>>>
>>>>           kimage_terminate(image);
>>>>
>>>> +       kimage_file_post_load(image);
>>> I think it should be only done for the reboot case,  please just
>>> exclude the kdump case here..
>>>
>> Thanks for the feedback RuiRui.  Appreciate it.
>>
>> Conceptually I agree with you that this needs to be done only for reboot.
>> I need to figure out how to do it implementation wise.
>>
>> If you can give me pointers/suggestions, that would help.
> Hi Tushar,
>
> You can check the flags argument in the function
> if (flags & KEXEC_FILE_ON_CRASH) is true then this is a kdump kernel
> loading, just skip the kimage_file_post_load in that case?
Great.  Thanks for the pointer.  Will do.
~Tushar
>> ~Tushar
>>>> +
>>>>           ret = machine_kexec_post_load(image);
>>>>           if (ret)
>>>>                   goto out;
>>>> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
>>>> index 74da1409cd14..98dd5fcafaf0 100644
>>>> --- a/kernel/kexec_internal.h
>>>> +++ b/kernel/kexec_internal.h
>>>> @@ -30,6 +30,7 @@ static inline void kexec_unlock(void)
>>>>
>>>>    #ifdef CONFIG_KEXEC_FILE
>>>>    #include <linux/purgatory.h>
>>>> +void kimage_file_post_load(struct kimage *image);
>>>>    void kimage_file_post_load_cleanup(struct kimage *image);
>>>>    extern char kexec_purgatory[];
>>>>    extern size_t kexec_purgatory_size;
>>>> --
>>>> 2.25.1
>>>>
>>>>
>>>> _______________________________________________
>>>> kexec mailing list
>>>> kexec@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Tushar Sugandhi]

On 7/11/23 13:16, Stefan Berger wrote:
>
>
> On 7/3/23 17:57, Tushar Sugandhi wrote:
>> The IMA subsystem needs a dedicated mechanism to reserve extra memory 
>> for
>> measurements added between the kexec 'load' and kexec 'execute'.
>>
>> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
>> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
>> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to 
>> align
>> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
>> buffer.
>>
>> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
>> sufficient to handle the additional measurements.  This should be as per
>> the system requirements and based on the number of additional 
>> measurements
>> expected during the window from kexec 'load' to kexec 'execute'.
>
> This could be the most problematic part if the 'execute' happens much 
> later
> than the 'load' with lots of measurement activity in between. I am 
> wondering
> whether not doing anything at 'load' time and doing the whole work at 
> 'execute' time
Technically I don't want to do anything at kexec 'load' time.
But as I said elsewhere in this series, I am under impression that 
segment allocation
must happen at kexec 'load' time.


But if that understanding is wrong - then we can move the entire logic 
to kexec 'execute' time.

I am currently trying to validate if moving the logic to kexec 'execute' 
works.
> wouldn't be the right thing to do ?
>
> Otherwise, if we wanted the work to be split as you suggest, could you
> - krealloc the src_pages (now in kimage_map_segment) to add space for 
> a few more pages needed for the additional measurements
> - add those few more pages to src_pages
> - vunmap the previous mapping
> - vmap the extended src_pages array
> - update ima_kexec_file.buf with the diff between the new and old 
> vmap'ed addresses
> - append to the existing log
>
> This presumably would help resolve this potential issue.
Thanks for the detailed thoughts.
I will investigate if this approach is needed, based on my comment above.
If it is needed and feasible, I will incorporate it in the next iteration.

Thanks again.

~Tushar

>
> The src_pages is currently not kfree'd -- may be a memory leak.
>
> Regards,
>    Stefan
>
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima.h       |  2 ++
>>   security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index c29db699c996..2ffda9449b9b 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, 
>> TPM_PCR10 = 10 };
>>     #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks 
>> : 0)
>>   +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
>> +
>>   /* current content of the policy */
>>   extern int ima_policy_flag;
>>   diff --git a/security/integrity/ima/ima_kexec.c 
>> b/security/integrity/ima/ima_kexec.c
>> index 858b67689701..7deb8df31485 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -188,31 +188,30 @@ 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_segment_size;
>>       int ret;
>>         /*
>> -     * Reserve an extra half page of memory for additional measurements
>> -     * added during the kexec load.
>> +     * Reserve extra memory for measurements added in the window from
>> +     * kexec 'load' to kexec 'execute'.
>>        */
>> -    binary_runtime_size = ima_get_binary_runtime_size();
>> +    binary_runtime_size = ima_get_binary_runtime_size() +
>> +                  sizeof(struct ima_kexec_hdr) +
>> +                  IMA_KEXEC_EXTRA_SIZE;
>> +
>>       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");
>>           return;
>>       }
>>   -    ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
>> -                  kexec_segment_size);
>> -    if (!kexec_buffer) {
>> -        pr_err("Not enough memory for the kexec measurement 
>> buffer.\n");
>> +    ret = ima_allocate_buf_at_kexec_load();
>> +    if (ret < 0)
>>           return;
>> -    }
>>         kbuf.buffer = kexec_buffer;
>>       kbuf.bufsz = kexec_buffer_size;

Re: [PATCH 03/10] ima: allocate buffer at kexec load to hold ima measurements

[Tushar Sugandhi]

On 7/11/23 13:16, Stefan Berger wrote:
>
>
> On 7/3/23 17:57, Tushar Sugandhi wrote:
>> The IMA subsystem needs a dedicated mechanism to reserve extra memory 
>> for
>> measurements added between the kexec 'load' and kexec 'execute'.
>>
>> Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
>> taking ima binary runtime measurements size, size of ima_kexec_hdr, and
>> IMA_KEXEC_EXTRA_SIZE into account.  Adjust the kexec_segment_size to 
>> align
>> to the PAGE_SIZE.  Call ima_allocate_buf_at_kexec_load() to allocate the
>> buffer.
>>
>> This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
>> sufficient to handle the additional measurements.  This should be as per
>> the system requirements and based on the number of additional 
>> measurements
>> expected during the window from kexec 'load' to kexec 'execute'.
>
> This could be the most problematic part if the 'execute' happens much 
> later
> than the 'load' with lots of measurement activity in between. I am 
> wondering
> whether not doing anything at 'load' time and doing the whole work at 
> 'execute' time
Technically I don't want to do anything at kexec 'load' time.
But as I said elsewhere in this series, I am under impression that 
segment allocation
must happen at kexec 'load' time.


But if that understanding is wrong - then we can move the entire logic 
to kexec 'execute' time.

I am currently trying to validate if moving the logic to kexec 'execute' 
works.
> wouldn't be the right thing to do ?
>
> Otherwise, if we wanted the work to be split as you suggest, could you
> - krealloc the src_pages (now in kimage_map_segment) to add space for 
> a few more pages needed for the additional measurements
> - add those few more pages to src_pages
> - vunmap the previous mapping
> - vmap the extended src_pages array
> - update ima_kexec_file.buf with the diff between the new and old 
> vmap'ed addresses
> - append to the existing log
>
> This presumably would help resolve this potential issue.
Thanks for the detailed thoughts.
I will investigate if this approach is needed, based on my comment above.
If it is needed and feasible, I will incorporate it in the next iteration.

Thanks again.

~Tushar

>
> The src_pages is currently not kfree'd -- may be a memory leak.
>
> Regards,
>    Stefan
>
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima.h       |  2 ++
>>   security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index c29db699c996..2ffda9449b9b 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, 
>> TPM_PCR10 = 10 };
>>     #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks 
>> : 0)
>>   +#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
>> +
>>   /* current content of the policy */
>>   extern int ima_policy_flag;
>>   diff --git a/security/integrity/ima/ima_kexec.c 
>> b/security/integrity/ima/ima_kexec.c
>> index 858b67689701..7deb8df31485 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -188,31 +188,30 @@ 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_segment_size;
>>       int ret;
>>         /*
>> -     * Reserve an extra half page of memory for additional measurements
>> -     * added during the kexec load.
>> +     * Reserve extra memory for measurements added in the window from
>> +     * kexec 'load' to kexec 'execute'.
>>        */
>> -    binary_runtime_size = ima_get_binary_runtime_size();
>> +    binary_runtime_size = ima_get_binary_runtime_size() +
>> +                  sizeof(struct ima_kexec_hdr) +
>> +                  IMA_KEXEC_EXTRA_SIZE;
>> +
>>       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");
>>           return;
>>       }
>>   -    ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
>> -                  kexec_segment_size);
>> -    if (!kexec_buffer) {
>> -        pr_err("Not enough memory for the kexec measurement 
>> buffer.\n");
>> +    ret = ima_allocate_buf_at_kexec_load();
>> +    if (ret < 0)
>>           return;
>> -    }
>>         kbuf.buffer = kexec_buffer;
>>       kbuf.bufsz = kexec_buffer_size;

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Tushar Sugandhi]

On 7/11/23 14:11, Mimi Zohar wrote:
> On Tue, 2023-07-11 at 10:59 -0700, Tushar Sugandhi wrote:
>> Adding Eric to cc.
>>
>> On 7/7/23 06:00, Mimi Zohar wrote:
>>> Hi Tushar,
>>>
>>> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>>>> IMA does not provide a mechanism to allocate memory for IMA log storage
>>>> during kexec operation.
>>> The IMA measurement list is currently being carried across kexec, so
>>> obviously a buffer is being allocated for it.  IMA not allocating
>>> memory for the measurment list is not the problem statement.  Please
>>> concisely provide the problem statement, explaining why IMA needs to
>>> allocate the buffer.
>>>
>> I meant IMA does not provide separate functions to allocate buffer and
>> populate measurements.  Both operations are wrapped in an atomic
>> ima_dump_measurement_list().
> Ok.
>
>> As I mentioned in the comment in the cover letter, if there is no such
>> technical limitation to allocate the buffer and copy the measurements at
>> kexec ‘execute’ – I will make the necessary code changes and update the
>> above line in the patch description accordingly.
> The "normal" way of making this type of change would be to split the
> existing ima_dump_measurement_list() function.  Copying the measurement
> list would still be named ima_dump_measurement_list().  The other could
> be named ima_alloc_kexec_buf().  Both functions initially would be
> called.
>
Sounds good.  I will make that change.
I will define ima_alloc_kexec_buf() to allocate memory at kexec 'load'.
And update ima_dump_measurement_list() to only copy the measurements.

Both the functions will be called during kexec 'load'.

And only the updated ima_dump_measurement_list() will be called
during kexec 'execute'.

Please correct me if I misunderstood.

BTW, as discussed elsewhere I am hoping to get clarity on if we can move
everything (memory allocation and copying measurements) to kexec 'execute'.

My current understanding is segment mapping must happen at kexec 'load'.
Hopefully someone on this thread can validate if its true or not.

> Eric, besides updating the buffer at kexec execute, is there anything
> else that needs to be done (e.g. updating digests)?
>
I will also wait for his response. :)
>>>> The function should handle the scenario where
>>>> the kexec load is called multiple times.
>>> Currently the buffer is being freed with the kexec 'unload'.  With this
>>> patch IMA is allocating a buffer for the measurement list, which needs
>>> to be freed independently of the kexec 'unload'.
>> If we end up allocating the buffer at kexec ‘execute’ (which results in
>> soft boot to next Kernel) – is it technically possible that
>> kexec ‘unload’ being called after calling kexec ‘execute’?
>> If not, should I still free the buffer at kexec ‘unload’ in this
>> scenario?
> The question is how to access the buffer once kexec_add_buffer() is
> called.
>
> Mimi

Re: [PATCH 01/10] ima: implement function to allocate buffer at kexec load

[Tushar Sugandhi]

On 7/11/23 14:11, Mimi Zohar wrote:
> On Tue, 2023-07-11 at 10:59 -0700, Tushar Sugandhi wrote:
>> Adding Eric to cc.
>>
>> On 7/7/23 06:00, Mimi Zohar wrote:
>>> Hi Tushar,
>>>
>>> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
>>>> IMA does not provide a mechanism to allocate memory for IMA log storage
>>>> during kexec operation.
>>> The IMA measurement list is currently being carried across kexec, so
>>> obviously a buffer is being allocated for it.  IMA not allocating
>>> memory for the measurment list is not the problem statement.  Please
>>> concisely provide the problem statement, explaining why IMA needs to
>>> allocate the buffer.
>>>
>> I meant IMA does not provide separate functions to allocate buffer and
>> populate measurements.  Both operations are wrapped in an atomic
>> ima_dump_measurement_list().
> Ok.
>
>> As I mentioned in the comment in the cover letter, if there is no such
>> technical limitation to allocate the buffer and copy the measurements at
>> kexec ‘execute’ – I will make the necessary code changes and update the
>> above line in the patch description accordingly.
> The "normal" way of making this type of change would be to split the
> existing ima_dump_measurement_list() function.  Copying the measurement
> list would still be named ima_dump_measurement_list().  The other could
> be named ima_alloc_kexec_buf().  Both functions initially would be
> called.
>
Sounds good.  I will make that change.
I will define ima_alloc_kexec_buf() to allocate memory at kexec 'load'.
And update ima_dump_measurement_list() to only copy the measurements.

Both the functions will be called during kexec 'load'.

And only the updated ima_dump_measurement_list() will be called
during kexec 'execute'.

Please correct me if I misunderstood.

BTW, as discussed elsewhere I am hoping to get clarity on if we can move
everything (memory allocation and copying measurements) to kexec 'execute'.

My current understanding is segment mapping must happen at kexec 'load'.
Hopefully someone on this thread can validate if its true or not.

> Eric, besides updating the buffer at kexec execute, is there anything
> else that needs to be done (e.g. updating digests)?
>
I will also wait for his response. :)
>>>> The function should handle the scenario where
>>>> the kexec load is called multiple times.
>>> Currently the buffer is being freed with the kexec 'unload'.  With this
>>> patch IMA is allocating a buffer for the measurement list, which needs
>>> to be freed independently of the kexec 'unload'.
>> If we end up allocating the buffer at kexec ‘execute’ (which results in
>> soft boot to next Kernel) – is it technically possible that
>> kexec ‘unload’ being called after calling kexec ‘execute’?
>> If not, should I still free the buffer at kexec ‘unload’ in this
>> scenario?
> The question is how to access the buffer once kexec_add_buffer() is
> called.
>
> Mimi

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Tushar Sugandhi]

On 7/11/23 12:19, Stefan Berger wrote:
>
>
> On 7/11/23 14:41, Tushar Sugandhi wrote:
>> Adding Eric to cc.
>>
>> On 7/7/23 05:28, Stefan Berger wrote:
>>>
>>>
>>> On 7/3/23 17:57, Tushar Sugandhi wrote:
>>>> Currently, there's no mechanism to map and unmap segments to the 
>>>> kimage
>>>> structure.  This functionality is needed when dealing with memory 
>>>> segments
>>>> in the context of a kexec operation.
>>>>
>>>> The patch adds two new functions: kimage_map_segment() and
>>>> kimage_unmap_segment().
>>>>
>>>> Implement kimage_map_segment() which takes a kimage pointer, an 
>>>> address,
>>>> and a size.  Ensures that the entire segment is being mapped by 
>>>> comparing
>>>> the given address and size to each segment in the kimage's segment 
>>>> array.
>>>> Collect the source pages that correspond to the given address range,
>>>> allocate an array of pointers to these pages, and map them to a 
>>>> contiguous
>>>> range of virtual addresses.  If the mapping operation is 
>>>> successful, the
>>>> function returns the start of this range.  Otherwise, it frees the 
>>>> page
>>>> pointer array and returns NULL.
>>>>
>>>> Implement kimage_unmap_segment() that takes a pointer to a segment 
>>>> buffer
>>>> and unmaps it using vunmap().
>>>>
>>>> Finally, move for_each_kimage_entry() macro to kexec.h.
>>>>
>>>> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
>>>> avoid memory leaks and ensure that all mapped segments are properly
>>>> unmapped when they're no longer needed.
>>>>
>>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>
>>>> +
>>>> +    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++] = phys_to_page(src_page_addr);
>>>
>>> Since phys_to_page is not defined on many/most architectures I 
>>> change it for ppc64 and have successfully used the following:
>>>
>>> +                               src_pages[i++] = 
>>> virt_to_page(__va(src_page_addr))
>>>
>>>
>>> After several kexecs the following check still works:
>>>
>>> # evmctl ima_measurement --ignore-violations 
>>> /sys/kernel/security/ima/binary_runtime_measurements
>>> Matched per TPM bank calculated digest(s).
>>>
>>>
>>>    Stefan
>> Thank you so much Stefan for reviewing this series, and catching this
>> issue.  Are you suggesting I should use virt_to_page on all 
>> architectures
>> unconditionally, or use it only when phys_to_page is not available?
>
> I would try to used it on all architectures.
>
>    Stefan
>
Ok.  Thanks.
>>
>> ~Tushar
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 05/10] kexec: implement functions to map and unmap segment to kimage

[Tushar Sugandhi]

On 7/11/23 12:19, Stefan Berger wrote:
>
>
> On 7/11/23 14:41, Tushar Sugandhi wrote:
>> Adding Eric to cc.
>>
>> On 7/7/23 05:28, Stefan Berger wrote:
>>>
>>>
>>> On 7/3/23 17:57, Tushar Sugandhi wrote:
>>>> Currently, there's no mechanism to map and unmap segments to the 
>>>> kimage
>>>> structure.  This functionality is needed when dealing with memory 
>>>> segments
>>>> in the context of a kexec operation.
>>>>
>>>> The patch adds two new functions: kimage_map_segment() and
>>>> kimage_unmap_segment().
>>>>
>>>> Implement kimage_map_segment() which takes a kimage pointer, an 
>>>> address,
>>>> and a size.  Ensures that the entire segment is being mapped by 
>>>> comparing
>>>> the given address and size to each segment in the kimage's segment 
>>>> array.
>>>> Collect the source pages that correspond to the given address range,
>>>> allocate an array of pointers to these pages, and map them to a 
>>>> contiguous
>>>> range of virtual addresses.  If the mapping operation is 
>>>> successful, the
>>>> function returns the start of this range.  Otherwise, it frees the 
>>>> page
>>>> pointer array and returns NULL.
>>>>
>>>> Implement kimage_unmap_segment() that takes a pointer to a segment 
>>>> buffer
>>>> and unmaps it using vunmap().
>>>>
>>>> Finally, move for_each_kimage_entry() macro to kexec.h.
>>>>
>>>> Note: Use kimage_map_segment() and kimage_unmap_segment() carefully to
>>>> avoid memory leaks and ensure that all mapped segments are properly
>>>> unmapped when they're no longer needed.
>>>>
>>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>
>>>> +
>>>> +    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++] = phys_to_page(src_page_addr);
>>>
>>> Since phys_to_page is not defined on many/most architectures I 
>>> change it for ppc64 and have successfully used the following:
>>>
>>> +                               src_pages[i++] = 
>>> virt_to_page(__va(src_page_addr))
>>>
>>>
>>> After several kexecs the following check still works:
>>>
>>> # evmctl ima_measurement --ignore-violations 
>>> /sys/kernel/security/ima/binary_runtime_measurements
>>> Matched per TPM bank calculated digest(s).
>>>
>>>
>>>    Stefan
>> Thank you so much Stefan for reviewing this series, and catching this
>> issue.  Are you suggesting I should use virt_to_page on all 
>> architectures
>> unconditionally, or use it only when phys_to_page is not available?
>
> I would try to used it on all architectures.
>
>    Stefan
>
Ok.  Thanks.
>>
>> ~Tushar
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

On 7/11/23 10:51, Tushar Sugandhi wrote:
> Thanks for reviewing this series Mimi. Appreciate it.
> 
> Adding Eric to cc.
> 
> On 7/7/23 08:55, Mimi Zohar wrote:
>> On Mon, 2023-07-03 at 14:56 -0700, Tushar Sugandhi wrote:
>>> 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'.
>>>
>>> Some devices can be configured to call kexec 'load' first, and followed
>>> by kexec 'execute' after some time. (as opposed to calling 'load' and
>>> 'execute' in one single kexec command).  In such scenario, if new IMA
>>> measurements are added between kexec 'load' and kexec 'execute', the
>>> TPM PCRs are extended with the IMA events between 'load' and 'execute';
>>> but those IMA events are not carried over to the new kernel after kexec
>>> soft reboot.  This results in mismatch between TPM PCR quotes and the
>>> actual IMA measurements list after the device boots into the new kexec
>>> image.  This mismatch results in the remote attestation failing for that
>>> device.
>>>
>>> This patch series proposes a solution to solve this problem by 
>>> allocating
>>> the necessary buffer at kexec 'load' time, and populating the buffer
>>> with the IMA measurements at kexec 'execute' time.
>> Thanks, Tushar.   Depending on the IMA policy, the above problem
>> statement is correct, but not all policies are affected by it.  It's
>> also unclear why so much needs to change.  Instead of copying the
>> measurement list at kexec 'load',  using the existing method and simply
>> copying them at kexec 'exec' would suffice.
> My understanding is the buffer must be allocated at kexec ‘load’ time.
> The segment size cannot change between kexec ‘load’ and kexec ‘execute’.
> Not sure if this is a technical limitation of IMA, or KEXEC.
> 
> Could you/someone from kexec side let me know?
> 
> If my current understanding is not correct, then I agree, simply copying
> the measurements at kexec ‘execute’ using ima_dump_measurement_list should
> suffice.
> 
>>
>> Also as mentioned in comment on 3/10, the ordering of this patch set is
>> not bisect safe.  If the same method of copying the measurement list
>> was used, changing from copying at  kexec 'load'  to kexec 'exec' could
>> be done in the same patch.
>>
>> Mimi
> Ok. Based on my above response, if there is no such technical limitation
> to allocate and copy at kexec ‘execute’ – I will simply move the call
> to ima_dump_measurement_list from kexec 'load' to 'exec'.
> And that can be done inthe same patch as you mentioned.
> 
> ~Tushar
Hello all,
For the last few days, I did several experiments to allocate the memory
during kexec 'execute' rather than at the existing kexec 'load'.

My experiments either resulted in Kernel panics, or didn't copy the IMA
log entries added between 'load' and 'execute'.

As per my understanding, and also documented in several comments in
kexec code (e.g. in kexec_file.c [1]), adding buffer after allocating
control pages is not allowed. All segments need to be placed first
before any control pages are allocated.

And control page allocation happens at kexec 'load' [3][4][5].

There are several checks that ensures new buffer segments are not
allocated after control pages are allocated at kexec 'load'.
e.g. kexec_file.c[1][3], kexec_core.c[2].

If we want to allocate buffer at kexec 'execute' to address this issue
of "missing entries in IMA log during kexec load-execute window", we
will have to bypass those checks. That would be a very significant
change in kexec logic. And I do not believe its the right approach.

I believe we have to stick with the current approach of allocating
memory segments at kexec 'load', and copy whatever we can, during
kexe 'execute'.

Thanks,
Tushar

[1] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_file.c#L652
int kexec_add_buffer(struct kexec_buf *kbuf)
{
...

  /*
  * Make sure we are not trying to add buffer after allocating
  * control pages. All segments need to be placed first before
  * any control pages are allocated. As control page allocation
  * logic goes through list of segments to make sure there are
  * no destination overlaps.
  */
  if (!list_empty(&kbuf->image->control_pages)) {
      WARN_ON(1);
      return -EINVAL;
  }
...
}

[2] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_core.c#L352
static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
                                          unsigned int order)
{
      /* Control pages are special, they are the intermediaries
       * that are needed while we copy the rest of the pages
       * to their final resting place.  As such they must
       * not conflict with either the destination addresses
       * or memory the kernel is already using.
       *
       * The only case where we really need more than one of
       * these are for architectures where we cannot disable
       * the MMU and must instead generate an identity mapped
       * page table for all of the memory.
       *
       * At worst this runs in O(N) of the image size.
       */
...
}

[3] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_file.c#L299
static int
kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
                   int initrd_fd, const char __user *cmdline_ptr,
                   unsigned long cmdline_len, unsigned long flags)
{
      image->control_code_page = kimage_alloc_control_pages(image,
                                 get_order(KEXEC_CONTROL_PAGE_SIZE));
      if (!image->control_code_page) {
            pr_err("Could not allocate control_code_buffer\n");
            goto out_free_post_load_bufs;
      }
}

[4] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_file.c#L366
SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
            unsigned long, cmdline_len, const char __user *, cmdline_ptr,
            unsigned long, flags)
{
...
ret = kimage_file_alloc_init(&image, kernel_fd, initrd_fd, cmdline_ptr,
...                          cmdline_len, flags);
}
                        
[5] https://elixir.bootlin.com/linux/latest/source/kernel/kexec.c#L63
static int kimage_alloc_init(struct kimage **rimage, unsigned long entry,
                       unsigned long nr_segments,
                       struct kexec_segment *segments,
                       unsigned long flags)
{
     ...
     image->control_code_page = kimage_alloc_control_pages(image,
                           get_order(KEXEC_CONTROL_PAGE_SIZE));
     if (!image->control_code_page) {
       pr_err("Could not allocate control_code_buffer\n");
      goto out_free_image;
     }
     ...
}

>>
>>> The solution includes:
>>>   - addition of new functionality to allocate a buffer to hold IMA
>>>     measurements at kexec 'load',
>>>
>>>   - ima functionality to suspend and resume measurements as needed 
>>> during
>>>     buffer copy at kexec 'execute',
>>>
>>>   - ima functionality for mapping the measurement list 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',
>>>
>>>   - and removal of deprecated functions.
>>>
>>> The modifications 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.
>>>
>>> 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/
>>>
>>> Tushar Sugandhi (10):
>>>    ima: implement function to allocate buffer at kexec load
>>>    ima: implement function to populate buffer at kexec execute
>>>    ima: allocate buffer at kexec load to hold ima measurements
>>>    ima: implement functions to suspend and resume measurements
>>>    kexec: implement functions to map and unmap segment to kimage
>>>    ima: update buffer at kexec execute with ima measurements
>>>    ima: remove function ima_dump_measurement_list
>>>    ima: implement and register a reboot notifier function to update 
>>> kexec
>>>      buffer
>>>    ima: suspend measurements while the kexec buffer is being copied
>>>    kexec: update kexec_file_load syscall to call ima_kexec_post_load
>>>
>>>   include/linux/ima.h                |   3 +
>>>   include/linux/kexec.h              |  13 ++
>>>   kernel/kexec_core.c                |  72 +++++++++-
>>>   kernel/kexec_file.c                |   7 +
>>>   kernel/kexec_internal.h            |   1 +
>>>   security/integrity/ima/ima.h       |   4 +
>>>   security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>>>   security/integrity/ima/ima_queue.c |  32 +++++
>>>   8 files changed, 295 insertions(+), 48 deletions(-)
>>>

Re: [PATCH 00/10] ima: measure events between kexec load and execute

[Tushar Sugandhi]

On 7/11/23 10:51, Tushar Sugandhi wrote:
> Thanks for reviewing this series Mimi. Appreciate it.
> 
> Adding Eric to cc.
> 
> On 7/7/23 08:55, Mimi Zohar wrote:
>> On Mon, 2023-07-03 at 14:56 -0700, Tushar Sugandhi wrote:
>>> 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'.
>>>
>>> Some devices can be configured to call kexec 'load' first, and followed
>>> by kexec 'execute' after some time. (as opposed to calling 'load' and
>>> 'execute' in one single kexec command).  In such scenario, if new IMA
>>> measurements are added between kexec 'load' and kexec 'execute', the
>>> TPM PCRs are extended with the IMA events between 'load' and 'execute';
>>> but those IMA events are not carried over to the new kernel after kexec
>>> soft reboot.  This results in mismatch between TPM PCR quotes and the
>>> actual IMA measurements list after the device boots into the new kexec
>>> image.  This mismatch results in the remote attestation failing for that
>>> device.
>>>
>>> This patch series proposes a solution to solve this problem by 
>>> allocating
>>> the necessary buffer at kexec 'load' time, and populating the buffer
>>> with the IMA measurements at kexec 'execute' time.
>> Thanks, Tushar.   Depending on the IMA policy, the above problem
>> statement is correct, but not all policies are affected by it.  It's
>> also unclear why so much needs to change.  Instead of copying the
>> measurement list at kexec 'load',  using the existing method and simply
>> copying them at kexec 'exec' would suffice.
> My understanding is the buffer must be allocated at kexec ‘load’ time.
> The segment size cannot change between kexec ‘load’ and kexec ‘execute’.
> Not sure if this is a technical limitation of IMA, or KEXEC.
> 
> Could you/someone from kexec side let me know?
> 
> If my current understanding is not correct, then I agree, simply copying
> the measurements at kexec ‘execute’ using ima_dump_measurement_list should
> suffice.
> 
>>
>> Also as mentioned in comment on 3/10, the ordering of this patch set is
>> not bisect safe.  If the same method of copying the measurement list
>> was used, changing from copying at  kexec 'load'  to kexec 'exec' could
>> be done in the same patch.
>>
>> Mimi
> Ok. Based on my above response, if there is no such technical limitation
> to allocate and copy at kexec ‘execute’ – I will simply move the call
> to ima_dump_measurement_list from kexec 'load' to 'exec'.
> And that can be done inthe same patch as you mentioned.
> 
> ~Tushar
Hello all,
For the last few days, I did several experiments to allocate the memory
during kexec 'execute' rather than at the existing kexec 'load'.

My experiments either resulted in Kernel panics, or didn't copy the IMA
log entries added between 'load' and 'execute'.

As per my understanding, and also documented in several comments in
kexec code (e.g. in kexec_file.c [1]), adding buffer after allocating
control pages is not allowed. All segments need to be placed first
before any control pages are allocated.

And control page allocation happens at kexec 'load' [3][4][5].

There are several checks that ensures new buffer segments are not
allocated after control pages are allocated at kexec 'load'.
e.g. kexec_file.c[1][3], kexec_core.c[2].

If we want to allocate buffer at kexec 'execute' to address this issue
of "missing entries in IMA log during kexec load-execute window", we
will have to bypass those checks. That would be a very significant
change in kexec logic. And I do not believe its the right approach.

I believe we have to stick with the current approach of allocating
memory segments at kexec 'load', and copy whatever we can, during
kexe 'execute'.

Thanks,
Tushar

[1] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_file.c#L652
int kexec_add_buffer(struct kexec_buf *kbuf)
{
...

  /*
  * Make sure we are not trying to add buffer after allocating
  * control pages. All segments need to be placed first before
  * any control pages are allocated. As control page allocation
  * logic goes through list of segments to make sure there are
  * no destination overlaps.
  */
  if (!list_empty(&kbuf->image->control_pages)) {
      WARN_ON(1);
      return -EINVAL;
  }
...
}

[2] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_core.c#L352
static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
                                          unsigned int order)
{
      /* Control pages are special, they are the intermediaries
       * that are needed while we copy the rest of the pages
       * to their final resting place.  As such they must
       * not conflict with either the destination addresses
       * or memory the kernel is already using.
       *
       * The only case where we really need more than one of
       * these are for architectures where we cannot disable
       * the MMU and must instead generate an identity mapped
       * page table for all of the memory.
       *
       * At worst this runs in O(N) of the image size.
       */
...
}

[3] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_file.c#L299
static int
kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
                   int initrd_fd, const char __user *cmdline_ptr,
                   unsigned long cmdline_len, unsigned long flags)
{
      image->control_code_page = kimage_alloc_control_pages(image,
                                 get_order(KEXEC_CONTROL_PAGE_SIZE));
      if (!image->control_code_page) {
            pr_err("Could not allocate control_code_buffer\n");
            goto out_free_post_load_bufs;
      }
}

[4] https://elixir.bootlin.com/linux/latest/source/kernel/kexec_file.c#L366
SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
            unsigned long, cmdline_len, const char __user *, cmdline_ptr,
            unsigned long, flags)
{
...
ret = kimage_file_alloc_init(&image, kernel_fd, initrd_fd, cmdline_ptr,
...                          cmdline_len, flags);
}
                        
[5] https://elixir.bootlin.com/linux/latest/source/kernel/kexec.c#L63
static int kimage_alloc_init(struct kimage **rimage, unsigned long entry,
                       unsigned long nr_segments,
                       struct kexec_segment *segments,
                       unsigned long flags)
{
     ...
     image->control_code_page = kimage_alloc_control_pages(image,
                           get_order(KEXEC_CONTROL_PAGE_SIZE));
     if (!image->control_code_page) {
       pr_err("Could not allocate control_code_buffer\n");
      goto out_free_image;
     }
     ...
}

>>
>>> The solution includes:
>>>   - addition of new functionality to allocate a buffer to hold IMA
>>>     measurements at kexec 'load',
>>>
>>>   - ima functionality to suspend and resume measurements as needed 
>>> during
>>>     buffer copy at kexec 'execute',
>>>
>>>   - ima functionality for mapping the measurement list 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',
>>>
>>>   - and removal of deprecated functions.
>>>
>>> The modifications 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.
>>>
>>> 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/
>>>
>>> Tushar Sugandhi (10):
>>>    ima: implement function to allocate buffer at kexec load
>>>    ima: implement function to populate buffer at kexec execute
>>>    ima: allocate buffer at kexec load to hold ima measurements
>>>    ima: implement functions to suspend and resume measurements
>>>    kexec: implement functions to map and unmap segment to kimage
>>>    ima: update buffer at kexec execute with ima measurements
>>>    ima: remove function ima_dump_measurement_list
>>>    ima: implement and register a reboot notifier function to update 
>>> kexec
>>>      buffer
>>>    ima: suspend measurements while the kexec buffer is being copied
>>>    kexec: update kexec_file_load syscall to call ima_kexec_post_load
>>>
>>>   include/linux/ima.h                |   3 +
>>>   include/linux/kexec.h              |  13 ++
>>>   kernel/kexec_core.c                |  72 +++++++++-
>>>   kernel/kexec_file.c                |   7 +
>>>   kernel/kexec_internal.h            |   1 +
>>>   security/integrity/ima/ima.h       |   4 +
>>>   security/integrity/ima/ima_kexec.c | 211 +++++++++++++++++++++++------
>>>   security/integrity/ima/ima_queue.c |  32 +++++
>>>   8 files changed, 295 insertions(+), 48 deletions(-)
>>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec