Re: [RFC 1/2] x86/mm/cpa: always fail when user address is passed

[Hyeonggon Yoo <42.hyeyoo@gmail.com>]

On Thu, Jun 16, 2022 at 07:20:09AM -0700, Dave Hansen wrote:
> 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

You mean nothing should not use set_memory code for PTEs with
_PAGE_USER|_PAGE_PRESENT but set_memory can still be used to clear
_PAGE_USER|_PAGE_PRESENT?

Can't we just simply deny any PTE/PMDs with _PAGE_PRESENT|_PAGE_USER?

> also shouldn't
> *result* in _PAGE_USER|_PAGE_PRESENT PTEs, even if those PTEs are in the
> kernel address space.

Makes sense.

> 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.

-- 
Thanks,
Hyeonggon