[PATCH 3/3] bootm: Support boot measurement

[Simon Glass]

Hi Eddie,

On Tue, 3 Jan 2023 at 13:42, Eddie James <eajames at linux.ibm.com> wrote:
>
> Add a configuration option to measure the boot through the bootm
> function.
>
> Signed-off-by: Eddie James <eajames at linux.ibm.com>
> ---
>  boot/bootm.c    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>  cmd/bootm.c     |  2 ++
>  common/Kconfig  |  6 ++++++
>  include/image.h |  1 +
>  4 files changed, 62 insertions(+)

Please add a test for this feature, e.g. in test/boot or test/bootm.c
or in test/py/tests/test_fit.py or similar.

>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index a4c0870c0f..7f64d79035 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -22,6 +22,9 @@
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <linux/sizes.h>
> +#if defined(CONFIG_MEASURED_BOOT)
> +#include <tpm-v2.h>
> +#endif

Drop the #ifdef as you can always include headers

>  #if defined(CONFIG_CMD_USB)
>  #include <usb.h>
>  #endif
> @@ -713,6 +716,56 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
>         if (!ret && (states & BOOTM_STATE_FINDOTHER))
>                 ret = bootm_find_other(cmdtp, flag, argc, argv);
>
> +#if defined(CONFIG_MEASURED_BOOT)
> +       if (!ret && (states & BOOTM_STATE_MEASURE)) {

Please avoid #ifdef

Are you using patman? If so, it will automatically warn you about this
sort of thing.

       if (!IS_ENABLED(CONFIG_MEASURED_BOOT) && ret && (states &
BOOTM_STATE_MEASURE)) {

> +               void *initrd_buf;
> +               void *image_buf;
> +               const char *s;
> +               u32 rd_len;
> +

Please put all this code in a separate function.

> +               ret = tcg2_measurement_init();
> +               if (ret)
> +                       goto measure_err;
> +
> +               image_buf = map_sysmem(images->os.image_start,
> +                                      images->os.image_len);
> +               ret = tcg2_measure_data(8, images->os.image_len, image_buf,
> +                                       EV_COMPACT_HASH, strlen("linux") + 1,
> +                                       (u8 *)"linux");
> +               if (ret)
> +                       goto unmap_image;
> +
> +               rd_len = images->rd_end - images->rd_start;
> +               initrd_buf = map_sysmem(images->rd_start, rd_len);
> +               ret = tcg2_measure_data(8, rd_len, initrd_buf, EV_COMPACT_HASH,
> +                                       strlen("initrd") + 1, (u8 *)"initrd");
> +               if (ret)
> +                       goto unmap_initrd;
> +
> +               ret = tcg2_measure_data(9, images->ft_len,
> +                                       (u8 *)images->ft_addr,
> +                                       EV_TABLE_OF_DEVICES, strlen("dts") + 1,
> +                                       (u8 *)"dts");
> +               if (ret)
> +                       goto unmap_initrd;
> +
> +               s = env_get("bootargs");
> +               if (!s)
> +                       s = "";
> +               tcg2_measure_data(1, strlen(s) + 1, (u8 *)s,
> +                                 EV_PLATFORM_CONFIG_FLAGS, strlen(s) + 1,
> +                                 (u8 *)s);
> +
> +unmap_initrd:
> +               unmap_sysmem(initrd_buf);
> +unmap_image:
> +               unmap_sysmem(image_buf);
> +               tcg2_measurement_term();
> +measure_err:
> +               ret = 0;
> +       }
> +#endif
> +
>         /* Load the OS */
>         if (!ret && (states & BOOTM_STATE_LOADOS)) {
>                 iflag = bootm_disable_interrupts();
> diff --git a/cmd/bootm.c b/cmd/bootm.c
> index 37c2af96e0..0c4a713e02 100644
> --- a/cmd/bootm.c
> +++ b/cmd/bootm.c
> @@ -161,6 +161,8 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>                 BOOTM_STATE_OS_GO;
>         if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
>                 states |= BOOTM_STATE_RAMDISK;
> +       if (IS_ENABLED(CONFIG_MEASURED_BOOT))
> +               states |= BOOTM_STATE_MEASURE;
>         if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS))
>                 states |= BOOTM_STATE_OS_CMDLINE;
>         ret = do_bootm_states(cmdtp, flag, argc, argv, states, &images, 1);
> diff --git a/common/Kconfig b/common/Kconfig
> index 21434c5cf1..57ef68e4f3 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -799,6 +799,12 @@ config AVB_BUF_SIZE
>
>  endif # AVB_VERIFY
>
> +config MEASURED_BOOT
> +       bool "Measure the boot to TPM and event log"
> +       depends on HASH && TPM_V2
> +       help
> +         This option enables measurement of the boot process.

Please add proper help. This will also come up as a warning if you use patman

> +
>  config SCP03
>         bool "Build SCP03 - Secure Channel Protocol O3 - controls"
>         depends on OPTEE || SANDBOX
> diff --git a/include/image.h b/include/image.h
> index 6f21dafba8..b00803eeac 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -406,6 +406,7 @@ struct bootm_headers {
>  #define BOOTM_STATE_OS_FAKE_GO 0x00000200      /* 'Almost' run the OS */
>  #define BOOTM_STATE_OS_GO      0x00000400
>  #define BOOTM_STATE_PRE_LOAD   0x00000800
> +#define BOOTM_STATE_MEASURE    0x00001000
>         int             state;
>
>  #if defined(CONFIG_LMB) && !defined(USE_HOSTCC)
> --
> 2.31.1
>

Regards,
Simon