On Wed, Oct 13, 2021, Brijesh Singh wrote: > > On 10/12/21 11:46 AM, Sean Christopherson wrote: > > On Fri, Aug 20, 2021, Brijesh Singh wrote: > >> When SEV-SNP is enabled, the guest private pages are added in the RMP > >> table; while adding the pages, the rmp_make_private() unmaps the pages > >> from the direct map. If KSM attempts to access those unmapped pages then > >> it will trigger #PF (page-not-present). > >> > >> Encrypted guest pages cannot be shared between the process, so an > >> userspace should not mark the region mergeable but to be safe, mark the > >> process vma unmerable before adding the pages in the RMP table. > > To be safe from what? Does the !PRESENT #PF crash the kernel? > > Yes, kernel crashes when KSM attempts to access to an unmaped pfn. Is this problem unique to nuking the direct map (patch 05), or would it also be a problem (in the form of an RMP violation) if the direct map were demoted to 4k pages? > [...] > >> + mmap_write_lock(kvm->mm); > >> + ret = snp_mark_unmergable(kvm, params.uaddr, params.len); > >> + mmap_write_unlock(kvm->mm); > > This does not, and practically speaking cannot, work. There are multiple TOCTOU > > bugs, here and in __snp_handle_page_state_change(). Userspace can madvise() the > > range at any later point, munmap()/mmap() the entire range, mess with the memslots > > in the PSC case, and so on and so forth. Relying on MADV_UNMERGEABLE for functional > > correctness simply cannot work in KVM, barring mmu_notifier and a big pile of code. > > AFAICT, ksm does not exclude the unmapped pfn from its scan list. We > need to tell ksm somehow to exclude the unmapped pfn from its scan list. > I understand that if userspace is messing with us, we have an issue, but > it's a userspace bug ;) To fix it right, we need to enhance ksm to > exclude the pfn when it is getting unmapped from the direct map. I > believe that work can be done outside of the SNP series. I am okay to > drop snp_mark_unmerable(), and until then, we just run with KSM > disabled. Thoughts? > > thanks