Re: [PATCHv6 07/30] x86/traps: Add #VE support for TDX guest

Thomas Gleixner
2022-03-17T18:18:24Z

On Thu, Mar 17 2022 at 20:33, Kirill A. Shutemov wrote:
> On Thu, Mar 17, 2022 at 01:48:54AM +0100, Thomas Gleixner wrote:
>> On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
>> Hmm?
>
> Does the changed version below address your concerns?
>
> void tdx_get_ve_info(struct ve_info *ve)
> {
> struct tdx_module_output out;
>
> /*
> * Called during #VE handling to retrieve the #VE info from the
> * TDX module.
> *
> * This has to be called early in #VE handling. A "nested" #VE which
> * occurs before this will raise a #DF and is not recoverable.
> *
> * The call retrieves the #VE info from the TDX module, which also
> * clears the "#VE valid" flag. This must be done before anything else
> * because any #VE that occurs while the valid flag is set will lead to
> * #DF.
> *
> * Note, the TDX module treats virtual NMIs as inhibited if the #VE
> * valid flag is set. It means that NMI=>#VE will not result in a #DF.
> */
> tdx_module_call(TDX_GET_VEINFO, 0, 0, 0, 0, &out);
>
> /* Transfer the output parameters */
> ve->exit_reason = out.rcx;
> ve->exit_qual = out.rdx;
> ve->gla = out.r8;
> ve->gpa = out.r9;
> ve->instr_len = lower_32_bits(out.r10);
> ve->instr_info = upper_32_bits(out.r10);
> }

Nice.

>> The point is that any #VE in such a code path is fatal and you better
>> come up with some reasonable explanation why this is not the case in
>> those code pathes and how a potential violation of that assumption might
>> be detected especially in rarely used corner cases. If such a violation
>> is not detectable by audit, CI, static code analysis or whatever then
>> document the consequences instead of pretending that the problem does
>> not exist and the kernel is perfect today and forever.
>
> It is detectable by audit. The critical windows very limited and located
> in the highly scrutinized entry code. But, yes, I cannot guarantee that
> this code will be perfect forever.

Fair enough.

> Consequences of #VE in these critical windows are mentioned in the
> comment:
>
> Any exception in this window leads to hard to debug issues and can
> be exploited for privilege escalation.
>
> I have hard time understanding what I has to change here. Do you want
> details of audit to be documented? Make consequences of #VE at the wrong
> point to be more prominent in the comment?

So having something like this in the comment would be helpful:

*
* The entry code has been audited carefuly for following these
* expectations. Changes in the entry code have to be audited for
* correctness vs. this aspect. #VE in these places will cause
* [an instant kernel panic | whatever | fill the blanks ]
*

Thanks,

tglx