The of-tree subsystem does not currently preserve the IBM vTPM 1.2 and
vTPM 2.0 measurement logs across a kexec on PowerVM and PowerKVM. This
series fixes this for the kexec_file_load() syscall using the flattened
device tree (fdt) to carry the TPM measurement log's buffer across kexec.
Stefan
v9:
- 2/4: Rebased on 6.3-rc7: call tpm_read_log_memory_region if -ENODEV returned
v8:
- Added Jarkko's, Coiby's, and Rob's tags
- Rebase on v6.0-rc3 that absorbed 2 already upstreamed patches
v7:
- Added Nageswara's Tested-by tags
- Added back original comment to inline function and removed Jarkko's R-b tag
v6:
- Add __init to get_kexec_buffer as suggested by Jonathan
- Fixed issue detected by kernel test robot
v5:
- Rebased on 1 more patch that would otherwise create merge conflicts
v4:
- Rebased on 2 patches that would otherwise create merge conflicts;
posting these patches in this series with several tags removed so
krobot can test the series already
- Changes to individual patches documented in patch descripitons
v3:
- Moved TPM Open Firmware related function to drivers/char/tpm/eventlog/tpm_of.c
v2:
- rearranged patches
- fixed compilation issues for x86
Palmer Dabbelt (1):
drivers: of: kexec ima: Support 32-bit platforms
Stefan Berger (3):
tpm: of: Make of-tree specific function commonly available
of: kexec: Refactor IMA buffer related functions to make them reusable
tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
drivers/char/tpm/eventlog/of.c | 30 +--
drivers/of/kexec.c | 336 ++++++++++++++++++++++++++++-----
include/linux/kexec.h | 6 +
include/linux/of.h | 6 +
include/linux/tpm.h | 36 ++++
kernel/kexec_file.c | 6 +
6 files changed, 344 insertions(+), 76 deletions(-)
base-commit: 6a8f57ae2eb07ab39a6f0ccad60c760743051026
--
2.38.1
The memory area of the TPM measurement log is currently not properly
duplicated for carrying it across kexec when an Open Firmware
Devicetree is used. Therefore, the contents of the log get corrupted.
Fix this for the kexec_file_load() syscall by allocating a buffer and
copying the contents of the existing log into it. The new buffer is
preserved across the kexec and a pointer to it is available when the new
kernel is started. To achieve this, store the allocated buffer's address
in the flattened device tree (fdt) under the name linux,tpm-kexec-buffer
and search for this entry early in the kernel startup before the TPM
subsystem starts up. Adjust the pointer in the of-tree stored under
linux,sml-base to point to this buffer holding the preserved log. The TPM
driver can then read the base address from this entry when making the log
available. Invalidate the log by removing 'linux,sml-base' from the
devicetree if anything goes wrong with updating the buffer.
Use subsys_initcall() to call the function to restore the buffer even if
the TPM subsystem or driver are not used. This allows the buffer to be
carried across the next kexec without involvement of the TPM subsystem
and ensures a valid buffer pointed to by the of-tree.
Use the subsys_initcall(), rather than an ealier initcall, since
page_is_ram() in get_kexec_buffer() only starts working at this stage.
Signed-off-by: Stefan Berger <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Cc: Eric Biederman <[email protected]>
Tested-by: Nageswara R Sastry <[email protected]>
Tested-by: Coiby Xu <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v6:
- Define prototype for tpm_add_kexec_buffer under same config options
as drivers/of/kexec.c is compiled, provide inline function otherwise.
(kernel test robot)
v4:
- Added #include <linux/vmalloc.h> due to parisc
- Use phys_addr_t for physical address rather than void *
- Remove linux,sml-base if the buffer cannot be updated after a kexec
- Added __init to functions where possible
---
drivers/of/kexec.c | 216 +++++++++++++++++++++++++++++++++++++++++-
include/linux/kexec.h | 6 ++
include/linux/of.h | 6 ++
kernel/kexec_file.c | 6 ++
4 files changed, 232 insertions(+), 2 deletions(-)
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index fa8c0c75adf9..9831d25dd83e 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -19,6 +19,8 @@
#include <linux/random.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/tpm.h>
+#include <linux/vmalloc.h>
#define RNG_SEED_SIZE 128
@@ -116,7 +118,6 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
-#ifdef CONFIG_HAVE_IMA_KEXEC
static int __init get_kexec_buffer(const char *name, unsigned long *addr,
size_t *size)
{
@@ -151,6 +152,7 @@ static int __init get_kexec_buffer(const char *name, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -239,7 +241,6 @@ static void remove_ima_buffer(void *fdt, int chosen_node)
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)
{
@@ -263,6 +264,7 @@ static int setup_buffer(void *fdt, int chosen_node, const char *name,
}
+#ifdef CONFIG_IMA_KEXEC
/**
* setup_ima_buffer - add IMA buffer information to the fdt
* @image: kexec image being loaded.
@@ -285,6 +287,213 @@ static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
}
#endif /* CONFIG_IMA_KEXEC */
+/**
+ * tpm_get_kexec_buffer - get TPM log buffer from the previous kernel
+ * @phyaddr: On successful return, set to physical address of buffer
+ * @size: On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static int __init tpm_get_kexec_buffer(phys_addr_t *phyaddr, size_t *size)
+{
+ unsigned long tmp_addr;
+ size_t tmp_size;
+ int ret;
+
+ ret = get_kexec_buffer("linux,tpm-kexec-buffer", &tmp_addr, &tmp_size);
+ if (ret)
+ return ret;
+
+ *phyaddr = (phys_addr_t)tmp_addr;
+ *size = tmp_size;
+
+ return 0;
+}
+
+/**
+ * tpm_of_remove_kexec_buffer - remove the linux,tpm-kexec-buffer node
+ */
+static int __init tpm_of_remove_kexec_buffer(void)
+{
+ struct property *prop;
+
+ prop = of_find_property(of_chosen, "linux,tpm-kexec-buffer", NULL);
+ if (!prop)
+ return -ENOENT;
+
+ return of_remove_property(of_chosen, prop);
+}
+
+/**
+ * remove_tpm_buffer - remove the TPM log buffer property and reservation from @fdt
+ *
+ * @fdt: Flattened Device Tree to update
+ * @chosen_node: Offset to the chosen node in the device tree
+ *
+ * The TPM log measurement buffer is of no use to a subsequent kernel, so we always
+ * remove it from the device tree.
+ */
+static void remove_tpm_buffer(void *fdt, int chosen_node)
+{
+ if (!IS_ENABLED(CONFIG_PPC64))
+ return;
+
+ remove_buffer(fdt, chosen_node, "linux,tpm-kexec-buffer");
+}
+
+/**
+ * setup_tpm_buffer - add TPM measurement log 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_tpm_buffer(const struct kimage *image, void *fdt,
+ int chosen_node)
+{
+ if (!IS_ENABLED(CONFIG_PPC64))
+ return 0;
+
+ return setup_buffer(fdt, chosen_node, "linux,tpm-kexec-buffer",
+ image->tpm_buffer_addr, image->tpm_buffer_size);
+}
+
+void tpm_add_kexec_buffer(struct kimage *image)
+{
+ struct kexec_buf kbuf = { .image = image, .buf_align = 1,
+ .buf_min = 0, .buf_max = ULONG_MAX,
+ .top_down = true };
+ struct device_node *np;
+ void *buffer;
+ u32 size;
+ u64 base;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_PPC64))
+ return;
+
+ np = of_find_node_by_name(NULL, "vtpm");
+ if (!np)
+ return;
+
+ if (of_tpm_get_sml_parameters(np, &base, &size) < 0)
+ return;
+
+ buffer = vmalloc(size);
+ if (!buffer)
+ return;
+ memcpy(buffer, __va(base), size);
+
+ kbuf.buffer = buffer;
+ kbuf.bufsz = size;
+ kbuf.memsz = size;
+ ret = kexec_add_buffer(&kbuf);
+ if (ret) {
+ pr_err("Error passing over kexec TPM measurement log buffer: %d\n",
+ ret);
+ return;
+ }
+
+ image->tpm_buffer = buffer;
+ image->tpm_buffer_addr = kbuf.mem;
+ image->tpm_buffer_size = size;
+}
+
+/**
+ * tpm_post_kexec - Make stored TPM log buffer available in of-tree
+ */
+static int __init tpm_post_kexec(void)
+{
+ struct property *newprop, *p;
+ struct device_node *np;
+ phys_addr_t phyaddr;
+ u32 oflogsize;
+ size_t size;
+ u64 unused;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_PPC64))
+ return 0;
+
+ np = of_find_node_by_name(NULL, "vtpm");
+ if (!np)
+ return 0;
+
+ if (!of_get_property(of_chosen, "linux,tpm-kexec-buffer", NULL)) {
+ /*
+ * linux,tpm-kexec-buffer may be missing on initial boot
+ * or if previous kernel didn't pass a buffer.
+ */
+ if (of_get_property(of_chosen, "linux,booted-from-kexec", NULL)) {
+ /* no buffer but kexec'd: remove 'linux,sml-base' */
+ ret = -EINVAL;
+ goto err_remove_sml_base;
+ }
+ return 0;
+ }
+
+ /*
+ * If any one of the following steps fails we remove linux,sml-base
+ * to invalidate the TPM log.
+ */
+ ret = tpm_get_kexec_buffer(&phyaddr, &size);
+ if (ret)
+ goto err_remove_kexec_buffer;
+
+ /* logsize must not have changed */
+ ret = of_tpm_get_sml_parameters(np, &unused, &oflogsize);
+ if (ret < 0)
+ goto err_free_memblock;
+ ret = -EINVAL;
+ if (oflogsize != size)
+ goto err_free_memblock;
+
+ /* replace linux,sml-base with new physical address of buffer */
+ ret = -ENOMEM;
+ newprop = kzalloc(sizeof(*newprop), GFP_KERNEL);
+ if (!newprop)
+ goto err_free_memblock;
+
+ newprop->name = kstrdup("linux,sml-base", GFP_KERNEL);
+ newprop->length = sizeof(phyaddr);
+ newprop->value = kmalloc(sizeof(phyaddr), GFP_KERNEL);
+ if (!newprop->name || !newprop->value)
+ goto err_free_newprop_struct;
+
+ if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
+ of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
+ ret = -ENODEV;
+ goto err_free_newprop_struct;
+ } else {
+ *(phys_addr_t *)newprop->value = phyaddr;
+ }
+
+ ret = of_update_property(np, newprop);
+ if (ret) {
+ pr_err("Could not update linux,sml-base with new address");
+ goto err_free_newprop_struct;
+ }
+
+ return 0;
+
+err_free_newprop_struct:
+ kfree(newprop->value);
+ kfree(newprop->name);
+ kfree(newprop);
+err_free_memblock:
+ memblock_phys_free((phys_addr_t)phyaddr, size);
+err_remove_kexec_buffer:
+ tpm_of_remove_kexec_buffer();
+err_remove_sml_base:
+ p = of_find_property(np, "linux,sml-base", NULL);
+ if (p)
+ of_remove_property(np, p);
+
+ return ret;
+}
+subsys_initcall(tpm_post_kexec);
+
/*
* of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
*
@@ -483,6 +692,9 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
remove_ima_buffer(fdt, chosen_node);
ret = setup_ima_buffer(image, fdt, fdt_path_offset(fdt, "/chosen"));
+ remove_tpm_buffer(fdt, chosen_node);
+ ret = setup_tpm_buffer(image, fdt, fdt_path_offset(fdt, "/chosen"));
+
out:
if (ret) {
kvfree(fdt);
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 6883c5922701..6116abdda590 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -380,6 +380,12 @@ struct kimage {
void *elf_headers;
unsigned long elf_headers_sz;
unsigned long elf_load_addr;
+
+ /* Virtual address of TPM log buffer for kexec syscall */
+ void *tpm_buffer;
+
+ phys_addr_t tpm_buffer_addr;
+ size_t tpm_buffer_size;
};
/* kexec interface functions */
diff --git a/include/linux/of.h b/include/linux/of.h
index 0af611307db2..9afa99950310 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1658,4 +1658,10 @@ static inline int of_overlay_notifier_unregister(struct notifier_block *nb)
#endif
+#if defined(CONFIG_KEXEC_FILE) && defined(CONFIG_OF_FLATTREE)
+void tpm_add_kexec_buffer(struct kimage *image);
+#else
+static inline void tpm_add_kexec_buffer(struct kimage *image) { }
+#endif
+
#endif /* _LINUX_OF_H */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f1a0e4e3fb5c..58c7aaf11883 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -27,6 +27,7 @@
#include <linux/kernel_read_file.h>
#include <linux/syscalls.h>
#include <linux/vmalloc.h>
+#include <linux/of.h>
#include "kexec_internal.h"
#ifdef CONFIG_KEXEC_SIG
@@ -113,6 +114,9 @@ void kimage_file_post_load_cleanup(struct kimage *image)
image->ima_buffer = NULL;
#endif /* CONFIG_IMA_KEXEC */
+ vfree(image->tpm_buffer);
+ image->tpm_buffer = NULL;
+
/* See if architecture has anything to cleanup post load */
arch_kimage_file_post_load_cleanup(image);
@@ -248,6 +252,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
/* IMA needs to pass the measurement list to the next kernel. */
ima_add_kexec_buffer(image);
+ /* Pass the TPM measurement log to next kernel */
+ tpm_add_kexec_buffer(image);
/* Call arch image load handlers */
ldata = arch_kexec_kernel_image_load(image);
--
2.38.1
From: Palmer Dabbelt <[email protected]>
RISC-V recently added kexec_file() support, which uses enables kexec
IMA. We're the first 32-bit platform to support this, so we found a
build bug.
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Palmer Dabbelt <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
---
drivers/of/kexec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index f26d2ba8a371..1373d7e0a9b3 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -250,8 +250,8 @@ static int setup_ima_buffer(const struct kimage *image, void *fdt,
if (ret)
return -EINVAL;
- pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
- image->ima_buffer_addr, image->ima_buffer_size);
+ pr_debug("IMA buffer at 0x%pa, size = 0x%zx\n",
+ &image->ima_buffer_addr, image->ima_buffer_size);
return 0;
}
--
2.38.1
Refactor IMA buffer related functions to make them reusable for carrying
TPM logs across kexec.
Signed-off-by: Stefan Berger <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Cc: Mimi Zohar <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Tested-by: Nageswara R Sastry <[email protected]>
Tested-by: Coiby Xu <[email protected]>
---
v6:
- Add __init to get_kexec_buffer as suggested by Jonathan
v5:
- Rebased on Jonathan McDowell's commit "b69a2afd5afc x86/kexec: Carry
forward IMA measurement log on kexec"
v4:
- Move debug output into setup_buffer()
---
drivers/of/kexec.c | 126 ++++++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 52 deletions(-)
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 1373d7e0a9b3..fa8c0c75adf9 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -117,45 +117,57 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
}
#ifdef CONFIG_HAVE_IMA_KEXEC
-/**
- * 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 __init ima_get_kexec_buffer(void **addr, size_t *size)
+static int __init 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;
- 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 __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ int ret;
+ unsigned long tmp_addr;
+ size_t tmp_size;
+
+ ret = get_kexec_buffer("linux,ima-kexec-buffer", &tmp_addr, &tmp_size);
+ if (ret)
+ return ret;
+
*addr = __va(tmp_addr);
*size = tmp_size;
@@ -188,72 +200,82 @@ int __init ima_free_kexec_buffer(void)
}
#endif
-/**
- * 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,
--
2.38.1
Simplify tpm_read_log_of() by moving reusable parts of the code into
an inline function that makes it commonly available so it can be
used also for kexec support. Call the new of_tpm_get_sml_parameters()
function from the TPM Open Firmware driver.
Signed-off-by: Stefan Berger <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Tested-by: Nageswara R Sastry <[email protected]>
Tested-by: Coiby Xu <[email protected]>
Acked-by: Jarkko Sakkinen <[email protected]>
---
v9:
- Rebased on 6.3-rc7; call tpm_read_log_memory_region if -ENODEV returned
v7:
- Added original comment back into inlined function
v4:
- converted to inline function
---
drivers/char/tpm/eventlog/of.c | 30 +++++-----------------------
include/linux/tpm.h | 36 ++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
index 930fe43d5daf..41688d9cbd3b 100644
--- a/drivers/char/tpm/eventlog/of.c
+++ b/drivers/char/tpm/eventlog/of.c
@@ -51,11 +51,10 @@ static int tpm_read_log_memory_region(struct tpm_chip *chip)
int tpm_read_log_of(struct tpm_chip *chip)
{
struct device_node *np;
- const u32 *sizep;
- const u64 *basep;
struct tpm_bios_log *log;
u32 size;
u64 base;
+ int ret;
log = &chip->log;
if (chip->dev.parent && chip->dev.parent->of_node)
@@ -66,30 +65,11 @@ int tpm_read_log_of(struct tpm_chip *chip)
if (of_property_read_bool(np, "powered-while-suspended"))
chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
- sizep = of_get_property(np, "linux,sml-size", NULL);
- basep = of_get_property(np, "linux,sml-base", NULL);
- if (sizep == NULL && basep == NULL)
+ ret = of_tpm_get_sml_parameters(np, &base, &size);
+ if (ret == -ENODEV)
return tpm_read_log_memory_region(chip);
- if (sizep == NULL || basep == NULL)
- return -EIO;
-
- /*
- * For both vtpm/tpm, firmware has log addr and log size in big
- * endian format. But in case of vtpm, there is a method called
- * sml-handover which is run during kernel init even before
- * device tree is setup. This sml-handover function takes care
- * of endianness and writes to sml-base and sml-size in little
- * endian format. For this reason, vtpm doesn't need conversion
- * but physical tpm needs the conversion.
- */
- if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
- of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
- size = be32_to_cpup((__force __be32 *)sizep);
- base = be64_to_cpup((__force __be64 *)basep);
- } else {
- size = *sizep;
- base = *basep;
- }
+ if (ret < 0)
+ return ret;
if (size == 0) {
dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 4dc97b9f65fb..dd9a376a1dbb 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -461,4 +461,40 @@ static inline struct tpm_chip *tpm_default_chip(void)
return NULL;
}
#endif
+
+#ifdef CONFIG_OF
+static inline int of_tpm_get_sml_parameters(struct device_node *np,
+ u64 *base, u32 *size)
+{
+ const u32 *sizep;
+ const u64 *basep;
+
+ sizep = of_get_property(np, "linux,sml-size", NULL);
+ basep = of_get_property(np, "linux,sml-base", NULL);
+ if (sizep == NULL && basep == NULL)
+ return -ENODEV;
+ if (sizep == NULL || basep == NULL)
+ return -EIO;
+
+ /*
+ * For both vtpm/tpm, firmware has log addr and log size in big
+ * endian format. But in case of vtpm, there is a method called
+ * sml-handover which is run during kernel init even before
+ * device tree is setup. This sml-handover function takes care
+ * of endianness and writes to sml-base and sml-size in little
+ * endian format. For this reason, vtpm doesn't need conversion
+ * but physical tpm needs the conversion.
+ */
+ if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
+ of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
+ *size = be32_to_cpup((__force __be32 *)sizep);
+ *base = be64_to_cpup((__force __be64 *)basep);
+ } else {
+ *size = *sizep;
+ *base = *basep;
+ }
+ return 0;
+}
+#endif
+
#endif
--
2.38.1
Hi Stefan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 6a8f57ae2eb07ab39a6f0ccad60c760743051026]
url: https://github.com/intel-lab-lkp/linux/commits/Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
base: 6a8f57ae2eb07ab39a6f0ccad60c760743051026
patch link: https://lore.kernel.org/r/20230418134409.177485-5-stefanb%40linux.ibm.com
patch subject: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230419/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/16a833d47b9aca53a1b099dea4066b76b7f14ee1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
git checkout 16a833d47b9aca53a1b099dea4066b76b7f14ee1
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash drivers/input/mouse/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from include/linux/irqdomain.h:36,
from include/linux/acpi.h:13,
from include/linux/i2c.h:13,
from drivers/input/mouse/synaptics.c:30:
>> include/linux/of.h:1664:48: warning: 'struct kimage' declared inside parameter list will not be visible outside of this definition or declaration
1664 | static inline void tpm_add_kexec_buffer(struct kimage *image) { }
| ^~~~~~
drivers/input/mouse/synaptics.c:164:27: warning: 'smbus_pnp_ids' defined but not used [-Wunused-const-variable=]
164 | static const char * const smbus_pnp_ids[] = {
| ^~~~~~~~~~~~~
vim +1664 include/linux/of.h
1660
1661 #if defined(CONFIG_KEXEC_FILE) && defined(CONFIG_OF_FLATTREE)
1662 void tpm_add_kexec_buffer(struct kimage *image);
1663 #else
> 1664 static inline void tpm_add_kexec_buffer(struct kimage *image) { }
1665 #endif
1666
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Tue, Apr 18, 2023 at 09:44:09AM -0400, Stefan Berger wrote:
> The memory area of the TPM measurement log is currently not properly
> duplicated for carrying it across kexec when an Open Firmware
> Devicetree is used. Therefore, the contents of the log get corrupted.
> Fix this for the kexec_file_load() syscall by allocating a buffer and
> copying the contents of the existing log into it. The new buffer is
> preserved across the kexec and a pointer to it is available when the new
> kernel is started. To achieve this, store the allocated buffer's address
> in the flattened device tree (fdt) under the name linux,tpm-kexec-buffer
> and search for this entry early in the kernel startup before the TPM
> subsystem starts up. Adjust the pointer in the of-tree stored under
> linux,sml-base to point to this buffer holding the preserved log. The TPM
> driver can then read the base address from this entry when making the log
> available. Invalidate the log by removing 'linux,sml-base' from the
> devicetree if anything goes wrong with updating the buffer.
>
> Use subsys_initcall() to call the function to restore the buffer even if
> the TPM subsystem or driver are not used. This allows the buffer to be
> carried across the next kexec without involvement of the TPM subsystem
> and ensures a valid buffer pointed to by the of-tree.
Hi Stefan,
some minor feedback from my side.
> Use the subsys_initcall(), rather than an ealier initcall, since
nit via checkpatch.pl --codespell: s/ealier/earlier/
> page_is_ram() in get_kexec_buffer() only starts working at this stage.
>
> Signed-off-by: Stefan Berger <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Cc: Eric Biederman <[email protected]>
> Tested-by: Nageswara R Sastry <[email protected]>
> Tested-by: Coiby Xu <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
...
> +void tpm_add_kexec_buffer(struct kimage *image)
> +{
> + struct kexec_buf kbuf = { .image = image, .buf_align = 1,
> + .buf_min = 0, .buf_max = ULONG_MAX,
> + .top_down = true };
> + struct device_node *np;
> + void *buffer;
> + u32 size;
> + u64 base;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_PPC64))
> + return;
> +
> + np = of_find_node_by_name(NULL, "vtpm");
> + if (!np)
> + return;
> +
> + if (of_tpm_get_sml_parameters(np, &base, &size) < 0)
> + return;
> +
> + buffer = vmalloc(size);
> + if (!buffer)
> + return;
> + memcpy(buffer, __va(base), size);
> +
> + kbuf.buffer = buffer;
> + kbuf.bufsz = size;
> + kbuf.memsz = size;
> + ret = kexec_add_buffer(&kbuf);
> + if (ret) {
> + pr_err("Error passing over kexec TPM measurement log buffer: %d\n",
> + ret);
Does buffer need to be freed here?
> + return;
> + }
> +
> + image->tpm_buffer = buffer;
> + image->tpm_buffer_addr = kbuf.mem;
> + image->tpm_buffer_size = size;
> +}
Hi Stefan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 6a8f57ae2eb07ab39a6f0ccad60c760743051026]
url: https://github.com/intel-lab-lkp/linux/commits/Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
base: 6a8f57ae2eb07ab39a6f0ccad60c760743051026
patch link: https://lore.kernel.org/r/20230418134409.177485-5-stefanb%40linux.ibm.com
patch subject: [PATCH v9 4/4] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec
config: x86_64-randconfig-a013-20230417 (https://download.01.org/0day-ci/archive/20230419/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/16a833d47b9aca53a1b099dea4066b76b7f14ee1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Stefan-Berger/drivers-of-kexec-ima-Support-32-bit-platforms/20230418-214600
git checkout 16a833d47b9aca53a1b099dea4066b76b7f14ee1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/mailbox/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/mailbox/mailbox.c:18:
In file included from include/linux/mailbox_client.h:10:
>> include/linux/of.h:1664:48: warning: declaration of 'struct kimage' will not be visible outside of this function [-Wvisibility]
static inline void tpm_add_kexec_buffer(struct kimage *image) { }
^
1 warning generated.
vim +1664 include/linux/of.h
1660
1661 #if defined(CONFIG_KEXEC_FILE) && defined(CONFIG_OF_FLATTREE)
1662 void tpm_add_kexec_buffer(struct kimage *image);
1663 #else
> 1664 static inline void tpm_add_kexec_buffer(struct kimage *image) { }
1665 #endif
1666
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Tue, Apr 18, 2023 at 09:44:06AM -0400, Stefan Berger wrote:
> From: Palmer Dabbelt <[email protected]>
>
> RISC-V recently added kexec_file() support, which uses enables kexec
> IMA. We're the first 32-bit platform to support this, so we found a
> build bug.
>
> Acked-by: Rob Herring <[email protected]>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
Reviewed-by: Jerry Snitselaar <[email protected]>
> ---
> drivers/of/kexec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index f26d2ba8a371..1373d7e0a9b3 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -250,8 +250,8 @@ static int setup_ima_buffer(const struct kimage *image, void *fdt,
> if (ret)
> return -EINVAL;
>
> - pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
> - image->ima_buffer_addr, image->ima_buffer_size);
> + pr_debug("IMA buffer at 0x%pa, size = 0x%zx\n",
> + &image->ima_buffer_addr, image->ima_buffer_size);
>
> return 0;
> }
> --
> 2.38.1
>
On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> Simplify tpm_read_log_of() by moving reusable parts of the code into
> an inline function that makes it commonly available so it can be
> used also for kexec support. Call the new of_tpm_get_sml_parameters()
> function from the TPM Open Firmware driver.
>
> Signed-off-by: Stefan Berger <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Tested-by: Nageswara R Sastry <[email protected]>
> Tested-by: Coiby Xu <[email protected]>
> Acked-by: Jarkko Sakkinen <[email protected]>
>
Reviewed-by: Jerry Snitselaar <[email protected]>
> ---
> v9:
> - Rebased on 6.3-rc7; call tpm_read_log_memory_region if -ENODEV returned
>
> v7:
> - Added original comment back into inlined function
>
> v4:
> - converted to inline function
> ---
> drivers/char/tpm/eventlog/of.c | 30 +++++-----------------------
> include/linux/tpm.h | 36 ++++++++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
> index 930fe43d5daf..41688d9cbd3b 100644
> --- a/drivers/char/tpm/eventlog/of.c
> +++ b/drivers/char/tpm/eventlog/of.c
> @@ -51,11 +51,10 @@ static int tpm_read_log_memory_region(struct tpm_chip *chip)
> int tpm_read_log_of(struct tpm_chip *chip)
> {
> struct device_node *np;
> - const u32 *sizep;
> - const u64 *basep;
> struct tpm_bios_log *log;
> u32 size;
> u64 base;
> + int ret;
>
> log = &chip->log;
> if (chip->dev.parent && chip->dev.parent->of_node)
> @@ -66,30 +65,11 @@ int tpm_read_log_of(struct tpm_chip *chip)
> if (of_property_read_bool(np, "powered-while-suspended"))
> chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
>
> - sizep = of_get_property(np, "linux,sml-size", NULL);
> - basep = of_get_property(np, "linux,sml-base", NULL);
> - if (sizep == NULL && basep == NULL)
> + ret = of_tpm_get_sml_parameters(np, &base, &size);
> + if (ret == -ENODEV)
> return tpm_read_log_memory_region(chip);
> - if (sizep == NULL || basep == NULL)
> - return -EIO;
> -
> - /*
> - * For both vtpm/tpm, firmware has log addr and log size in big
> - * endian format. But in case of vtpm, there is a method called
> - * sml-handover which is run during kernel init even before
> - * device tree is setup. This sml-handover function takes care
> - * of endianness and writes to sml-base and sml-size in little
> - * endian format. For this reason, vtpm doesn't need conversion
> - * but physical tpm needs the conversion.
> - */
> - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
> - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
> - size = be32_to_cpup((__force __be32 *)sizep);
> - base = be64_to_cpup((__force __be64 *)basep);
> - } else {
> - size = *sizep;
> - base = *basep;
> - }
> + if (ret < 0)
> + return ret;
>
> if (size == 0) {
> dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 4dc97b9f65fb..dd9a376a1dbb 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -461,4 +461,40 @@ static inline struct tpm_chip *tpm_default_chip(void)
> return NULL;
> }
> #endif
> +
> +#ifdef CONFIG_OF
> +static inline int of_tpm_get_sml_parameters(struct device_node *np,
> + u64 *base, u32 *size)
> +{
> + const u32 *sizep;
> + const u64 *basep;
> +
> + sizep = of_get_property(np, "linux,sml-size", NULL);
> + basep = of_get_property(np, "linux,sml-base", NULL);
> + if (sizep == NULL && basep == NULL)
> + return -ENODEV;
> + if (sizep == NULL || basep == NULL)
> + return -EIO;
> +
> + /*
> + * For both vtpm/tpm, firmware has log addr and log size in big
> + * endian format. But in case of vtpm, there is a method called
> + * sml-handover which is run during kernel init even before
> + * device tree is setup. This sml-handover function takes care
> + * of endianness and writes to sml-base and sml-size in little
> + * endian format. For this reason, vtpm doesn't need conversion
> + * but physical tpm needs the conversion.
> + */
> + if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
> + of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
> + *size = be32_to_cpup((__force __be32 *)sizep);
> + *base = be64_to_cpup((__force __be64 *)basep);
> + } else {
> + *size = *sizep;
> + *base = *basep;
> + }
> + return 0;
> +}
> +#endif
> +
> #endif
> --
> 2.38.1
>
On Tue, Apr 18, 2023 at 09:44:08AM -0400, Stefan Berger wrote:
> Refactor IMA buffer related functions to make them reusable for carrying
> TPM logs across kexec.
>
> Signed-off-by: Stefan Berger <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Cc: Mimi Zohar <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Tested-by: Nageswara R Sastry <[email protected]>
> Tested-by: Coiby Xu <[email protected]>
>
Reviewed-by: Jerry Snitselaar <[email protected]>
> ---
> v6:
> - Add __init to get_kexec_buffer as suggested by Jonathan
>
> v5:
> - Rebased on Jonathan McDowell's commit "b69a2afd5afc x86/kexec: Carry
> forward IMA measurement log on kexec"
> v4:
> - Move debug output into setup_buffer()
> ---
> drivers/of/kexec.c | 126 ++++++++++++++++++++++++++-------------------
> 1 file changed, 74 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index 1373d7e0a9b3..fa8c0c75adf9 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -117,45 +117,57 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
> }
>
> #ifdef CONFIG_HAVE_IMA_KEXEC
> -/**
> - * 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 __init ima_get_kexec_buffer(void **addr, size_t *size)
> +static int __init 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;
>
> - 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 __init ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> + int ret;
> + unsigned long tmp_addr;
> + size_t tmp_size;
> +
> + ret = get_kexec_buffer("linux,ima-kexec-buffer", &tmp_addr, &tmp_size);
> + if (ret)
> + return ret;
> +
> *addr = __va(tmp_addr);
> *size = tmp_size;
>
> @@ -188,72 +200,82 @@ int __init ima_free_kexec_buffer(void)
> }
> #endif
>
> -/**
> - * 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,
> --
> 2.38.1
>
On 5/24/23 19:16, Jerry Snitselaar wrote:
> On Tue, Apr 18, 2023 at 09:44:08AM -0400, Stefan Berger wrote:
>> Refactor IMA buffer related functions to make them reusable for carrying
>> TPM logs across kexec.
>>
>> Signed-off-by: Stefan Berger <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Frank Rowand <[email protected]>
>> Cc: Mimi Zohar <[email protected]>
>> Reviewed-by: Mimi Zohar <[email protected]>
>> Reviewed-by: Rob Herring <[email protected]>
>> Tested-by: Nageswara R Sastry <[email protected]>
>> Tested-by: Coiby Xu <[email protected]>
>>
>
> Reviewed-by: Jerry Snitselaar <[email protected]>
Thanks a lot for looking at the patches. Unfortunately this series only resolves the issue with the newer kexec call but we have seen systems where the older one is used for setting a crash kernel and when the crash happens we're back to square one. I have been trying to embed the log into the memory of the flattened of-device tree but that also turns out to be not so straight forward.
Stefan
>> ---
>> v6:
>> - Add __init to get_kexec_buffer as suggested by Jonathan
>>
>> v5:
>> - Rebased on Jonathan McDowell's commit "b69a2afd5afc x86/kexec: Carry
>> forward IMA measurement log on kexec"
>> v4:
>> - Move debug output into setup_buffer()
>> ---
>> drivers/of/kexec.c | 126 ++++++++++++++++++++++++++-------------------
>> 1 file changed, 74 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
>> index 1373d7e0a9b3..fa8c0c75adf9 100644
>> --- a/drivers/of/kexec.c
>> +++ b/drivers/of/kexec.c
>> @@ -117,45 +117,57 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
>> }
>>
>> #ifdef CONFIG_HAVE_IMA_KEXEC
>> -/**
>> - * 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 __init ima_get_kexec_buffer(void **addr, size_t *size)
>> +static int __init 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;
>>
>> - 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 __init ima_get_kexec_buffer(void **addr, size_t *size)
>> +{
>> + int ret;
>> + unsigned long tmp_addr;
>> + size_t tmp_size;
>> +
>> + ret = get_kexec_buffer("linux,ima-kexec-buffer", &tmp_addr, &tmp_size);
>> + if (ret)
>> + return ret;
>> +
>> *addr = __va(tmp_addr);
>> *size = tmp_size;
>>
>> @@ -188,72 +200,82 @@ int __init ima_free_kexec_buffer(void)
>> }
>> #endif
>>
>> -/**
>> - * 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,
>> --
>> 2.38.1
>>
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec
On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
> On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> > Simplify tpm_read_log_of() by moving reusable parts of the code into
> > an inline function that makes it commonly available so it can be
> > used also for kexec support. Call the new of_tpm_get_sml_parameters()
> > function from the TPM Open Firmware driver.
> >
> > Signed-off-by: Stefan Berger <[email protected]>
> > Cc: Jarkko Sakkinen <[email protected]>
> > Cc: Jason Gunthorpe <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Frank Rowand <[email protected]>
> > Reviewed-by: Mimi Zohar <[email protected]>
> > Tested-by: Nageswara R Sastry <[email protected]>
> > Tested-by: Coiby Xu <[email protected]>
> > Acked-by: Jarkko Sakkinen <[email protected]>
> >
>
> Reviewed-by: Jerry Snitselaar <[email protected]>
If I just pick tpm only patches they won't apply so maybe TPM changes
should be better separated if that is by any means possible.
Open for counter proposals. Just my thoughts...
I.e. I'm mainly wondering why TPM patches depend on IMA patches?
BR, Jarkko
On 6/9/23 14:18, Jarkko Sakkinen wrote:
> On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
>> On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
>>> Simplify tpm_read_log_of() by moving reusable parts of the code into
>>> an inline function that makes it commonly available so it can be
>>> used also for kexec support. Call the new of_tpm_get_sml_parameters()
>>> function from the TPM Open Firmware driver.
>>>
>>> Signed-off-by: Stefan Berger <[email protected]>
>>> Cc: Jarkko Sakkinen <[email protected]>
>>> Cc: Jason Gunthorpe <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Frank Rowand <[email protected]>
>>> Reviewed-by: Mimi Zohar <[email protected]>
>>> Tested-by: Nageswara R Sastry <[email protected]>
>>> Tested-by: Coiby Xu <[email protected]>
>>> Acked-by: Jarkko Sakkinen <[email protected]>
>>>
>>
>> Reviewed-by: Jerry Snitselaar <[email protected]>
>
> If I just pick tpm only patches they won't apply so maybe TPM changes
> should be better separated if that is by any means possible.
Per the comment here I am putting this series here on hold.
https://lore.kernel.org/linux-integrity/[email protected]/T/#m03745c2af2c46f19f329522fcb6ccb2bf2eaedc7
BR,
Stefan
On Fri Jun 9, 2023 at 9:49 PM EEST, Stefan Berger wrote:
>
>
> On 6/9/23 14:18, Jarkko Sakkinen wrote:
> > On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
> >> On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> >>> Simplify tpm_read_log_of() by moving reusable parts of the code into
> >>> an inline function that makes it commonly available so it can be
> >>> used also for kexec support. Call the new of_tpm_get_sml_parameters()
> >>> function from the TPM Open Firmware driver.
> >>>
> >>> Signed-off-by: Stefan Berger <[email protected]>
> >>> Cc: Jarkko Sakkinen <[email protected]>
> >>> Cc: Jason Gunthorpe <[email protected]>
> >>> Cc: Rob Herring <[email protected]>
> >>> Cc: Frank Rowand <[email protected]>
> >>> Reviewed-by: Mimi Zohar <[email protected]>
> >>> Tested-by: Nageswara R Sastry <[email protected]>
> >>> Tested-by: Coiby Xu <[email protected]>
> >>> Acked-by: Jarkko Sakkinen <[email protected]>
> >>>
> >>
> >> Reviewed-by: Jerry Snitselaar <[email protected]>
> >
> > If I just pick tpm only patches they won't apply so maybe TPM changes
> > should be better separated if that is by any means possible.
>
> Per the comment here I am putting this series here on hold.
> https://lore.kernel.org/linux-integrity/[email protected]/T/#m03745c2af2c46f19f329522fcb6ccb2bf2eaedc7
OK, cool.
I've mentioned this in few other emails but say this here too:
I was relocating for last couple of weeks, and thus some latency.
If you choose to repost the series, I'm happy to review it, thanks
:-)
BR, Jarkko
On Fri, 2023-06-09 at 14:49 -0400, Stefan Berger wrote:
>
> On 6/9/23 14:18, Jarkko Sakkinen wrote:
> > On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
> > > On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> > > > Simplify tpm_read_log_of() by moving reusable parts of the code into
> > > > an inline function that makes it commonly available so it can be
> > > > used also for kexec support. Call the new of_tpm_get_sml_parameters()
> > > > function from the TPM Open Firmware driver.
> > > >
> > > > Signed-off-by: Stefan Berger <[email protected]>
> > > > Cc: Jarkko Sakkinen <[email protected]>
> > > > Cc: Jason Gunthorpe <[email protected]>
> > > > Cc: Rob Herring <[email protected]>
> > > > Cc: Frank Rowand <[email protected]>
> > > > Reviewed-by: Mimi Zohar <[email protected]>
> > > > Tested-by: Nageswara R Sastry <[email protected]>
> > > > Tested-by: Coiby Xu <[email protected]>
> > > > Acked-by: Jarkko Sakkinen <[email protected]>
> > > >
> > >
> > > Reviewed-by: Jerry Snitselaar <[email protected]>
> >
> > If I just pick tpm only patches they won't apply so maybe TPM changes
> > should be better separated if that is by any means possible.
>
> Per the comment here I am putting this series here on hold.
> https://lore.kernel.org/linux-integrity/[email protected]/T/#m03745c2af2c46f19f329522fcb6ccb2bf2eaedc7
Hi, sorry for late response. The Midsummer weekend really
messed my schedules (it is a big thing Finland). This year
the timing with the kernel cycle has been conflicting.
OK cool.
BR, Jarkko