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

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]

Message-ID: <YV8vN3SniVshnaHu@zn.tnic>
Date:   Thu, 7 Oct 2021 19:32:39 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Kuppuswamy, Sathyanarayanan" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Juergen Gross <jgross@...e.com>, Deep Shah <sdeep@...are.com>,
        VMware Inc <pv-drivers@...are.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, Peter H Anvin <hpa@...or.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 06/11] x86/traps: Add #VE support for TDX guest

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