Re: [PATCH 2/3] tpm/kexec: Duplicate TPM measurement log in of-tree for kexec

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

 



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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux