linux-kernel - Re: [PATCH v6 1/5] x86/tdx: Add TDX Guest attestation interface driver

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 for Android: free password hash cracker in your pocket

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

Date:   Sun, 22 May 2022 20:41:48 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Kai Huang <kai.huang@...el.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
Cc:     "H . Peter Anvin" <hpa@...or.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Andi Kleen <ak@...ux.intel.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
Subject: Re: [PATCH v6 1/5] x86/tdx: Add TDX Guest attestation interface
 driver



On 5/22/22 7:52 PM, Kai Huang wrote:
> On Tue, 2022-05-17 at 07:54 -0700, Sathyanarayanan Kuppuswamy wrote:
>>>> +struct tdx_report_req {
>>>> +	union {
>>>> +		__u8 reportdata[TDX_REPORTDATA_LEN];
>>>> +		__u8 tdreport[TDX_REPORT_LEN];
>>>> +	};
>>>> +};
>>>
>>> As a userspace ABI, one concern is this doesn't provide any space for future
>>> extension.  But probably it's OK since I don't see any possible additional
>>> input
>>> for now.  And although TDREPORT may have additional information in future
>>> generation of TDX but the spec says the size is 1024 so perhaps this won't
>>> change even in the future.
>>>
>>> Anyway will leave to others.
>>
>> IMO, if the spec changes in future we can revisit it.
> 
> I don't think the problem is how to revisit _this_ ABI.  The problem is, once it
> is introduced, you cannot break the ABI for the compatibility of supporting the
> userspace software written for old platforms.  So basically you cannot just
> increase the TDX_REPORT_LEN to a larger value.  This means if we have a larger
> than 1024B TDREPORT in future, the old userspace TD attestation software which
> uses this ABI will not work anymore on the new platforms.
> 
> If we need to make sure this ABI work for _ANY_ TDX platforms, I think we either
> need to make sure TDREPORT will always be 1024B for _ANY_ TDX platforms, or we
> need to have a flexible ABI which doesn't assume TDREPORT size.
> 
> For instance, we might need another IOCTL (or other interfaces such as /sysfs)
> to query the TDREPORT size, and make this IOCTL like below:
> 
> 	struct tdx_report_req {
> 		__u8 reportdata[TDX_REPORTDATA_LEN];
> 		__u8 reserved[...];
> 		__u8 tdreport[0];
> 	};
> 
> The actual TDREPORT buffer size is allocated by userspace after it queries the
> TDREPORT size.

I don't want to over design it just based on the assumption that length
will change in the future. I don't see any statement in spec supporting
the possibility of length changes. IMO, since the possibility is very
small, we don't need to overthink about it.

@maintainers, please let me know if you think otherwise.
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer