Re: [PATCH 2/2] arch: ensure idle domain is not left privileged

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 31 Mar 2022, Julien Grall wrote:
> On 31/03/2022 13:46, Roger Pau Monné wrote:
> > On Wed, Mar 30, 2022 at 07:05:49PM -0400, Daniel P. Smith wrote:
> > > It is now possible to promote the idle domain to privileged during setup.
> > > It
> > > is not desirable for the idle domain to still be privileged when moving
> > > into a
> > > running state. If the idle domain was elevated and not properly demoted,
> > > it is
> > > desirable to fail at this point. This commit adds an assert for both x86
> > > and
> > > Arm just before transitioning to a running state that ensures the idle is
> > > not
> > > privileged.
> > > 
> > > Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > > ---
> > >   xen/arch/arm/setup.c | 3 +++
> > >   xen/arch/x86/setup.c | 3 +++
> > >   2 files changed, 6 insertions(+)
> > > 
> > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > > index 7968cee47d..3de394e946 100644
> > > --- a/xen/arch/arm/setup.c
> > > +++ b/xen/arch/arm/setup.c
> > > @@ -973,6 +973,9 @@ void __init start_xen(unsigned long boot_phys_offset,
> > >       /* Hide UART from DOM0 if we're using it */
> > >       serial_endboot();
> > >   +    /* Ensure idle domain was not left privileged */
> > > +    ASSERT(current->domain->is_privileged == false) ;
> > > +
> > >       system_state = SYS_STATE_active;
> > >         create_domUs();
> > 
> > Hm, I think you want to use the permission promotion of the idle
> > domain in create_domUs() likely?
> > 
> > At which point the check should be after create_domUs, and it would
> > seem that logically SYS_STATE_active should be set after creating the
> > domUs.
> > 
> > Also, FWIW, I'm not seeing this create_domUs() call in my context,
> > maybe you have other patches on your queue?
> I think the code is based on an old version of Xen (looks like 4.14). In newer
> version create_domUs() is called before just before discard_initial_modules()
> (see XSA-372 for the rationale).
> 
> Daniel, can you please rebase this series to the latest staging?

Yeah they should be rebased. I have done it so that I could test this
approach as well, see attached.

I also added a patch that calls:

  xsm_elevate_priv(current->domain);

at the beginning of create_domUs, then calls:

  xsm_demote_priv(current->domain);

at the end of create_domUs.

With all that in place, dom0less+PV drivers works fine.

Note that I don't know if we want to do this within create_domUs of if
there is a better place, I was just trying to make sure everything works
as expected.

Attachment: 0001-xsm-add-ability-to-elevate-a-domain-to-privileged.patch
Description: Text Data

Attachment: 0002-arch-ensure-idle-domain-is-not-left-privileged.patch
Description: Text Data

Attachment: 0003-xen-arm-temporarily-elevate-idle_domain-privileged-d.patch
Description: Text Data