On Wed, May 11, 2022 at 07:30:34AM -0400, Daniel P. Smith wrote: > There are new capabilities, dom0less and hyperlaunch, that introduce internal > hypervisor logic which needs to make resource allocation calls that are > protected by XSM access checks. This creates an issue as a subset of the > hypervisor code is executed under a system domain, the idle domain, that is > represented by a per-CPU non-privileged struct domain.
Should you mention that this subset of hypervisor code that requires extended privileges but executed in the idle vCPU context strictly only happens during initial domain(s) creation? > To enable these new > capabilities to function correctly but in a controlled manner, this commit > changes the idle system domain to be created as a privileged domain under the > default policy and demoted before transitioning to running. A new XSM hook, > xsm_set_system_active(), is introduced to allow each XSM policy type to demote > the idle domain appropriately for that policy type. In the case of SILO, it > inherits the default policy's hook for xsm_set_system_active(). > > For flask a stub is added to ensure that flask policy system will function > correctly with this patch until flask is extended with support for starting > the > idle domain privileged and properly demoting it on the call to > xsm_set_system_active(). > > Signed-off-by: Daniel P. Smith <[email protected]> > Reviewed-by: Jason Andryuk <[email protected]> > Reviewed-by: Luca Fancellu <[email protected]> > Acked-by: Julien Grall <[email protected]> # arm > --- > xen/arch/arm/setup.c | 3 +++ > xen/arch/x86/setup.c | 4 ++++ > xen/common/sched/core.c | 7 ++++++- > xen/include/xsm/dummy.h | 17 +++++++++++++++++ > xen/include/xsm/xsm.h | 6 ++++++ > xen/xsm/dummy.c | 1 + > xen/xsm/flask/hooks.c | 23 +++++++++++++++++++++++ > 7 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index d5d0792ed4..7f3f00aa6a 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -1048,6 +1048,9 @@ void __init start_xen(unsigned long boot_phys_offset, > /* Hide UART from DOM0 if we're using it */ > serial_endboot(); > > + if ( (rc = xsm_set_system_active()) != 0 ) > + panic("xsm(err=%d): unable to set hypervisor to SYSTEM_ACTIVE > privilege\n", rc); > + > system_state = SYS_STATE_active; > > for_each_domain( d ) > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 6f20e17892..57ee6cc407 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -620,6 +620,10 @@ static void noreturn init_done(void) > { > void *va; > unsigned long start, end; > + int err; > + > + if ( (err = xsm_set_system_active()) != 0 ) > + panic("xsm(err=%d): unable to set hypervisor to SYSTEM_ACTIVE > privilege\n", err); Can you place err on a new line to make the line length no longer than strictly necessary. I think you could also reduce the printed message to: "unable to switch to SYSTEM_ACTIVE privilege: %d\n" Which could likely fit in a line (seeing as others are fine with the longer message I'm not going to insist). > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c > index 0bf63ffa84..54745e6c6a 100644 > --- a/xen/xsm/flask/hooks.c > +++ b/xen/xsm/flask/hooks.c > @@ -186,6 +186,28 @@ static int cf_check flask_domain_alloc_security(struct > domain *d) > return 0; > } > > +static int cf_check flask_set_system_active(void) > +{ > + struct domain *d = current->domain; > + > + ASSERT(d->is_privileged); > + > + if ( d->domain_id != DOMID_IDLE ) > + { > + printk("%s: should only be called by idle domain\n", __func__); > + return -EPERM; > + } > + > + /* > + * While is_privileged has no significant meaning under flask, set to > false > + * as is_privileged is not only used for a privilege check but also as a > type Nit: I think this line is over 80 cols. Thanks, Roger.
