Re: [Xen-devel] [PATCH v2 03/21] xen: allow console_io hypercalls from certain DomUs

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 17/07/2018 21:05, Stefano Stabellini wrote:
> On Mon, 9 Jul 2018, Julien Grall wrote:
> > Hi,
> >
> > On 07/07/18 00:11, Stefano Stabellini wrote:
> > > Introduce an is_console option to allow certain classes of domUs to use
> > > the Xen console. Specifically, it will be used to give console access to
> > > all domUs started from Xen from information on device tree.
> > >
> > > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > > CC: andrew.cooper3@xxxxxxxxxx
> > > CC: George.Dunlap@xxxxxxxxxxxxx
> > > CC: ian.jackson@xxxxxxxxxxxxx
> > > CC: jbeulich@xxxxxxxx
> > > CC: konrad.wilk@xxxxxxxxxx
> > > CC: tim@xxxxxxx
> > > CC: wei.liu2@xxxxxxxxxx
> > > CC: dgdegra@xxxxxxxxxxxxx
> > > ---
> > > Changes in v2:
> > > - introduce is_console
> > > - remove #ifdefs
> > > ---
> > >    xen/include/xen/sched.h | 2 ++
> > >    xen/include/xsm/dummy.h | 2 ++
> > >    xen/xsm/flask/hooks.c   | 5 ++++-
> > >    3 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > > index 99d2af2..d66cec0 100644
> > > --- a/xen/include/xen/sched.h
> > > +++ b/xen/include/xen/sched.h
> > > @@ -379,6 +379,8 @@ struct domain
> > >        bool             auto_node_affinity;
> > >        /* Is this guest fully privileged (aka dom0)? */
> > >        bool             is_privileged;
> > > +    /* Can this guest access the Xen console? */
> > > +    bool             is_console;
> > >        /* Is this a xenstore domain (not dom0)? */
> > >        bool             is_xenstore;
> > >        /* Domain's VCPUs are pinned 1:1 to physical CPUs? */
> > > diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> > > index ff6b2db..3888817 100644
> > > --- a/xen/include/xsm/dummy.h
> > > +++ b/xen/include/xsm/dummy.h
> > > @@ -230,6 +230,8 @@ static XSM_INLINE int
> > > xsm_memory_stat_reservation(XSM_DEFAULT_ARG struct domain
> > >    static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain *d, int
> > > cmd)
> > >    {
> > >        XSM_ASSERT_ACTION(XSM_OTHER);
> > > +    if ( d->is_console )
> > > +        return xsm_default_action(XSM_HOOK, d, NULL);
> > I will let Daniel commenting on this change. However ...
> >
> > >    #ifdef CONFIG_VERBOSE_DEBUG
> > >        if ( cmd == CONSOLEIO_write )
> > >            return xsm_default_action(XSM_HOOK, d, NULL);
> > > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> > > index 78bc326..2551e4e 100644
> > > --- a/xen/xsm/flask/hooks.c
> > > +++ b/xen/xsm/flask/hooks.c
> > > @@ -443,7 +443,10 @@ static int flask_console_io(struct domain *d, int cmd)
> > >            return avc_unknown_permission("console_io", cmd);
> > >        }
> > >    -    return domain_has_xen(d, perm);
> > > +    if ( !d->is_console )
> > > +        return domain_has_xen(d, perm);
> > > +    else
> > > +        return 0;
> > ... I don't think this change is correct. When a policy is used, the user is
> > free to define what is the behavior. With your solution, you impose the
> > console access even if the user didn't to not give the permission.
> I was hoping Daniel would advise on the best way to do things here.
>
> I thought that the idea was that granting a domain "is_console" is
> equivalent to granting a domain XEN__READCONSOLE and XEN__WRITECONSOLE
> permissions.  Thus, if is_console is set, we return 0 from
> flask_console_io because the permissions check succeeds.
Well, yes and no. That's equivalent when you use the dummy policy. When 
you have a flask policy you want to give the control to the user.
If you look at the code there are no such as d->is_privilege in that 
function. This means that the user define the policy for the hardware 
domain. Why would be d->is_console different here?
> Given that I have accumulated many changes to this patch series, I'll
> send out a new version now without making changes to this patch for now.
I haven't finished to review the series yet and would appreciate some 
more time before resending it.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel