Re: [PATCH 1/2] xsm: create idle domain privieged and demote after setup

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 4/20/22 14:31, Jason Andryuk wrote:
> On Wed, Apr 20, 2022 at 1:02 PM Daniel P. Smith
> <dpsmith@xxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> There are now instances where internal hypervisor logic needs to make 
>> resource
>> allocation calls that are protectd by XSM checks. The internal hypervisor 
>> logic
>> is represented a number of system domains which by designed are represented 
>> by
>> non-privileged struct domain instances. To enable these logic blocks to
>> function correctly but in a controlled manner, this commit changes the idle
>> domain to be created as a privileged domain under the default policy, which 
>> is
>> inherited by the SILO policy, and demoted before transitioning to running. A
>> new XSM hook, xsm_transition_running, is introduced to allow each XSM policy
>> type to demote the idle domain appropriately for that policy type.
>>
>> 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_transtion_running.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>> ---
> 
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 6f20e17892..72695dcb07 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -621,6 +621,12 @@ static void noreturn init_done(void)
>>      void *va;
>>      unsigned long start, end;
>>
>> +    xsm_transition_running();
>> +
>> +    /* Ensure idle domain was not left privileged */
>> +    if ( current->domain->is_privileged )
>> +        panic("idle domain did not properly transition from setup 
>> privilege\n");
> 
> Checking immediately after the XSM hook seems redundant, though I
> guess having a sanity check isn't harmful.

I was back and forth on this, so I threw it in and figured if there was
strong opinions against it I could easily remove and respin the series.

>>  static void cf_check flask_domain_free_security(struct domain *d)
>>  {
>>      struct domain_security_struct *dsec = d->ssid;
>> @@ -1766,6 +1780,7 @@ static int cf_check flask_argo_send(
>>  #endif
>>
>>  static const struct xsm_ops __initconst_cf_clobber flask_ops = {
>> +    .transition_running = flask_domain_runtime_security,
> 
> I'd prefer flask_transition_running.  That way grep for the hook name
> also finds the flask implementation.

Sure.

v/r,
dps