2022-07-01 02:27:53

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 0/5] tpm: Preserve TPM measurement log across kexec (ppc64)

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

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

Vaibhav Jain (1):
of: check previous kernel's ima-kexec-buffer against memory bounds

drivers/char/tpm/eventlog/of.c | 31 +---
drivers/of/kexec.c | 328 +++++++++++++++++++++++++++++----
include/linux/kexec.h | 6 +
include/linux/of.h | 8 +-
include/linux/tpm.h | 27 +++
kernel/kexec_file.c | 6 +
6 files changed, 340 insertions(+), 66 deletions(-)


base-commit: 03c765b0e3b4cb5063276b086c76f7a612856a9a
--
2.35.1


2022-07-01 02:28:21

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 5/5] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec

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]>

---
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 | 214 +++++++++++++++++++++++++++++++++++++++++-
include/linux/kexec.h | 6 ++
include/linux/of.h | 8 +-
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 0710703acfb0..6d1eac0e2e3c 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -18,6 +18,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

@@ -241,7 +243,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)
{
@@ -265,6 +266,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.
@@ -287,6 +289,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
*
@@ -485,6 +694,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 ce6536f1d269..f0940396c3d4 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -349,6 +349,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 f0a5d6b10c5a..d95f3fc2eccf 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -100,6 +100,8 @@ struct of_reconfig_data {
struct property *old_prop;
};

+struct kimage;
+
/* initialize a node */
extern struct kobj_type of_node_ktype;
extern const struct fwnode_operations of_fwnode_ops;
@@ -436,13 +438,13 @@ int of_map_id(struct device_node *np, u32 id,

phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);

-struct kimage;
void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
int ima_get_kexec_buffer(void **addr, size_t *size);
int ima_free_kexec_buffer(void);
+void tpm_add_kexec_buffer(struct kimage *image);
#else /* CONFIG_OF */

static inline void of_core_init(void)
@@ -844,6 +846,10 @@ static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
return PHYS_ADDR_MAX;
}

+static inline void tpm_add_kexec_buffer(struct kimage *image)
+{
+}
+
#define of_match_ptr(_ptr) NULL
#define of_match_node(_matches, _node) NULL
#endif /* CONFIG_OF */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 145321a5e798..d2e0751852dc 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"

static int kexec_calculate_store_digests(struct kimage *image);
@@ -137,6 +138,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);

@@ -243,6 +247,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.35.1

2022-07-01 02:29:12

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 1/5] of: check previous kernel's ima-kexec-buffer against memory bounds

From: Vaibhav Jain <[email protected]>

Presently ima_get_kexec_buffer() doesn't check if the previous kernel's
ima-kexec-buffer lies outside the addressable memory range. This can result
in a kernel panic if the new kernel is booted with 'mem=X' arg and the
ima-kexec-buffer was allocated beyond that range by the previous kernel.
The panic is usually of the form below:

$ sudo kexec --initrd initrd vmlinux --append='mem=16G'

<snip>
BUG: Unable to handle kernel data access on read at 0xc000c01fff7f0000
Faulting instruction address: 0xc000000000837974
Oops: Kernel access of bad area, sig: 11 [#1]
<snip>
NIP [c000000000837974] ima_restore_measurement_list+0x94/0x6c0
LR [c00000000083b55c] ima_load_kexec_buffer+0xac/0x160
Call Trace:
[c00000000371fa80] [c00000000083b55c] ima_load_kexec_buffer+0xac/0x160
[c00000000371fb00] [c0000000020512c4] ima_init+0x80/0x108
[c00000000371fb70] [c0000000020514dc] init_ima+0x4c/0x120
[c00000000371fbf0] [c000000000012240] do_one_initcall+0x60/0x2c0
[c00000000371fcc0] [c000000002004ad0] kernel_init_freeable+0x344/0x3ec
[c00000000371fda0] [c0000000000128a4] kernel_init+0x34/0x1b0
[c00000000371fe10] [c00000000000ce64] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
f92100b8 f92100c0 90e10090 910100a0 4182050c 282a0017 3bc00000 40810330
7c0802a6 fb610198 7c9b2378 f80101d0 <a1240000> 2c090001 40820614 e9240010
---[ end trace 0000000000000000 ]---

Fix this issue by checking returned PFN range of previous kernel's
ima-kexec-buffer with page_is_ram() to ensure correct memory bounds.

Fixes: 467d27824920 ("powerpc: ima: get the kexec buffer passed by the previous kernel")
[ Several tags removed; for testing by krobot ]
Signed-off-by: Vaibhav Jain <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/of/kexec.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc552be..91b04b04eec4 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -126,6 +126,7 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
{
int ret, len;
unsigned long tmp_addr;
+ unsigned long start_pfn, end_pfn;
size_t tmp_size;
const void *prop;

@@ -140,6 +141,22 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
if (ret)
return ret;

+ /* Do some sanity on the returned size for the ima-kexec buffer */
+ if (!tmp_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);
+ 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);
+ return -EINVAL;
+ }
+
*addr = __va(tmp_addr);
*size = tmp_size;

