| Message ID | 20220531145646.10062-2-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Adds starting the idle domain privileged | expand |
On 31.05.2022 16:56, 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. The need for these resource allocations are > necessary for dom0less and hyperlaunch when they are constructing the initial > domain(s). 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. 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 <dpsmith@apertussolutions.com> > Reviewed-by: Jason Andryuk <jandryuk@gmail.com> > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > Acked-by: Julien Grall <jgrall@amazon.com> # arm Hmm, here and on patch 2 you've lost Rahul's R-b and T-b, afaict. Jan
On 5/31/22 11:16, Jan Beulich wrote: > On 31.05.2022 16:56, 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. The need for these resource allocations are >> necessary for dom0less and hyperlaunch when they are constructing the initial >> domain(s). 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. 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 <dpsmith@apertussolutions.com> >> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> >> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> >> Acked-by: Julien Grall <jgrall@amazon.com> # arm > > Hmm, here and on patch 2 you've lost Rahul's R-b and T-b, afaict. erg, you are right, my apologies. Would you like me to respin as v9 to get it in there, so it is not lost? v/r, dps
On 31.05.2022 17:19, Daniel P. Smith wrote: > > On 5/31/22 11:16, Jan Beulich wrote: >> On 31.05.2022 16:56, 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. The need for these resource allocations are >>> necessary for dom0less and hyperlaunch when they are constructing the initial >>> domain(s). 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. 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 <dpsmith@apertussolutions.com> >>> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> >>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> >>> Acked-by: Julien Grall <jgrall@amazon.com> # arm >> >> Hmm, here and on patch 2 you've lost Rahul's R-b and T-b, afaict. > > erg, you are right, my apologies. Would you like me to respin as v9 to > get it in there, so it is not lost? Not sure; much depends on who would commit this if this ends up being the final version. (If you re-send, I'd suggest v8.1 rather than v9.) Jan
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index d5d0792ed4..6b88320588 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: unable to switch to SYSTEM_ACTIVE privilege: %d\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..962da03f80 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: unable to switch to SYSTEM_ACTIVE privilege: %d\n", err); system_state = SYS_STATE_active; diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index 19ab678181..7b1c03a0e1 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -3021,7 +3021,12 @@ void __init scheduler_init(void) sched_ratelimit_us = SCHED_DEFAULT_RATELIMIT_US; } - idle_domain = domain_create(DOMID_IDLE, NULL, 0); + /* + * The idle dom is created privileged to ensure unrestricted access during + * setup and will be demoted by xsm_set_system_active() when setup is + * complete. + */ + idle_domain = domain_create(DOMID_IDLE, NULL, CDF_privileged); BUG_ON(IS_ERR(idle_domain)); BUG_ON(nr_cpu_ids > ARRAY_SIZE(idle_vcpu)); idle_domain->vcpu = idle_vcpu; diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 58afc1d589..77f27e7163 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -101,6 +101,23 @@ static always_inline int xsm_default_action( } } +static XSM_INLINE int cf_check xsm_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; + } + + d->is_privileged = false; + + return 0; +} + static XSM_INLINE void cf_check xsm_security_domaininfo( struct domain *d, struct xen_domctl_getdomaininfo *info) { diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 3e2b7fe3db..8dad03fd3d 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -52,6 +52,7 @@ typedef enum xsm_default xsm_default_t; * !!! WARNING !!! */ struct xsm_ops { + int (*set_system_active)(void); void (*security_domaininfo)(struct domain *d, struct xen_domctl_getdomaininfo *info); int (*domain_create)(struct domain *d, uint32_t ssidref); @@ -208,6 +209,11 @@ extern struct xsm_ops xsm_ops; #ifndef XSM_NO_WRAPPERS +static inline int xsm_set_system_active(void) +{ + return alternative_call(xsm_ops.set_system_active); +} + static inline void xsm_security_domaininfo( struct domain *d, struct xen_domctl_getdomaininfo *info) { diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 8c044ef615..e6ffa948f7 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -14,6 +14,7 @@ #include <xsm/dummy.h> static const struct xsm_ops __initconst_cf_clobber dummy_ops = { + .set_system_active = xsm_set_system_active, .security_domaininfo = xsm_security_domaininfo, .domain_create = xsm_domain_create, .getdomaininfo = xsm_getdomaininfo, diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 0bf63ffa84..06ca4e7a91 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 of domain check, specifically if the domain is the control domain. + */ + d->is_privileged = false; + + return 0; +} + static void cf_check flask_domain_free_security(struct domain *d) { struct domain_security_struct *dsec = d->ssid; @@ -1766,6 +1788,7 @@ static int cf_check flask_argo_send( #endif static const struct xsm_ops __initconst_cf_clobber flask_ops = { + .set_system_active = flask_set_system_active, .security_domaininfo = flask_security_domaininfo, .domain_create = flask_domain_create, .getdomaininfo = flask_getdomaininfo,