On 6/16/22 01:49, Hyeonggon Yoo wrote: > On Tue, Jun 14, 2022 at 11:31:48AM -0700, Dave Hansen wrote: >> On 6/13/22 23:39, Hyeonggon Yoo wrote: >>> @@ -1514,6 +1515,11 @@ static int __change_page_attr(struct cpa_data *cpa, int primary) >>> pte_t *kpte, old_pte; >>> >>> address = __cpa_addr(cpa, cpa->curpage); >>> + >>> + if (WARN((IS_ENABLED(CONFIG_EFI) ? cpa->pgd != efi_mm.pgd : true) >>> + && address <= TASK_SIZE_MAX, >>> + KERN_WARNING "CPA: Got a user address")) >>> + return -EINVAL; >> >> I was expecting this to actually go after _PAGE_USER, not necessarily >> userspace addresses themselves. > > userspace ptes may not have _PAGE_USER set. (e.g. swap entry) > I think it's more accurate to go after user addresses. It would, of course, have to be paired with _PAGE_PRESENT checks. This works both on the way in and out of the set_memory code. It shouldn't clear other bits a PTE with _PAGE_PRESENT|_PAGE_USER and also shouldn't *result* in _PAGE_USER|_PAGE_PRESENT PTEs, even if those PTEs are in the kernel address space. Filtering on the addresses also makes sense. >> What does and should happen with the VDSO, for instance? It's a >> _PAGE_USER mapping, but it's >TASK_SIZE. > > you mean vsyscall? AFAIK address of mapped vDSO image is < TASK_SIZE. > (or please tell me I'm wrong) You're right. That was a silly thinko. >> Should set_page_attr() work on it? > > vsyscall does not need CPA functionalities. > So I don't think it (__change_page_attr()) should work on vsyscall. Agreed.
