On 11/1/22 6:35 AM, Wander Lairson Costa wrote: > On Thu, Oct 27, 2022 at 05:28:20PM -0700, Kuppuswamy Sathyanarayanan wrote: >> Attestation is used to verify the trustworthiness of a TDX guest. >> During the guest bring-up, the Intel TDX module measures and records >> the initial contents and configuration of the guest, and at runtime, >> guest software uses runtime measurement registers (RMTRs) to measure >> and record details related to kernel image, command line params, ACPI >> tables, initrd, etc. At guest runtime, the attestation process is used >> to attest to these measurements. >> >> The first step in the TDX attestation process is to get the TDREPORT >> data. It is a fixed size data structure generated by the TDX module >> which includes the above mentioned measurements data, a MAC ID to >> protect the integrity of the TDREPORT, and a 64-Byte of user specified >> data passed during TDREPORT request which can uniquely identify the >> TDREPORT. >> >> Intel's TDX guest driver exposes TDX_CMD_GET_REPORT IOCTL interface to >> enable guest userspace to get the TDREPORT. >> >> Add a kernel self test module to test this ABI and verify the validity >> of the generated TDREPORT. >> >> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx> >> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> >> Acked-by: Kai Huang <kai.huang@xxxxxxxxx> >> Acked-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx> >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> >> --- >> >> Changes since v15: >> * None >> >> Changes since v14: >> * Fixed format issue in struct comments. >> * Rebased on top of v6.1-rc1 >> >> Changes since v13: >> * Removed __packed from TDREPORT structs. >> * Since the guest driver is moved to drivers/virt/coco, removed >> tools/arch/x86/include header folder usage. >> * Fixed struct comments to match kernel-doc format. >> * Fixed commit log as per review comments. >> * Fixed some format issues in the code. >> >> Changes since v12: >> * Changed #ifdef DEBUG usage with if (DEBUG). >> * Initialized reserved entries values to zero. >> >> Changes since v11: >> * Renamed devname with TDX_GUEST_DEVNAME. >> >> Changes since v10: >> * Replaced TD/TD Guest usage with guest or TDX guest. >> * Reworded the subject line. >> >> Changes since v9: >> * Copied arch/x86/include/uapi/asm/tdx.h to tools/arch/x86/include to >> decouple header dependency between kernel source and tools dir. >> * Fixed Makefile to adapt to above change. >> * Fixed commit log and comments. >> * Added __packed to hardware structs. >> >> Changes since v8: >> * Please refer to https://lore.kernel.org/all/ \ >> 20220728034420.648314-1-sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx/ >> >> tools/testing/selftests/Makefile | 1 + >> tools/testing/selftests/tdx/Makefile | 7 + >> tools/testing/selftests/tdx/config | 1 + >> tools/testing/selftests/tdx/tdx_guest_test.c | 175 +++++++++++++++++++ >> 4 files changed, 184 insertions(+) >> create mode 100644 tools/testing/selftests/tdx/Makefile >> create mode 100644 tools/testing/selftests/tdx/config >> create mode 100644 tools/testing/selftests/tdx/tdx_guest_test.c >> >> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile >> index 0464b2c6c1e4..f60e14d16bfd 100644 >> --- a/tools/testing/selftests/Makefile >> +++ b/tools/testing/selftests/Makefile >> @@ -73,6 +73,7 @@ TARGETS += sync >> TARGETS += syscall_user_dispatch >> TARGETS += sysctl >> TARGETS += tc-testing >> +TARGETS += tdx >> TARGETS += timens >> ifneq (1, $(quicktest)) >> TARGETS += timers >> diff --git a/tools/testing/selftests/tdx/Makefile b/tools/testing/selftests/tdx/Makefile >> new file mode 100644 >> index 000000000000..8dd43517cd55 >> --- /dev/null >> +++ b/tools/testing/selftests/tdx/Makefile >> @@ -0,0 +1,7 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> + >> +CFLAGS += -O3 -Wl,-no-as-needed -Wall -static >> + >> +TEST_GEN_PROGS := tdx_guest_test >> + >> +include ../lib.mk >> diff --git a/tools/testing/selftests/tdx/config b/tools/testing/selftests/tdx/config >> new file mode 100644 >> index 000000000000..aa1edc829ab6 >> --- /dev/null >> +++ b/tools/testing/selftests/tdx/config >> @@ -0,0 +1 @@ >> +CONFIG_TDX_GUEST_DRIVER=y >> diff --git a/tools/testing/selftests/tdx/tdx_guest_test.c b/tools/testing/selftests/tdx/tdx_guest_test.c >> new file mode 100644 >> index 000000000000..a5c243f73adc >> --- /dev/null >> +++ b/tools/testing/selftests/tdx/tdx_guest_test.c >> @@ -0,0 +1,175 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Test TDX guest features >> + * >> + * Copyright (C) 2022 Intel Corporation. >> + * >> + * Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> >> + */ >> + >> +#include <sys/ioctl.h> >> + >> +#include <errno.h> >> +#include <fcntl.h> >> + >> +#include "../kselftest_harness.h" >> +#include "../../../../include/uapi/linux/tdx-guest.h" >> + >> +#define TDX_GUEST_DEVNAME "/dev/tdx_guest" >> +#define HEX_DUMP_SIZE 8 >> +#define DEBUG 0 >> + >> +/** >> + * struct tdreport_type - Type header of TDREPORT_STRUCT. >> + * @type: Type of the TDREPORT (0 - SGX, 81 - TDX, rest are reserved) >> + * @sub_type: Subtype of the TDREPORT (Default value is 0). >> + * @version: TDREPORT version (Default value is 0). >> + * @reserved: Added for future extension. >> + * >> + * More details can be found in TDX v1.0 module specification, sec >> + * titled "REPORTTYPE". >> + */ >> +struct tdreport_type { >> + __u8 type; >> + __u8 sub_type; >> + __u8 version; >> + __u8 reserved; >> +}; >> + >> +/** >> + * struct reportmac - TDX guest report data, MAC and TEE hashes. >> + * @type: TDREPORT type header. >> + * @reserved1: Reserved for future extension. >> + * @cpu_svn: CPU security version. >> + * @tee_tcb_info_hash: SHA384 hash of TEE TCB INFO. >> + * @tee_td_info_hash: SHA384 hash of TDINFO_STRUCT. >> + * @reportdata: User defined unique data passed in TDG.MR.REPORT request. >> + * @reserved2: Reserved for future extension. >> + * @mac: CPU MAC ID. >> + * >> + * It is MAC-protected and contains hashes of the remainder of the >> + * report structure along with user provided report data. More details can >> + * be found in TDX v1.0 Module specification, sec titled "REPORTMACSTRUCT" >> + */ >> +struct reportmac { >> + struct tdreport_type type; >> + __u8 reserved1[12]; >> + __u8 cpu_svn[16]; >> + __u8 tee_tcb_info_hash[48]; >> + __u8 tee_td_info_hash[48]; >> + __u8 reportdata[64]; >> + __u8 reserved2[32]; >> + __u8 mac[32]; >> +}; >> + >> +/** >> + * struct td_info - TDX guest measurements and configuration. >> + * @attr: TDX Guest attributes (like debug, spet_disable, etc). >> + * @xfam: Extended features allowed mask. >> + * @mrtd: Build time measurement register. >> + * @mrconfigid: Software-defined ID for non-owner-defined configuration >> + * of the guest - e.g., run-time or OS configuration. >> + * @mrowner: Software-defined ID for the guest owner. >> + * @mrownerconfig: Software-defined ID for owner-defined configuration of >> + * the guest - e.g., specific to the workload. >> + * @rtmr: Run time measurement registers. >> + * @reserved: Added for future extension. >> + * >> + * It contains the measurements and initial configuration of the TDX guest >> + * that was locked at initialization and a set of measurement registers >> + * that are run-time extendable. More details can be found in TDX v1.0 >> + * Module specification, sec titled "TDINFO_STRUCT". >> + */ >> +struct td_info { >> + __u8 attr[8]; >> + __u64 xfam; >> + __u64 mrtd[6]; >> + __u64 mrconfigid[6]; >> + __u64 mrowner[6]; >> + __u64 mrownerconfig[6]; >> + __u64 rtmr[24]; >> + __u64 reserved[14]; >> +}; >> + >> +/* >> + * struct tdreport - Output of TDCALL[TDG.MR.REPORT]. >> + * @reportmac: Mac protected header of size 256 bytes. >> + * @tee_tcb_info: Additional attestable elements in the TCB are not >> + * reflected in the reportmac. >> + * @reserved: Added for future extension. >> + * @tdinfo: Measurements and configuration data of size 512 bytes. >> + * >> + * More details can be found in TDX v1.0 Module specification, sec >> + * titled "TDREPORT_STRUCT". >> + */ >> +struct tdreport { >> + struct reportmac reportmac; >> + __u8 tee_tcb_info[239]; >> + __u8 reserved[17]; >> + struct td_info tdinfo; >> +}; >> + >> +static void print_array_hex(const char *title, const char *prefix_str, >> + const void *buf, int len) >> +{ >> + int i, j, line_len, rowsize = HEX_DUMP_SIZE; >> + const __u8 *ptr = buf; >> + >> + if (!len || !buf) > > If len is not zero and buf is null, this would be bug. It is better to > let the code crash or assert buf isn't null after testing len. It is just a debug dump routine. So I don't think you need to add assert or crash here. If you really want some indication, maybe we can add an error message for invalid params. > -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
