linux-kernel - Re: [PATCH] x86/tdx: Handle load_unaligned

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: <4ee65514-2b08-9ca8-84e8-92a6ab1d9652@intel.com>
Date:   Tue, 17 May 2022 15:52:03 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        luto@...nel.org, peterz@...radead.org,
        sathyanarayanan.kuppuswamy@...ux.intel.com, ak@...ux.intel.com,
        dan.j.williams@...el.com, david@...hat.com, hpa@...or.com,
        thomas.lendacky@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/tdx: Handle load_unaligned_zeropad() page-cross to a
 shared page

On 5/17/22 15:40, Sean Christopherson wrote:
> On Tue, May 17, 2022, Dave Hansen wrote:
>> The SDM makes it sound like we should be more judicious about using
>> 've->instr_len' though.  "All VM exits other than those listed in the
>> above items leave this field undefined."  Looking over
>> virt_exception_kernel(), we've got five cases from CPU instructions that
>> cause unconditional VMEXITs:
> 
> None of the below exit unconditionally.
...
>   For fault-like VM exits due to attempts to execute one of the following
>   instructions that cause VM exits unconditionally or based on the settings of
>   VM-execution controls.

Ahh, got it, thanks.  I bailed on reading that sentence before I got to
the "VM-execution controls" bit.

>> Then handle_mmio() can say:
>>
>> 	/*
>> 	 * VM-exit instruction length is not provided for the EPT
>> 	 * violations that MMIO causes.  Use the insn_decode() length:
> 
> This is inaccurate.  The instruction length _is_ provided on EPT Violation VM-Exits
> (it's also provided by all Intel CPUs on EPT Misconfigs even though the SDM doesn't
> say so).
> 
> The instruction length is wrong in the TDX case because there is no EPT Violation
> VM-Exit.  The EPT Violation is morphed to a #VE by the CPU, and the instruction
> length isn't one of the fields that's saved into the #VE info struct by the CPU.
> When the TDX Module gets control on the TDCALL, VMCS.INSTRUCTION_LENGTH will hold
> the length of the TDCALL, not the instruction that caused the #VE, i.e. the TDX
> Module can't provide the correct length.
> 
> For all other #VE cases in TDX, the #VE is injected by software (TDX module) after
> the instruction-based VM-Exit.  Before injecting the #VE, the TDX module grabs the
> length from the VMCS and manually records it in the #VE info struct.

That's horribly entertaining background. :)

But, it doesn't get us much closer to deciding when we can consume the
instruction length out of the ve_info.  It seems like magic that jut
happens to work at the moment.