On 7/6/22 10:00, Jonathan McDowell wrote:
On Tue, Jul 05, 2022 at 06:46:54PM -0400, Mimi Zohar wrote:
[Cc'ing Borislav Petkov <b...@suse.de>, Jonathan McDowell <nood...@fb.com
]

Hi Stefan,

On Thu, 2022-06-30 at 22:26 -0400, Stefan Berger wrote:
Refactor IMA buffer related functions to make them reusable for carrying
TPM logs across kexec.

Signed-off-by: Stefan Berger <stef...@linux.ibm.com>
Cc: Rob Herring <robh...@kernel.org>
Cc: Frank Rowand <frowand.l...@gmail.com>
Cc: Mimi Zohar <zo...@linux.ibm.com>

Refactoring the ima_get_kexec_buffer sounds good, but there's a merge
conflict with Jonathan McDowell's commit "b69a2afd5afc x86/kexec: Carry
forward IMA measurement log on kexec".
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/of/kexec.c

None of this looks difficult to re-do on top of my changes that are in
-next; the only thing to watch out for is a couple of functions have
moved into the __init section but that looks appropriate for your TPM
log carry-over too.

Yes, I am rebasing my series now and will post v5 of this series with your patch prepended as well.

   Stefan


---
v4:
  - Move debug output into setup_buffer()
---
  drivers/of/kexec.c | 131 ++++++++++++++++++++++++++-------------------
  1 file changed, 76 insertions(+), 55 deletions(-)

diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index c4f9b6655a2e..0710703acfb0 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -115,48 +115,59 @@ static int do_get_kexec_buffer(const void *prop, int len, 
unsigned long *addr,
        return 0;
  }
-/**
- * ima_get_kexec_buffer - get IMA buffer from the previous kernel
- * @addr:      On successful return, set to point to the buffer contents.
- * @size:      On successful return, set to the buffer size.
- *
- * Return: 0 on success, negative errno on error.
- */
-int ima_get_kexec_buffer(void **addr, size_t *size)
+static int get_kexec_buffer(const char *name, unsigned long *addr, size_t 
*size)
  {
        int ret, len;
-       unsigned long tmp_addr;
        unsigned long start_pfn, end_pfn;
-       size_t tmp_size;
        const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
-               return -ENOTSUPP;
-
-       prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
+       prop = of_get_property(of_chosen, name, &len);
        if (!prop)
                return -ENOENT;
- ret = do_get_kexec_buffer(prop, len, &tmp_addr, &tmp_size);
+       ret = do_get_kexec_buffer(prop, len, addr, size);
        if (ret)
                return ret;
- /* Do some sanity on the returned size for the ima-kexec buffer */
-       if (!tmp_size)
+       /* Do some sanity on the returned size for the kexec buffer */
+       if (!*size)
                return -ENOENT;
/*
         * Calculate the PFNs for the buffer and ensure
         * they are with in addressable memory.
         */
-       start_pfn = PHYS_PFN(tmp_addr);
-       end_pfn = PHYS_PFN(tmp_addr + tmp_size - 1);
+       start_pfn = PHYS_PFN(*addr);
+       end_pfn = PHYS_PFN(*addr + *size - 1);
        if (!page_is_ram(start_pfn) || !page_is_ram(end_pfn)) {
-               pr_warn("IMA buffer at 0x%lx, size = 0x%zx beyond memory\n",
-                       tmp_addr, tmp_size);
+               pr_warn("%s buffer at 0x%lx, size = 0x%zx beyond memory\n",
+                       name, *addr, *size);
                return -EINVAL;
        }
+ return 0;
+}
+
+/**
+ * ima_get_kexec_buffer - get IMA buffer from the previous kernel
+ * @addr:      On successful return, set to point to the buffer contents.
+ * @size:      On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int ima_get_kexec_buffer(void **addr, size_t *size)
+{
+       int ret;
+       unsigned long tmp_addr;
+       size_t tmp_size;
+
+       if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
+               return -ENOTSUPP;
+
+       ret = get_kexec_buffer("linux,ima-kexec-buffer", &tmp_addr, &tmp_size);
+       if (ret)
+               return ret;
+
        *addr = __va(tmp_addr);
        *size = tmp_size;
@@ -191,72 +202,82 @@ int ima_free_kexec_buffer(void)
        return memblock_phys_free(addr, size);
  }
-/**
- * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
- *
- * @fdt: Flattened Device Tree to update
- * @chosen_node: Offset to the chosen node in the device tree
- *
- * The IMA measurement buffer is of no use to a subsequent kernel, so we always
- * remove it from the device tree.
- */
-static void remove_ima_buffer(void *fdt, int chosen_node)
+static int remove_buffer(void *fdt, int chosen_node, const char *name)
  {
        int ret, len;
        unsigned long addr;
        size_t size;
        const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
-               return;
-
-       prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
+       prop = fdt_getprop(fdt, chosen_node, name, &len);
        if (!prop)
-               return;
+               return -ENOENT;
ret = do_get_kexec_buffer(prop, len, &addr, &size);
-       fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
+       fdt_delprop(fdt, chosen_node, name);
        if (ret)
-               return;
+               return ret;
ret = fdt_find_and_del_mem_rsv(fdt, addr, size);
        if (!ret)
-               pr_debug("Removed old IMA buffer reservation.\n");
+               pr_debug("Remove old %s buffer reserveration", name);
+       return ret;
  }
-#ifdef CONFIG_IMA_KEXEC
  /**
- * setup_ima_buffer - add IMA buffer information to the fdt
- * @image:             kexec image being loaded.
- * @fdt:               Flattened device tree for the next kernel.
- * @chosen_node:       Offset to the chosen node.
+ * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
   *
- * Return: 0 on success, or negative errno on error.
+ * @fdt: Flattened Device Tree to update
+ * @chosen_node: Offset to the chosen node in the device tree
+ *
+ * The IMA measurement buffer is of no use to a subsequent kernel, so we always
+ * remove it from the device tree.
   */
-static int setup_ima_buffer(const struct kimage *image, void *fdt,
-                           int chosen_node)
+static void remove_ima_buffer(void *fdt, int chosen_node)
+{
+       if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
+               return;
+
+       remove_buffer(fdt, chosen_node, "linux,ima-kexec-buffer");
+}
+
+#ifdef CONFIG_IMA_KEXEC
+static int setup_buffer(void *fdt, int chosen_node, const char *name,
+                       phys_addr_t addr, size_t size)
  {
        int ret;
- if (!image->ima_buffer_size)
+       if (!size)
                return 0;
ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
-                                      "linux,ima-kexec-buffer",
-                                      image->ima_buffer_addr,
-                                      image->ima_buffer_size);
+                                      name, addr, size);
        if (ret < 0)
                return -EINVAL;
- ret = fdt_add_mem_rsv(fdt, image->ima_buffer_addr,
-                             image->ima_buffer_size);
+       ret = fdt_add_mem_rsv(fdt, addr, size);
        if (ret)
                return -EINVAL;
- pr_debug("IMA buffer at 0x%pa, size = 0x%zx\n",
-                &image->ima_buffer_addr, image->ima_buffer_size);
+       pr_debug("%s at 0x%pa, size = 0x%zx\n", name, &addr, size);
return 0;
+
+}
+
+/**
+ * setup_ima_buffer - add IMA buffer information to the fdt
+ * @image:             kexec image being loaded.
+ * @fdt:               Flattened device tree for the next kernel.
+ * @chosen_node:       Offset to the chosen node.
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+static int setup_ima_buffer(const struct kimage *image, void *fdt,
+                           int chosen_node)
+{
+       return setup_buffer(fdt, chosen_node, "linux,ima-kexec-buffer",
+                           image->ima_buffer_addr, image->ima_buffer_size);
  }
  #else /* CONFIG_IMA_KEXEC */
  static inline int setup_ima_buffer(const struct kimage *image, void *fdt,

Reply via email to