From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AA50AC433FE for ; Tue, 1 Nov 2022 16:54:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230397AbiKAQya (ORCPT ); Tue, 1 Nov 2022 12:54:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53864 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231209AbiKAQyV (ORCPT ); Tue, 1 Nov 2022 12:54:21 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 39BA2B03 for ; Tue, 1 Nov 2022 09:53:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667321600; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VlbZG9ub6DF/LY2VtLFrDY1eKkkq4JGcfuS3h450tTI=; b=Lnsvyiruor4rGBNls8aTQBPmXM1lpE+VEBlKH8Zujp0UfIEPg7FeaLXGfxw90pY1mAqxQb TnmZwQqk2YRvA42VcRc/QleWDQxezi4O/EY0TW+6tSI9HH221VANAFhu/d7OcfvqwCSD7u kIjGtV41nLDtDhNY62y1lJPD2MRHaSM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-622-AyImlOReMvids-H-Ejt9cQ-1; Tue, 01 Nov 2022 12:53:16 -0400 X-MC-Unique: AyImlOReMvids-H-Ejt9cQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5E87F85A5A6; Tue, 1 Nov 2022 16:53:15 +0000 (UTC) Received: from fedora (unknown [10.22.32.201]) by smtp.corp.redhat.com (Postfix) with SMTP id A6E492166B2D; Tue, 1 Nov 2022 16:53:11 +0000 (UTC) Date: Tue, 1 Nov 2022 13:53:09 -0300 From: Wander Lairson Costa To: Sathyanarayanan Kuppuswamy Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Shuah Khan , Jonathan Corbet , "H . Peter Anvin" , Greg Kroah-Hartman , "Kirill A . Shutemov" , Tony Luck , Kai Huang , Isaku Yamahata , marcelo.cerri@canonical.com, tim.gardner@canonical.com, khalid.elmously@canonical.com, philip.cox@canonical.com, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org Subject: Re: [PATCH v16 3/3] selftests: tdx: Test TDX attestation GetReport support Message-ID: <20221101165309.uihcsbjiejkrkiov@fedora> References: <20221028002820.3303030-1-sathyanarayanan.kuppuswamy@linux.intel.com> <20221028002820.3303030-4-sathyanarayanan.kuppuswamy@linux.intel.com> <20221101133556.l7dffiwl24lqjioz@fedora> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 01, 2022 at 06:43:31AM -0700, Sathyanarayanan Kuppuswamy wrote: > > > 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 > >> Reviewed-by: Mika Westerberg > >> Acked-by: Kai Huang > >> Acked-by: Kirill A. Shutemov > >> Signed-off-by: Kuppuswamy Sathyanarayanan > >> --- > >> > >> 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@linux.intel.com/ > >> > >> 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 > >> + */ > >> + > >> +#include > >> + > >> +#include > >> +#include > >> + > >> +#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. Honestly, by design the code never enters in the `if` condition. So I am in favor of removing it. > > > > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer >