On Wed, 2021-02-03 at 15:59 -0800, Sean Christopherson wrote: > On Thu, Feb 04, 2021, Kai Huang wrote: > > On Wed, 2021-02-03 at 15:36 -0800, Sean Christopherson wrote: > > > On Thu, Feb 04, 2021, Kai Huang wrote: > > > > On Wed, 2021-02-03 at 11:36 -0800, Sean Christopherson wrote: > > > > > On Wed, Feb 03, 2021, Edgecombe, Rick P wrote: > > > > > > On Tue, 2021-01-26 at 22:31 +1300, Kai Huang wrote: > > > > > > Don't you need to deep copy the pageinfo.contents struct as well? > > > > > > Otherwise the guest could change these after they were checked. > > > > > > > > > > > > But it seems it is checked by the HW and something is caught that would > > > > > > inject a GP anyway? Can you elaborate on the importance of these > > > > > > checks? > > > > > > > > > > Argh, yes. These checks are to allow migration between systems with different > > > > > SGX capabilities, and more importantly to prevent userspace from doing an end > > > > > around on the restricted access to PROVISIONKEY. > > > > > > > > > > IIRC, earlier versions did do a deep copy, but then I got clever. Anyways, yeah, > > > > > sadly the entire pageinfo.contents page will need to be copied. > > > > > > > > I don't fully understand the problem. Are you worried about contents being updated by > > > > other vcpus during the trap? > > > > > > > > And I don't see how copy can avoid this problem. Even you do copy, the content can > > > > still be modified afterwards, correct? So what's the point of copying? > > > > > > The goal isn't correctness, it's to prevent a TOCTOU bug. E.g. the guest could > > > do ECREATE w/ SECS.SGX_ATTR_PROVISIONKEY=0, and simultaneously set > > > SGX_ATTR_PROVISIONKEY to bypass the above check. > > > > Oh ok. Agreed. > > > > However, such attack would require precise timing. Not sure whether it is feasible in > > practice. > > It's very feasible. XOR the bit in a tight loop, build the enclave on a > separate thread. Do that until EINIT succeeds. Compared to other timing > attacks, I doubt it'd take all that long to get a successful result. How does it work? The setting PROVISION bit needs to be set after KVM checks SECS's attribute, and before KVM actually does ECREATE, right? > > Regardless, the difficulty in exploiting the bug is irrelevant, it's a glaring > flaw that needs to be fixed. Sure.
