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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.

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.

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