[PATCH 3/3] tpm: allocate tpm event log if missing

[Simon Glass]

Hi Ludwig,

On 2026-04-29T11:41:40, Ludwig Nussel <ludwig.nussel at siemens.com> wrote:
> tpm: allocate tpm event log if missing
>
> Allocate and attach the TPM event log to the TPM device if not
> initialized yet. The address is copied into the device tree for the
> kernel so the event log is accessible from Linux userspace too.
>
> Signed-off-by: Ludwig Nussel <ludwig.nussel at siemens.com>
>
> boot/Kconfig         |   1 +
>  boot/bootm.c         |  18 ++++-----
>  boot/image-fdt.c     |  67 ++++++++++++++++++++++++++++++++++
>  include/tpm-common.h |   1 +
>  include/tpm_tcg2.h   |   9 +++--
>  lib/tpm_tcg2.c       | 101 ++++++++++++++++++++++++++++++++++++++++-----------
>  6 files changed, 160 insertions(+), 37 deletions(-)

> diff --git a/boot/bootm.c b/boot/bootm.c
> @@ -973,7 +969,7 @@ unmap_initrd:
>
>  unmap_image:
>               unmap_sysmem(image_buf);
> -             tcg2_measurement_term(dev, &elog, ret != 0);
> +             tcg2_measurement_term(dev, NULL, ret != 0);

This passes NULL for elog, but tcg2_measurement_term() still does if
(elog->log) unmap_physmem(...) and calls tcg2_measure_event() which
ends up in tcg2_log_append_check() doing if (!elog->log_size). Both
deref a NULL pointer. tcg2_measure_data() looks the log up via
tcg2_platform_get_dev_log() when elog is NULL - so _term and
tcg2_measure_event() need the same treatment.

> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> @@ -24,6 +24,10 @@
>  #include <asm/io.h>
>  #include <dm/ofnode.h>
>  #include <tee/optee.h>
> +#if defined(CONFIG_MEASURED_BOOT)
> +#include <tpm_tcg2.h>
> +#include <dm/device.h>
> +#endif

Please drop the #if around the includes, and inside the function body
prefer if (IS_ENABLED(CONFIG_MEASURED_BOOT)) over #if defined(...) so
the compiler still type-checks the body when the option is off. That
is the U-Boot convention.

> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> @@ -589,6 +593,67 @@ __weak int arch_fixup_fdt(void *blob)
> +     strlcat(path, '/linux,sml-base', sizeof(path));
> +     if (fdt_path_offset(blob, path) >= 0) {
> +             log_warning("kernel dt already defines an event log address at %s\n", path);
> +             return 0;
> +     }
> +     end = strrchr(path, '/');
> +     *end = 0;

fdt_path_offset() resolves nodes, not properties, so this never
matches - linux,sml-base is a property of the TPM node. Use
fdt_getprop() against the node offset you already located above.

> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> @@ -589,6 +593,67 @@ __weak int arch_fixup_fdt(void *blob)
> +     fdt64_t a = cpu_to_fdt64((u64)virt_to_phys(log->log));
> +     fdt32_t s = cpu_to_fdt32((u32)log->log_position);

linux,sml-size is documented (and used by Linux) as the size of the
reserved buffer, not the bytes written so far. Otherwise the kernel
cannot append further entries. This should be log->log_size. Please
also give the locals descriptive names (sml_base/sml_size).

> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> @@ -631,6 +696,8 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb)
> +     copy_tpm_event_log(blob);
> +

The return value is silently dropped. At minimum log a warning when it
fails with anything other than -ENOENT, otherwise misconfigurations
(e.g. allocation succeeded but fdt_find_and_setprop failed) will be
invisible.

> diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c
> @@ -564,11 +576,29 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct tcg2_event_log *elog,
> +     } else if (tcg2_platform_get_dev_log(dev)) {
> +             return -EEXIST;
> +     }
>
>       rc = tcg2_platform_get_log(dev, (void **)&log.log, &log.log_size);
> -     if (!rc) {
> +     /* no existing event log found and none allocated yet */
> +     if (rc && !elog) {
> +             /* magic size value. Same as in acpi_table.c */
> +             log.log_size = 0x10000;
> +             log.log = devm_kzalloc(dev, log.log_size, 0);
> +             if (log.log) {
> +                     log.allocated = true;
> +                     ignore_existing_log = true;
> +             } else {
> +                     log_err("Failed to allocate %u bytes event log\n", log.log_size);
> +             }
> +     }
> +
> +     if (log.log) {

When tcg2_platform_get_log() fails and elog is non-NULL we fall
through with log.log uninitialised, then test if (log.log) on stack
garbage. Either zero-initialise struct tcg2_event_log log = {} at the
top, or skip the block explicitly.

For 0x10000, please introduce a named constant (e.g.
TPM2_EVENT_LOG_SIZE) shared with acpi_table.c rather than duplicating
the comment.

> diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c
> @@ -580,26 +610,40 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct tcg2_event_log *elog,
> +             } else {
> +                     struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> +
> +                     elog = devm_kzalloc(dev, sizeof(struct tcg2_event_log), 0);
> +
>                       elog->log = log.log;

devm_kzalloc() can return NULL - this dereferences it immediately, and
on failure leaks the 64KB buffer allocated above. Please check and
unwind cleanly.

> diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c
> @@ -662,6 +710,15 @@ void tcg2_measurement_term(struct udevice *dev, struct tcg2_event_log *elog,
> +struct tcg2_event_log *tcg2_platform_get_dev_log(struct udevice *dev)
> +{
> +     struct tpm_chip_priv *priv;
> +
> +     priv = dev_get_uclass_priv(dev);
> +
> +     return priv->log;
> +}

Missing kernel-doc.

> diff --git a/include/tpm-common.h b/include/tpm-common.h
> @@ -83,6 +83,7 @@ struct tpm_chip_priv {
>       u32 active_banks[TPM2_NUM_PCR_BANKS];
>  #endif
>       bool plat_hier_disabled;
> +     void *log; /* struct tcg2_event_log */

Please use a forward declaration ('struct tcg2_event_log;') and type
the field properly as struct tcg2_event_log *log. Hiding the type
behind void * loses compiler checking and forces every reader to trust
the comment.

Regards,
Simon