linux-kernel - Re: [PATCH v17 1/3] x86/tdx: Add a wrapper to get TDREPORT from the TDX Module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2022 10:35:54 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Shuah Khan <shuah@...nel.org>, Jonathan Corbet <corbet@....net>
Cc:     "H . Peter Anvin" <hpa@...or.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Kai Huang <kai.huang@...el.com>,
        Wander Lairson Costa <wander@...hat.com>,
        Isaku Yamahata <isaku.yamahata@...il.com>,
        marcelo.cerri@...onical.com, tim.gardner@...onical.com,
        khalid.elmously@...onical.com, philip.cox@...onical.com,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH v17 1/3] x86/tdx: Add a wrapper to get TDREPORT from the
 TDX Module

On 11/3/22 20:23, Kuppuswamy Sathyanarayanan wrote:
> To support TDX attestation, the TDX guest driver exposes an IOCTL
> interface to allow userspace to get the TDREPORT from the TDX module
> via TDG.MR.TDREPORT TDCALL.

This all acts and is named like this is *THE* way to do a TD report.
This is the only type of TD report.

Is it?

If so, why is there a subtype in the TDX module ABI?  It's easy to miss
in the kernel code, btw:

> +int tdx_mcall_get_report(u8 *reportdata, u8 *tdreport)
> +{
> +	u64 ret;
> +
> +	ret = __tdx_module_call(TDX_GET_REPORT, virt_to_phys(tdreport),
> +				virt_to_phys(reportdata), 0, 0, NULL);

					     subtype here ^

mixed in next to another magic 0.

> +	if (ret) {
> +		if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
> +			return -EINVAL;
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tdx_mcall_get_report);

What happens to this interface when subtype 1 is added?

TDX_CMD_GET_REPORT can only get subtype 0.  So, we'll have, what, a new
ioctl()?  TDX_CMD_GET_REPORT_SUBTYPE1?

This is why I was pushing for a more generic ABI that would actually
work for more than one subtype.  Other folks thought that was a bad
idea.  I can live with that.  But, what I can't live with is just
pretending that this is the one and only forever "tdreport" interface.

This is *NOT* "a wrapper to get TDREPORT from the TDX Module", this is
at best "a wrapper to get TDREPORT sub type 0 from the TDX Module".

It also occurs to me that "sub type 0" could use an actual name.  Could
we give it one, please?