[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Date: Mon, 4 Apr 2022 10:56:13 -0400
  • Arc-authentication-results: i=1; mx.zohomail.com; dkim=pass header.i=apertussolutions.com; spf=pass smtp.mailfrom=dpsmith@xxxxxxxxxxxxxxxxxxxx; dmarc=pass header.from=<dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1649084200; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=nMNcmVrf2F6pgJCVlrEQLsJYEWaCohoN0zG8AdD682k=; b=DBFHs5CYB4CZkuIzUV7uDVSWtZoV5I8yREhO/eosEX6KgI6FIYMtuwub9VXdYRDpwQbnz5jhr+VYQd8OiRs78O0lj2bzpwwhURJ79ay89RKbC3p0/o2YKui682IMJaE8wfRrcnqK4MU/qwBurONhisGFbyT42125fLgbRWVCaho=
  • Arc-seal: i=1; a=rsa-sha256; t=1649084200; cv=none; d=zohomail.com; s=zohoarc; b=ShqYNoKYZx66XfZ6O5DbKl7Rg1VAkJS6qqXeXwQlwt2dsbacqjLorak/gAZ7pGWfeX1khRrQmHYKxPHggjYli1J3S4UHg2vIPzLB1Iz4x7WP8cPT4MglBaoTP3tzl1io3fhAfxU1d2K24FcgLNNeAwiJqFefOfd+C29WDGsyFLM=
  • Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, scott.davis@xxxxxxxxxx, jandryuk@xxxxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Mon, 04 Apr 2022 14:56:54 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 3/31/22 08: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?

Apologies, I cherry-picked this onto a branch of staging of what I
thought was an up to date remote, but as Julien pointed out I was not.

> 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?
> 
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 885919d5c3..b868463f83 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -589,6 +589,9 @@ static void noinline init_done(void)
>>      void *va;
>>      unsigned long start, end;
>>  
>> +    /* Ensure idle domain was not left privileged */
>> +    ASSERT(current->domain->is_privileged == false) ;
>                                                       ^ extra space.
> 
> I think you could squash this patch with the previous one and also
> squash it with a patch that actually makes use of the introduced
> permission promotion functions (or at least in a patch series where
> further patches make use the introduced functions).

Ack, I can squash them together.

v/r,
dps



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.