linux-kernel - Re: [PATCH v3 00/21] Enable CET Virtualization

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: <ZIufL7p/ZvxjXwK5@google.com>
Date:   Thu, 15 Jun 2023 16:30:55 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Yang Weijiang <weijiang.yang@...el.com>
Cc:     pbonzini@...hat.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, peterz@...radead.org,
        rppt@...nel.org, binbin.wu@...ux.intel.com,
        rick.p.edgecombe@...el.com, john.allen@....com
Subject: Re: [PATCH v3 00/21] Enable CET Virtualization

On Thu, May 11, 2023, Yang Weijiang wrote:
> The last patch is introduced to support supervisor SHSTK but the feature is
> not enabled on Intel platform for now, the main purpose of this patch is to
> facilitate AMD folks to enable the feature.

I am beyond confused by the SDM's wording of CET_SSS.

First, it says that CET_SSS says the CPU isn't buggy (or maybe "less buggy" is
more appropriate phrasing).

  Bit 18: CET_SSS. If 1, indicates that an operating system can enable supervisor
  shadow stacks as long as it ensures that certain supervisor shadow-stack pushes
  will not cause page faults (see Section 17.2.3 of the Intel® 64 and IA-32
  Architectures Software Developer’s Manual, Volume 1).

But then it says says VMMs shouldn't set the bit.

  When emulating the CPUID instruction, a virtual-machine monitor should return
  this bit as 0 if those pushes can cause VM exits.

Based on the Xen code (which is sadly a far better source of information than the
SDM), I *think* that what the SDM is trying to say is that VMMs should not set
CET_SS if VM-Exits can occur ***and*** the bit is not set in the host CPU.  Because
if the SDM really means "VMMs should never set the bit", then what on earth is the
point of the bit.

> In summary, this new series enables CET user SHSTK/IBT and kernel IBT, but
> doesn't fully support CET supervisor SHSTK, the enabling work is left for
> the future.

Why?  If my interpretation of the SDM is correct, then all the pieces are there.

> Executed all KVM-unit-test cases and KVM selftests against this series, all
> test cases passed except the vmx test, the failure is due to CR4_CET bit
> testing in test_vmxon_bad_cr(). After add CR4_CET bit to skip list, the test
> passed. I'll send a patch to fix this issue later.

Your cover letter from v2 back in April said the same thing.  Why hasn't the patch
been posted?  And what exactly is the issue?  IIUC, setting CR4.CET with
MSR_IA32_S_CET=0 and MSR_IA32_U_CET=0 should be a nop, which suggests that there's
a KVM bug.  And if that's the case, the next obvious questions is, why are you
posting known buggy code?