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

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

On Tue, Jun 14, 2022 at 05:52:31PM +0000, Edgecombe, Rick P wrote:
> On Tue, 2022-06-14 at 15:39 +0900, Hyeonggon Yoo wrote:
> > Currently CPA is not used for user mappings (only pgd of init_mm
> > or and efi_mm is used). For simplicity, always fail when user address
> > is passed.
> > 
> > Note that efi_mm uses 1:1 mapping so its address should not be
> > considered as user address.
> > 
> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> > ---
> >  arch/x86/mm/pat/set_memory.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/x86/mm/pat/set_memory.c
> > b/arch/x86/mm/pat/set_memory.c
> > index 1abd5438f126..67cf969fed0d 100644
> > --- a/arch/x86/mm/pat/set_memory.c
> > +++ b/arch/x86/mm/pat/set_memory.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/cc_platform.h>
> >  #include <linux/set_memory.h>
> > +#include <linux/efi.h>
> >  
> >  #include <asm/e820/api.h>
> >  #include <asm/processor.h>
> > @@ -1514,6 +1515,11 @@ static int __change_page_attr(struct cpa_data
> > *cpa, int primary)
> 
> I guess having it here instead of __change_page_attr_set_clr() will
> result in the direct map alias addresses getting checked as well. Since
> these are determined inside of CPA, I'm not sure if it's needed as
> much.

It does not check alias address when it failed.
I put it in __change_page_attr() with CPA_ARRAY in mind.
Because it may not be a single continuous area.

> 
> >  	pte_t *kpte, old_pte;
> >  
> >  	address = __cpa_addr(cpa, cpa->curpage);
> > +
> > +	if (WARN((IS_ENABLED(CONFIG_EFI) ? cpa->pgd != efi_mm.pgd :
> > true)
> 
> Could it be?
> 
> (!IS_ENABLED(CONFIG_EFI) || cpa->pgd != efi_mm.pgd)

Looks better, will update in v2.

Thanks!
Hyeonggon

> 
> > +		  && address <= TASK_SIZE_MAX,
> > +		 KERN_WARNING "CPA: Got a user address"))
> > +		return -EINVAL;
> >  repeat:
> >  	kpte = _lookup_address_cpa(cpa, address, &level);
> >  	if (!kpte)

-- 
Thanks,
Hyeonggon