Re: [Xen-devel] [PATCH 1/2] xen/xsm: forbid PV guest console reads

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 09/30/2013 02:06 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Sep 30, 2013 at 01:29:00PM -0400, Daniel De Graaf wrote:
> > On 09/30/2013 12:10 PM, Jan Beulich wrote:
> > > > > > On 30.09.13 at 17:48, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> > > > When the hypervisor was compiled in debug mode (with VERBOSE defined),
> > > > PV guests incorrectly had access to both read and write to the console.
> > > > Change this to only allow write access; since such writes were limited
> > > > by log levels in 48d50de8e0, remove the dependency on VERBOSE
> > > > completely.
> > > I disagree, and iirc I disagreed already when you tried to drop the
> > > dependency on VERBOSE with that earlier patch.
> > >
> > > > Reported-by: Jan Beulich <JBeulich@xxxxxxxx>
> > > > Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> > > > ---
> > > >
> > > > Alternatively, if controlling writes with VERBOSE is still desired, the
> > > > ifdef VERBOSE can be retained surrounding the if() with the following
> > > > commit message:
> > > That's what I'd want to see go in.
> > >
> > > Jan
> > This patch retains the existing behavior where only HVM guests can use
> > the console for output, and only via the 0xE9 I/O port. With Konrad's
> > Linux patch, this means that xen_raw_console_write in non-dom0 will
> > produce output only on Xen <= 4.3 (which returns -ENOSYS rather than
> > -EPERM, as this code does).
> I was under the impression that what we wanted is:
>    - normal PV guests can do console_write
>    - HVM guests can do console_write
>    - priv guests (irregardless if they are HVM or PV) can do console_write
>      _and_ console_read.
>
> Which I think the patch below still allows right?
All of your bullets are true if the hypervisor is compiled with VERBOSE
(which is enabled if debug=y). Otherwise, only priv guests will be able
to use console_write (and it won't matter if they are PV or HVM). Either
way, only priv guests will be able to use console_read - which is the
only thing the below patch actually changes.

> > ------------------------8<----------------------------------------------
> >
> > The CONSOLEIO_read operation was incorrectly allowed to PV guests if the
> > hypervisor was compiled in debug mode (with VERBOSE defined).
> >
> > Reported-by: Jan Beulich <JBeulich@xxxxxxxx>
> > Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> > ---
> >   xen/include/xsm/dummy.h | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> > index 2abf018..b1edd29 100644
> > --- a/xen/include/xsm/dummy.h
> > +++ b/xen/include/xsm/dummy.h
> > @@ -233,10 +233,10 @@ static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG 
> > struct domain *d, int cmd)
> >   {
> >       XSM_ASSERT_ACTION(XSM_OTHER);
> >   #ifdef VERBOSE
> > -    return xsm_default_action(XSM_HOOK, current->domain, NULL);
> > -#else
> > -    return xsm_default_action(XSM_PRIV, current->domain, NULL);
> > +    if ( cmd == CONSOLEIO_write )
> > +        return xsm_default_action(XSM_HOOK, d, NULL);
> >   #endif
> > +    return xsm_default_action(XSM_PRIV, d, NULL);
> >   }
> >   static XSM_INLINE int xsm_profile(XSM_DEFAULT_ARG struct domain *d, int op)

--
Daniel De Graaf
National Security Agency

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