(),On Tue, Jun 14, 2022 at 10:13 AM Stefan Berger <stefanb@xxxxxxxxxxxxx> 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.
This series really needs a wider Cc list of folks that worry about
TPMs and kexec.
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: Frank Rowand <frowand.list@xxxxxxxxx>
> Cc: Eric Biederman <ebiederm@xxxxxxxxxxxx>
> ---
> drivers/of/device.c | 24 +++++
> drivers/of/kexec.c | 190 +++++++++++++++++++++++++++++++++++++-
> include/linux/kexec.h | 6 ++
> include/linux/of.h | 5 +
> include/linux/of_device.h | 3 +
> kernel/kexec_file.c | 6 ++
> 6 files changed, 233 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 874f031442dc..0cbd47b1cabc 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -382,3 +382,27 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
> return 0;
> }
> EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
> +
> +int of_tpm_get_sml_parameters(struct device_node *np, u64 *base, u32 *size)
of/device.c is for functions that work on a struct device. This is not
the case here.
> +{
> + const u32 *sizep;
> + const u64 *basep;
> +
> + sizep = of_get_property(np, "linux,sml-size", NULL);
> + basep = of_get_property(np, "linux,sml-base", NULL);
Any new properties need a schema. For chosen, that's located here[1].
The more common pattern for similar properties is <base size>.
Don't use of_get_property(), but the typed functions instead.
> + if (sizep == NULL && basep == NULL)
> + return -ENODEV;
> + if (sizep == NULL || basep == NULL)
> + return -EIO;
Do we really need the error distinction?
> +
> + 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;
What? Some platforms aren't properly encoded? Fix the firmware.
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_tpm_get_sml_parameters);
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index eef6f3b9939c..db5441123a70 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -14,6 +14,7 @@
> #include <linux/memblock.h>
> #include <linux/libfdt.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/of_fdt.h>
> #include <linux/random.h>
> #include <linux/slab.h>
> @@ -221,7 +222,6 @@ static void remove_ima_buffer(void *fdt, int chosen_node)
> pr_debug("Removed old IMA buffer reservation.\n");
> }
>
> -#ifdef CONFIG_IMA_KEXEC
> static int setup_buffer(void *fdt, int chosen_node, const char *name,
> uint64_t addr, uint64_t size)
> {
> @@ -243,6 +243,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.
> @@ -275,6 +276,190 @@ 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 tpm_get_kexec_buffer(void **phyaddr, size_t *size)
> +{
> + int ret;
> + unsigned long tmp_addr;
> + size_t tmp_size;
> +
> + ret = get_kexec_buffer("linux,tpm-kexec-buffer", &tmp_addr, &tmp_size);
Again, must be documented.
> + if (ret)
> + return ret;
> +
> + *phyaddr = (void *)tmp_addr;
> + *size = tmp_size;
> +
> + return 0;
> +}
> +
> +/**
> + * tpm_free_kexec_buffer - free memory used by the IMA buffer
> + */
> +static int 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)
> +{
> + int ret;
> +
> + ret = remove_buffer(fdt, chosen_node, "linux,tpm-kexec-buffer");
> + if (!ret)
> + pr_debug("Removed old TPM log buffer reservation.\n");
Do we really need this print? If so, perhaps in remove_buffer() rather
than having every caller do it.
> +}
> +
> +/**
> + * 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)
> +{
> + 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;
> +
> + 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;
> + struct device_node *np;
> + void *phyaddr;
> + size_t size;
> + u32 oflogsize;
> + u64 unused;
> + int ret;
> +
> + ret = tpm_get_kexec_buffer(&phyaddr, &size);
> + if (ret)
> + return 0;
> +
> + /*
> + * If any one of the following steps fails then the next kexec will
> + * cause issues due to linux,sml-base pointing to a stale buffer.
> + */
> + np = of_find_node_by_name(NULL, "vtpm");
This seems pretty IBM specific.
> + if (!np)
> + goto err_free_memblock;
> +
> + /* logsize must not have changed */
> + if (of_tpm_get_sml_parameters(np, &unused, &oflogsize) < 0)
> + goto err_free_memblock;
> + 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);
> + if (!newprop->name)
> + goto err_free_newprop;
> +
> + newprop->length = sizeof(phyaddr);
> +
> + newprop->value = kmalloc(sizeof(u64), GFP_KERNEL);
> + if (!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 {
> + *(u64 *)newprop->value = (u64)phyaddr;
> + }
> +
> + ret = of_update_property(np, newprop);
Just FYI for now, there's some work happening on a better API for
writing nodes and properties.
> + if (ret) {
> + pr_err("Could not update linux,sml-base with new address");
> + goto err_free_newprop_struct;
> + }
> +
> + goto exit;
> +
> +err_free_newprop_struct:
> + kfree(newprop->name);
> +err_free_newprop:
> + kfree(newprop);
> +err_free_memblock:
> + memblock_phys_free((phys_addr_t)phyaddr, size);
> +exit:
> + tpm_of_remove_kexec_buffer();
> +
> + return ret;
> +}
> +postcore_initcall(tpm_post_kexec);
Would be better if this is called explicitly at the right time when
needed rather than using an initcall.
> +
> /*
> * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
> *
> @@ -464,6 +649,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 58d1b58a971e..a0fd7ac0398c 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -319,6 +319,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;
Probably should use the same types as other buffers...
> };
Rob
[1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/chosen.yaml
