linux-kernel - Re: [PATCH 01/10] x86/tdx: Zero out the missing RSI in TDX

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c29cb23b-d14c-4449-2881-5b2d3f9e7a74@linux.intel.com>
Date:   Wed, 12 Jul 2023 12:19:12 -0700
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Kai Huang <kai.huang@...el.com>, peterz@...radead.org,
        kirill.shutemov@...ux.intel.com, linux-kernel@...r.kernel.org
Cc:     dave.hansen@...el.com, tglx@...utronix.de, bp@...en8.de,
        mingo@...hat.com, hpa@...or.com, x86@...nel.org, seanjc@...gle.com,
        pbonzini@...hat.com, kvm@...r.kernel.org, isaku.yamahata@...el.com
Subject: Re: [PATCH 01/10] x86/tdx: Zero out the missing RSI in TDX_HYPERCALL
 macro



On 7/12/23 1:55 AM, Kai Huang wrote:
> In the TDX_HYPERCALL asm, after the TDCALL instruction returns from the
> untrusted VMM, the registers that the TDX guest shares to the VMM need
> to be cleared to avoid speculative execution of VMM-provided values.
> 
> RSI is specified in the bitmap of those registers, but it is missing
> when zeroing out those registers in the current TDX_HYPERCALL.
> 
> It was there when it was originally added in commit 752d13305c78
> ("x86/tdx: Expand __tdx_hypercall() to handle more arguments"), but was
> later removed in commit 1e70c680375a ("x86/tdx: Do not corrupt
> frame-pointer in __tdx_hypercall()"), which was correct because %rsi is
> later restored in the "pop %rsi".  However a later commit 7a3a401874be
> ("x86/tdx: Drop flags from __tdx_hypercall()") removed that "pop %rsi"
> but forgot to add the "xor %rsi, %rsi" back.
> 
> Fix by adding it back.
> 
> Fixes: 7a3a401874be ("x86/tdx: Drop flags from __tdx_hypercall()")
> Signed-off-by: Kai Huang <kai.huang@...el.com>
> ---

Looks fine to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>

>  arch/x86/coco/tdx/tdcall.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/coco/tdx/tdcall.S b/arch/x86/coco/tdx/tdcall.S
> index b193c0a1d8db..2eca5f43734f 100644
> --- a/arch/x86/coco/tdx/tdcall.S
> +++ b/arch/x86/coco/tdx/tdcall.S
> @@ -195,6 +195,7 @@ SYM_FUNC_END(__tdx_module_call)
>  	xor %r10d, %r10d
>  	xor %r11d, %r11d
>  	xor %rdi,  %rdi
> +	xor %rsi,  %rsi
>  	xor %rdx,  %rdx
>  
>  	/* Restore callee-saved GPRs as mandated by the x86_64 ABI */

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer