On Fri, 2022-11-04 at 16:26 +0000, Sean Christopherson wrote: > On Fri, Nov 04, 2022, Huang, Kai wrote: > > > > In fact, to share a virtual EPC instance in userspace doesn't make any > > > > sense. Even though it can be shared by child, the virtual EPC page > > > > cannot be used by child correctly. > > > > > > OK, makes sense, thanks for the explanation! > > > > > > Why would we want to enforce for user space not to do this, even > > > if it does cause malfunctioning program? > > > > > > BR, Jarkko > > > > Hi Jarkko, Dave, > > > > I've been re-thinking about this #MC handle on virtual EPC by stepping back to > > the beginning, and I think we have more problems than this "whether kernel > > should enforce child cannot mmap() virtual EPC". > > IMO, virtual EPC should be restricted to a single mm_struct, which is what was > originally proposed many years ago[*]. I should have pushed back harder, but by > that point I had mostly stopped caring about SGX. > > There is no use case for sharing a virtual EPC, and conceptually it just doesn't > make sense because all use would need to be mutually exclusive on a per-page basis > to keep the EPCM happy. > > [*] https://lore.kernel.org/kvm/ace9d4cb10318370f6145aaced0cfa73dda36477.1609890536.git.kai.huang@xxxxxxxxx Thanks for your time. Yes I agree. Also, IMHO we can further enforce to only allow associating one VMA with one virtual EPC instance. It also doesn't make sense to have multi-mappings (VMAs) to the same virtual EPC pages even for one mm_struct, right? This also makes zapping PTE easier (as mentioned below). > > > First of all, if we want to use epc->owner to carry the userspace virtual > > address, "make kernel enforce child cannot mmap() virtual EPC" alone isn't good > > enough -- nothing prevents userspace to call mmap() several times to map the > > same virtual EPC pages. So additionally, we also need to "make kernel enforce > > one virtual EPC can only be mmap()-ed once". > > > > Secondly, I am thinking that the current arch_memory_failure() cannot really > > handle #MC for virtual EPC page correctly. The problem is, even we mark the > > page as poisoned, and send signal to userspace to inject #MC to guest to handle, > > the poisoned virtual EPC page is never unmapped from the guest and then freed. > > > > This means a malicious guest can always try to use the poisoned EPC page again > > after it receives #MC on some EPC page. I am not entirely sure what kind > > behaviour/attack can be done in such case, but it seems the right behaviour > > should be the KVM to inject the #MC and unmap the poisoned EPC page from guest. > > And if guest ever tries to use this "guest's EPC page" (GFN) again, KVM should > > kill the guest. > > Just zap the PTEs for the affected mm_struct, mmu_notifier => KVM will do the rest. Zapping PTEs alone can, i.e. take the poisoned virtual EPC page away from the guest and map a new one, but it doesn't inject #MC to the guest. W/o injecting #MC, the guest will just see sudden lose of enclave which consumes that EPC page (enclave is marked as bad by hardware if any page caused #MC). But I guess it's also acceptable since we already have other cases that guest can possibly see sudden lose of enclave (i.e. live migration)? Btw, currently the virtual EPC driver doesn't have infrastructure to zap PTEs. If we can associate one virtual EPC instance to one VMA, then zapping PTE can be easily done by bookkeeping this VMA. Does this look good?