--
2.35.1

2022-07-01 02:41:18

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 4/5] of: kexec: Refactor IMA buffer related functions to make them reusable

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]>

---
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,
--
2.35.1

2022-07-01 02:43:18

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 3/5] tpm: of: Make of-tree specific function commonly available

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]>

---
v4:
- converted to inline function
---
drivers/char/tpm/eventlog/of.c | 31 +++++--------------------------
include/linux/tpm.h | 27 +++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
index a9ce66d09a75..f9462d19632e 100644
--- a/drivers/char/tpm/eventlog/of.c
+++ b/drivers/char/tpm/eventlog/of.c
@@ -12,6 +12,7 @@

#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/tpm.h>
#include <linux/tpm_eventlog.h>

#include "../tpm.h"
@@ -20,11 +21,10 @@
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)
@@ -35,30 +35,9 @@ 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)
- 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;
- }
+ ret = of_tpm_get_sml_parameters(np, &base, &size);
+ 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 dfeb25a0362d..b3dff255bc58 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -460,4 +460,31 @@ 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;
+
+ 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.35.1

2022-07-01 02:44:09

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 2/5] drivers: of: kexec ima: Support 32-bit platforms

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.

[ Several tags removed; for testing by krobot ]
Signed-off-by: Palmer Dabbelt <[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 91b04b04eec4..c4f9b6655a2e 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -253,8 +253,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.35.1

2022-07-05 22:57:41

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] of: kexec: Refactor IMA buffer related functions to make them reusable

[Cc'ing Borislav Petkov <[email protected]>, Jonathan McDowell <[email protected]
>]

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 <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Cc: Mimi Zohar <[email protected]>

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

thanks,

Mimi

>
> ---
> 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,


2022-07-06 14:07:02

by Jonathan McDowell

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] of: kexec: Refactor IMA buffer related functions to make them reusable

On Tue, Jul 05, 2022 at 06:46:54PM -0400, Mimi Zohar wrote:
> [Cc'ing Borislav Petkov <[email protected]>, Jonathan McDowell <[email protected]
> >]
>
> 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 <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Frank Rowand <[email protected]>
> > Cc: Mimi Zohar <[email protected]>
>
> 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.

> > ---
> > 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,

2022-07-06 15:49:55

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] of: kexec: Refactor IMA buffer related functions to make them reusable



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 <[email protected]>, Jonathan McDowell <[email protected]
>>> ]
>>
>> 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 <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Frank Rowand <[email protected]>
>>> Cc: Mimi Zohar <[email protected]>
>>
>> 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,

2022-07-11 00:41:26

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] tpm: of: Make of-tree specific function commonly available

On Thu, Jun 30, 2022 at 10:26:01PM -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]>
>
> ---
> v4:
> - converted to inline function
> ---
> drivers/char/tpm/eventlog/of.c | 31 +++++--------------------------
> include/linux/tpm.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
> index a9ce66d09a75..f9462d19632e 100644
> --- a/drivers/char/tpm/eventlog/of.c
> +++ b/drivers/char/tpm/eventlog/of.c
> @@ -12,6 +12,7 @@
>
> #include <linux/slab.h>
> #include <linux/of.h>
> +#include <linux/tpm.h>
> #include <linux/tpm_eventlog.h>
>
> #include "../tpm.h"
> @@ -20,11 +21,10 @@
> 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)
> @@ -35,30 +35,9 @@ 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)
> - 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;
> - }
> + ret = of_tpm_get_sml_parameters(np, &base, &size);
> + 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 dfeb25a0362d..b3dff255bc58 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -460,4 +460,31 @@ 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;
> +
> + 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.35.1
>


Reviewed-by: Jarkko Sakkinen <[email protected]>

BR, Jarkko