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

[Julien Grall <julien@xxxxxxx>]

Hi,

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?

Cheer,

--
Julien Grall