Re: [PATCH v4 5/6] test: Add sandbox TPM boot measurement

Simon Glass
2023-01-26T01:43:00Z

Hi Eddie,

On Wed, 25 Jan 2023 at 10:18, Eddie James <eajames@linux.ibm.com> wrote:
>
> Use the sandbox TPM driver to measure some boot images in a unit
> test case.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> arch/sandbox/dts/sandbox.dtsi | 14 ++++++++
> arch/sandbox/dts/test.dts | 13 +++++++
> configs/sandbox_defconfig | 1 +
> include/test/suites.h | 1 +
> test/boot/Makefile | 1 +
> test/boot/measurement.c | 66 +++++++++++++++++++++++++++++++++++
> test/cmd_ut.c | 2 ++
> 7 files changed, 98 insertions(+)
> create mode 100644 test/boot/measurement.c

Reviewed-by: Simon Glass <sjg@chromium.org>

Please see below

>
> diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
> index 18bf1cb5b6..3f0e192a83 100644
> --- a/arch/sandbox/dts/sandbox.dtsi
> +++ b/arch/sandbox/dts/sandbox.dtsi
> @@ -4,9 +4,22 @@
> * and sandbox64 builds.
> */
>
> +#include <config.h>
> +
> #define USB_CLASS_HUB 9
>
> / {
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + event_log: tcg_event_log {
> + no-map;
> + reg = <(CFG_SYS_SDRAM_SIZE - 0x2000) 0x2000>;
> + };
> + };
> +
> binman {
> };
>
> @@ -332,6 +345,7 @@
>
> tpm2 {
> compatible = "sandbox,tpm2";
> + memory-region = <&event_log>;
> };
>
> triangle {
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 9d96e479ca..c334b89faa 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -9,6 +9,7 @@
>
> /dts-v1/;
>
> +#include <config.h>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/gpio/sandbox-gpio.h>
> #include <dt-bindings/input/input.h>
> @@ -66,6 +67,17 @@
> osd0 = "/osd";
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + event_log: tcg_event_log {
> + no-map;
> + reg = <(CFG_SYS_SDRAM_SIZE - 0x2000) 0x2000>;
> + };
> + };
> +
> binman: binman {
> };
>
> @@ -1343,6 +1355,7 @@
>
> tpm2 {
> compatible = "sandbox,tpm2";
> + memory-region = <&event_log>;
> };
>
> uart0: serial {
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 34c342b6f5..9c4985adcf 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -337,3 +337,4 @@ CONFIG_TEST_FDTDEC=y
> CONFIG_UNIT_TEST=y
> CONFIG_UT_TIME=y
> CONFIG_UT_DM=y
> +CONFIG_MEASURED_BOOT=y
> diff --git a/include/test/suites.h b/include/test/suites.h
> index 9ce49cbb03..4c284bbeaa 100644
> --- a/include/test/suites.h
> +++ b/include/test/suites.h
> @@ -44,6 +44,7 @@ int do_ut_font(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> int do_ut_lib(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> int do_ut_loadm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> int do_ut_log(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
> +int do_ut_measurement(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
> int do_ut_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> int do_ut_optee(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> int do_ut_overlay(struct cmd_tbl *cmdtp, int flag, int argc,
> diff --git a/test/boot/Makefile b/test/boot/Makefile
> index 22ed61c8fa..2dbb032a7e 100644
> --- a/test/boot/Makefile
> +++ b/test/boot/Makefile
> @@ -4,6 +4,7 @@
>
> obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o bootmeth.o
> obj-$(CONFIG_FIT) += image.o
> +obj-$(CONFIG_MEASURED_BOOT) += measurement.o
>
> obj-$(CONFIG_EXPO) += expo.o
>
> diff --git a/test/boot/measurement.c b/test/boot/measurement.c
> new file mode 100644
> index 0000000000..2155208894
> --- /dev/null
> +++ b/test/boot/measurement.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Test for measured boot functions
> + *
> + * Copyright 2023 IBM Corp.
> + * Written by Eddie James <eajames@linux.ibm.com>
> + */
> +
> +#include <common.h>
> +#include <bootm.h>
> +#include <malloc.h>
> +#include <test/suites.h>
> +#include <test/test.h>
> +#include <test/ut.h>
> +#include <asm/io.h>
> +
> +#define MEASUREMENT_TEST(_name, _flags) \
> + UNIT_TEST(_name, _flags, measurement_test)
> +
> +static int measure(struct unit_test_state *uts)
> +{
> + struct bootm_headers images;
> + const size_t size = 1024;
> + u8 *kernel;
> + u8 *initrd;
> + size_t i;
> +
> + kernel = malloc(size);
> + initrd = malloc(size);
> +
> + images.os.image_start = map_to_sysmem(kernel);
> + images.os.image_len = size;
> +
> + images.rd_start = map_to_sysmem(initrd);
> + images.rd_end = images.rd_start + size;
> +
> + images.ft_addr = malloc(size);

> + images.ft_len = size;
> +
> + env_set("bootargs", "measurement testing");
> +
> + for (i = 0; i < size; ++i) {
> + kernel[i] = (u8)(0xf0 | (i & 0xf));
> + initrd[i] = (u8)((i & 0xf0) | 0xf);
> + ((u8 *)images.ft_addr)[i] = (u8)(i & 0xff);

Gosh this is pretty ugly with all the casting (which I suspect could
be dropped).

At least, can you drop the cast on images.ft_addr? If you don't want
to do that, please create a u8 * local variable.

> + }
> +
> + ut_assertok(bootm_measure(&images));
> +
> + free(images.ft_addr);
> + free(initrd);
> + free(kernel);
> +
> + return 0;
> +}
> +MEASUREMENT_TEST(measure, 0);
> +
> +int do_ut_measurement(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + struct unit_test *tests = UNIT_TEST_SUITE_START(measurement_test);
> + const int n_ents = UNIT_TEST_SUITE_COUNT(measurement_test);
> +
> + return cmd_ut_category("measurement", "measurement_test_", tests,
> + n_ents, argc, argv);
> +}
> diff --git a/test/cmd_ut.c b/test/cmd_ut.c
> index 1713d0d1c8..2bd020cf74 100644
> --- a/test/cmd_ut.c
> +++ b/test/cmd_ut.c
> @@ -101,6 +101,8 @@ static struct cmd_tbl cmd_ut_sub[] = {
> "", ""),
> U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist,
> "", ""),
> + U_BOOT_CMD_MKENT(measurement, CONFIG_SYS_MAXARGS, 1, do_ut_measurement,
> + "", ""),
> U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""),
> #endif
> U_BOOT_CMD_MKENT(str, CONFIG_SYS_MAXARGS, 1, do_ut_str, "", ""),
> --
> 2.31.1
>

Regards,
Simon