Re: [Xen-devel] [PATCH v1 2/6] x86: Temporary disable SMAP to legally access user pages in kernel mode

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Tian, Kevin
> Sent: Tuesday, April 15, 2014 2:36 PM
> To: Wu, Feng; JBeulich@xxxxxxxx; Ian.Campbell@xxxxxxxxxx;
> xen-devel@xxxxxxxxxxxxx
> Cc: Dong, Eddie; Nakajima, Jun
> Subject: RE: [Xen-devel] [PATCH v1 2/6] x86: Temporary disable SMAP to legally
> access user pages in kernel mode
> 
> > From: Wu, Feng
> > Sent: Tuesday, April 15, 2014 2:21 PM
> >
> > > -----Original Message-----
> > > From: Tian, Kevin
> > > Sent: Tuesday, April 15, 2014 2:08 PM
> > > To: Wu, Feng; JBeulich@xxxxxxxx; Ian.Campbell@xxxxxxxxxx;
> > > xen-devel@xxxxxxxxxxxxx
> > > Cc: Wu, Feng; Dong, Eddie; Nakajima, Jun
> > > Subject: RE: [Xen-devel] [PATCH v1 2/6] x86: Temporary disable SMAP to
> > legally
> > > access user pages in kernel mode
> > >
> > > > From: Feng Wu
> > > > Sent: Tuesday, April 15, 2014 9:02 PM
> > > >
> > > > Use STAC/CLAC to temporary disable SMAP to allow legal accesses to
> > > > user pages in kernel mode
> > > >
> > > > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > > > ---
> > > >  xen/arch/x86/clear_page.S           |  3 +++
> > > >  xen/arch/x86/domain_build.c         | 16 ++++++++++++++++
> > > >  xen/arch/x86/usercopy.c             |  6 ++++++
> > > >  xen/arch/x86/x86_64/compat/entry.S  |  2 ++
> > > >  xen/arch/x86/x86_64/entry.S         |  4 ++++
> > > >  xen/include/asm-x86/uaccess.h       |  4 ++++
> > > >  xen/include/asm-x86/x86_64/system.h |  2 ++
> > > >  7 files changed, 37 insertions(+)
> > > >
> > > > diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> > > > index 84ce392..1ba138b 100644
> > > > --- a/xen/arch/x86/domain_build.c
> > > > +++ b/xen/arch/x86/domain_build.c
> > > > @@ -789,8 +789,10 @@ int __init construct_dom0(
> > > >              rc = -1;
> > > >              goto out;
> > > >          }
> > > > +        stac();
> > > >          hypercall_page_initialise(
> > > >              d, (void *)(unsigned long)parms.virt_hypercall);
> > > > +        clac();
> > > >      }
> > >
> > > construct_dom0 happens before dom0 starts execution. It should be fine to
> > > keep AC cleared in the whole phase.
> >
> > construct_dom0() accesses user pages in the supervisor mode, so I added the
> > stac()/clac()
> > one by one, otherwise, Xen hypervisor will receive an SMAP violation.
> >
> 
> I meant you can do once to enable user page accesses, and then disable it
> at end of construct_dom0. There should be no security concern at this stage.

Yes, that is a clean solution. I also thought about it before, but I was not 
sure whether
there was security issues with it. As you said, if there is no security 
concern, your
suggestion is better for sure.

> 
> Thanks
> Kevin

Thanks,
Feng

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel