Re: [PATCH v2 2/7] ima: move ima_dump_measurement_list call from kexec load to execute

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/12/23 17:28, Stefan Berger wrote:

On 10/5/23 14:25, Tushar Sugandhi wrote:
In the current IMA implementation, ima_dump_measurement_list() is called
during the kexec 'load' operation.  This can result in loss of IMA
measurements taken between the 'load' and 'execute' phases when the
system goes through Kexec soft reboot to a new Kernel.  The call to the
function ima_dump_measurement_list() needs to be moved out of the
function ima_add_kexec_buffer() and needs to be called during the kexec
'execute' operation.

Implement a function ima_update_kexec_buffer() that is called during
kexec 'execute', allowing IMA to update the measurement list with the
events between kexec 'load' and 'execute'.  Move the
ima_dump_measurement_list() call from ima_add_kexec_buffer() to
ima_update_kexec_buffer().  Make ima_kexec_buffer and kexec_segment_size
variables global, so that they can be accessed during both kexec 'load'
and 'execute'.  Add functions ima_measurements_suspend() and
ima_measurements_resume() to set and reset the 'suspend_ima_measurements'
variable respectively, to suspend/resume IMA measurements.  Use
the existing 'ima_extend_list_mutex' to ensure that the operations are
thread-safe.  These function calls will help maintaining the integrity
of the IMA log while it is being copied to the new Kernel's buffer.
Add a reboot notifier_block 'update_buffer_nb' to ensure
the function ima_update_kexec_buffer() gets called during kexec
soft-reboot.

Signed-off-by: Tushar Sugandhi<tusharsu@xxxxxxxxxxxxxxxxxxx>
---
  security/integrity/ima/ima.h       |  2 ++
  security/integrity/ima/ima_kexec.c | 58 +++++++++++++++++++++++++-----
  security/integrity/ima/ima_queue.c | 18 ++++++++++
  3 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c29db699c996..49a6047dd8eb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -161,6 +161,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_kexec.c b/security/integrity/ima/ima_kexec.c
index 307e07991865..2c11bbe6efef 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -17,6 +17,8 @@
  #ifdef CONFIG_IMA_KEXEC
  struct seq_file ima_kexec_file;
  struct ima_kexec_hdr ima_khdr;
+static void *ima_kexec_buffer;
+static size_t kexec_segment_size;
  void ima_clear_kexec_file(void)
  {
@@ -142,7 +144,6 @@ void ima_add_kexec_buffer(struct kimage *image)
      /* use more understandable variable names than defined in kbuf */
      void *kexec_buffer = NULL;
      size_t kexec_buffer_size;
-    size_t kexec_segment_size;
      int ret;
      /*
@@ -167,14 +168,6 @@ void ima_add_kexec_buffer(struct kimage *image)
          return;
      }
-    ret = ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
-                    kexec_segment_size);
-    if (ret < 0) {
-        pr_err("%s: Failed to dump IMA measurements. Error:%d.\n",
-               __func__, ret);
-        return;
-    }
-
      kbuf.buffer = kexec_buffer;
      kbuf.bufsz = kexec_buffer_size;
      kbuf.memsz = kexec_segment_size;
@@ -192,6 +185,53 @@ 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 *buf = NULL;
+    size_t buf_size;
+    bool resume = false;
+    int ret;
+
+    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();
+
+    buf_size = ima_get_binary_runtime_size();
There doesn't seem to be a need to call this function and pass in the binary runtime size into the dump function. You should be able to remove it.
Oh. This was an oversight.
This line is removed in patch 7/7.  It should have been removed here
itself.
Thanks for catching it. Will fix.
+    ret = ima_dump_measurement_list(&buf_size, &buf,
+                    kexec_segment_size);
+
+    if (!buf || ret < 0) {
I don't think this function can (or should) ever return ret >= 0 with buf == NULL.
I will simplify this check. Thanks.

~Tushar
+        pr_err("%s: Dump measurements failed. Error:%d\n",
+               __func__, ret);
+        resume = true;
+        goto out;
+    }
+    memcpy(ima_kexec_buffer, buf, buf_size);
+out:
+    ima_kexec_buffer = NULL;
+
+    if (resume)
+        ima_measurements_resume();
+
+    return NOTIFY_OK;
+}
+struct notifier_block update_buffer_nb = {
+    .notifier_call = ima_update_kexec_buffer,
+};
+
  #endif /* IMA_KEXEC */
  /*
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 532da87ce519..9e7d1196006e 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)
@@ -147,6 +152,19 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)           pr_err("Error Communicating to TPM chip, result: %d\n", result);
      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

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




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux