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

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Wed, 2022-06-15 at 12:26 +0900, Hyeonggon Yoo wrote:
> 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.

Makes sense.

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

Christoph's seems better to me.