diff mbox series

[v5,1/2] xsm: create idle domain privileged and demote after setup

Message ID 20220502133027.920-2-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series Adds starting the idle domain privileged | expand

Commit Message

Daniel P. Smith May 2, 2022, 1:30 p.m. UTC 
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. 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>
---
 xen/arch/arm/setup.c    |  4 ++++
 xen/arch/x86/setup.c    |  5 +++++
 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   | 14 ++++++++++++++
 7 files changed, 53 insertions(+), 1 deletion(-)

Comments

Jason Andryuk May 2, 2022, 1:42 p.m. UTC | #1 
On Mon, May 2, 2022 at 9:31 AM Daniel P. Smith
<dpsmith@apertussolutions.com> wrote:
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed4..b9057222d6 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -1048,6 +1048,10 @@ 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", err);

You want to print rc in this message.

Regards,
Jason
Daniel P. Smith May 2, 2022, 1:49 p.m. UTC | #2 
On 5/2/22 09:42, Jason Andryuk wrote:
> On Mon, May 2, 2022 at 9:31 AM Daniel P. Smith
> <dpsmith@apertussolutions.com> wrote:
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index d5d0792ed4..b9057222d6 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -1048,6 +1048,10 @@ 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", err);
> 
> You want to print rc in this message.

Thanks, but now I want to figure out how that compiled.

v/r,
dps
Daniel P. Smith May 2, 2022, 1:53 p.m. UTC | #3 
On 5/2/22 09:49, Daniel P. Smith wrote:
> On 5/2/22 09:42, Jason Andryuk wrote:
>> On Mon, May 2, 2022 at 9:31 AM Daniel P. Smith
>> <dpsmith@apertussolutions.com> wrote:
>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>> index d5d0792ed4..b9057222d6 100644
>>> --- a/xen/arch/arm/setup.c
>>> +++ b/xen/arch/arm/setup.c
>>> @@ -1048,6 +1048,10 @@ 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", err);
>>
>> You want to print rc in this message.
> 
> Thanks, but now I want to figure out how that compile

Ah, arm which I do not have a build env to do build tests.

v/r,
dps
Luca Fancellu May 3, 2022, 9:43 a.m. UTC | #4 
> On 2 May 2022, at 14:53, Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> 
> On 5/2/22 09:49, Daniel P. Smith wrote:
>> On 5/2/22 09:42, Jason Andryuk wrote:
>>> On Mon, May 2, 2022 at 9:31 AM Daniel P. Smith
>>> <dpsmith@apertussolutions.com> wrote:
>>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>>> index d5d0792ed4..b9057222d6 100644
>>>> --- a/xen/arch/arm/setup.c
>>>> +++ b/xen/arch/arm/setup.c
>>>> @@ -1048,6 +1048,10 @@ 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", err);
>>> 
>>> You want to print rc in this message.
>> 
>> Thanks, but now I want to figure out how that compile
> 
> Ah, arm which I do not have a build env to do build tests.

I’ve built this patch on arm (changing err to rc), everything looks fine, so with that
addressed:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>



> 
> v/r,
> dps
> 
>
Daniel P. Smith May 3, 2022, 11:30 a.m. UTC | #5 
On 5/3/22 05:43, Luca Fancellu wrote:
> 
> 
>> On 2 May 2022, at 14:53, Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
>>
>> On 5/2/22 09:49, Daniel P. Smith wrote:
>>> On 5/2/22 09:42, Jason Andryuk wrote:
>>>> On Mon, May 2, 2022 at 9:31 AM Daniel P. Smith
>>>> <dpsmith@apertussolutions.com> wrote:
>>>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>>>> index d5d0792ed4..b9057222d6 100644
>>>>> --- a/xen/arch/arm/setup.c
>>>>> +++ b/xen/arch/arm/setup.c
>>>>> @@ -1048,6 +1048,10 @@ 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", err);
>>>>
>>>> You want to print rc in this message.
>>>
>>> Thanks, but now I want to figure out how that compile
>>
>> Ah, arm which I do not have a build env to do build tests.
> 
> I’ve built this patch on arm (changing err to rc), everything looks fine, so with that
> addressed:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Thank you and my apologies for not adding your review-by this morning. I
had v6 queued to go out last night and missed this email before releasing.
Luca Fancellu May 3, 2022, 12:21 p.m. UTC | #6 
> On 3 May 2022, at 12:30, Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> 
> On 5/3/22 05:43, Luca Fancellu wrote:
>> 
>> 
>>> On 2 May 2022, at 14:53, Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
>>> 
>>> On 5/2/22 09:49, Daniel P. Smith wrote:
>>>> On 5/2/22 09:42, Jason Andryuk wrote:
>>>>> On Mon, May 2, 2022 at 9:31 AM Daniel P. Smith
>>>>> <dpsmith@apertussolutions.com> wrote:
>>>>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>>>>> index d5d0792ed4..b9057222d6 100644
>>>>>> --- a/xen/arch/arm/setup.c
>>>>>> +++ b/xen/arch/arm/setup.c
>>>>>> @@ -1048,6 +1048,10 @@ 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", err);
>>>>> 
>>>>> You want to print rc in this message.
>>>> 
>>>> Thanks, but now I want to figure out how that compile
>>> 
>>> Ah, arm which I do not have a build env to do build tests.
>> 
>> I’ve built this patch on arm (changing err to rc), everything looks fine, so with that
>> addressed:
>> 
>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> Thank you and my apologies for not adding your review-by this morning. I
> had v6 queued to go out last night and missed this email before releasing.
> 

Hi Daniel,

It’s ok I will put it again in the new serie.

Cheers,
Luca
diff mbox series

Patch

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index d5d0792ed4..b9057222d6 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1048,6 +1048,10 @@  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", err);
+
     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..36a60ce884 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -620,6 +620,11 @@  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);
 
     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..3291fb5396 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("xsm_set_system_active: should only be called by idle domain\n");
+        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..0bd4e8a4bd 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -186,6 +186,19 @@  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;
+
+    if ( d->domain_id != DOMID_IDLE )
+    {
+        printk("xsm_set_system_active should only be called by idle domain\n");
+        return -EPERM;
+    }
+
+    return 0;
+}
+
 static void cf_check flask_domain_free_security(struct domain *d)
 {
     struct domain_security_struct *dsec = d->ssid;
@@ -1766,6 +1779,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,