Re: [PATCH v1] virt: tdx-guest: Handle GetQuote request error code

[Kirill A . Shutemov]

On Tue, Jan 09, 2024 at 07:56:56PM -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> 
> On 1/9/2024 5:17 AM, Kirill A . Shutemov wrote:
> > On Tue, Jan 09, 2024 at 05:46:04AM +0000, Kuppuswamy Sathyanarayanan wrote:
> >> Currently when a user requests for the Quote generation, the Quote
> >> generation handler (tdx_report_new()) only checks whether the VMM
> >> successfully processes the Quote generation request (status !=
> >> GET_QUOTE_IN_FLIGHT) and returns the output to the user without
> >> validating the status of the output data. Since VMM can return error
> >> even after processing the Quote request, returning success just after
> >> successful processing will create confusion to the user. Although for
> >> the failed request, output buffer length will be zero and can also be
> >> used by the user to identify the failure case, it will be more clear to
> >> return error for all failed cases. So validate the Quote output status
> >> and return error code for all failed cases.
> > 
> > Could you split commit message into several paragraphs? It would be easier
> > to get along.
> > 
> > It can be helpful to follow structure like:
> > 
> > <Background>
> > 
> > <Problem>
> > 
> > <Solution>
> > 
> 
> How about the following version?
> 
> During the TDX guest attestation process, TSM ConfigFS ABI is used by
> the user attestation agent to get the signed VM measurement data (a.k.a
> Quote), which can be used by a remote verifier to validate the
> trustworthiness of the guest. When a user requests for the Quote data
> via the ConfigFS ABI, the TDX Quote generation handler
> (tdx_report_new()) forwards the request to VMM (or QE) via a hypercall,
> and then shares the output with the user.
> 
> Currently, when handling the Quote generation request, tdx_report_new()
> handler only checks whether the VMM successfully processed the request
> and if it is true it returns success and shares the output to the user
> without actually validating the output data. Since the VMM can return
> error even after processing the Quote request, always returning success
> for the processed requests is incorrect and will create confusion to
> the user. Although for the failed request, output buffer length will
> be zero and can also be used by the user to identify the failure case,
> it will be more clear to return error for all failed cases.
> 
> So when handling the Quote generation request, validate the Quote data
> output status and return error code for all failed cases.

I would drop the start of the sentence upto ',' here, but otherwise looks
good to me.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov