Re: [PATCH v8 06/11] x86/traps: Add #VE support for TDX guest

[Borislav Petkov <bp@alien8.de>]

On Thu, Oct 07, 2021 at 10:22:16AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> Now checking again, I think we don't to initialize the @out pointer. I will
> fix this in call cases.
> 
> But for tdx_get_ve_info() case, we are updating the @ve pointer without
> checking the tdcall return value and __tdx_module_call() will update the
> @out only if tdcall is successful. Currently due to @out=0 initialization,
> this logic does not cause any issue. But to properly fix it, I need to
> check for tdcall return value before updating the @ve value.

Yes, that looked weird too but I assumed since you enforce @out to be
non-NULL, you know that you'll get ve-> populated.

However, yes, you need to check the retval for that other case:

        /* Check for TDCALL success: 0 - Successful, otherwise failed */
        test %rax, %rax
        jnz 1f


> Current use case of tdx_get_ve_info() can never be NULL. But if you
> want to add this check for possible future issues, I can do it.
> 
> arch/x86/kernel/traps.c:1205:	ret = tdx_get_ve_info(&ve);
> arch/x86/kernel/tdx.c:945:	if (tdx_get_ve_info(&ve))

Yes, you want to fail gracefully in the case where someone forgets to
pass in a proper pointer to tdx_get_ve_info() in the future. As it is
now, the guest would simply explode and that is not nice.

Also, that function doesn't need to return an unsigned long if all it
wants to return is a bool, from looking at its call sites.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette