Introducing a module memory protector that encapsulates internals of …

…the hardware memory isolation mechanism configuration setup.

Signed-off-by: Wojciech Ozga <woz@zurich.ibm.com>

security-monitor/src/confidential_flow/control_flow/mod.rs

@@ -2,7 +2,6 @@
 // SPDX-FileContributor: Wojciech Ozga <woz@zurich.ibm.com>, IBM Research - Zurich
 // SPDX-License-Identifier: Apache-2.0
 use crate::core::control_data::{ConfidentialVmId, ControlData, HardwareHart};
-use crate::core::memory_partitioner::MemoryPartitioner;
 use crate::core::transformations::{ExposeToConfidentialVm, PendingRequest, TrapReason};
 use crate::non_confidential_flow::NonConfidentialFlow;
 
@@ -55,7 +54,6 @@ impl<'a> ConfidentialFlow<'a> {
         use crate::confidential_flow::handlers::{
             guest_load_page_fault_result, guest_store_page_fault_result, hypercall_result, share_page_result,
         };
-
         match self.hart.confidential_hart_mut().take_request() {
             Some(PendingRequest::SbiRequest()) => hypercall_result::handle(self.hart.hypercall_result(), self),
             Some(PendingRequest::GuestLoadPageFault(request)) => {
@@ -72,12 +70,7 @@ impl<'a> ConfidentialFlow<'a> {
     pub fn into_non_confidential_flow(self) -> NonConfidentialFlow<'a> {
         let id = self.confidential_vm_id();
         match ControlData::try_confidential_vm(id, |mut cvm| Ok(cvm.return_confidential_hart(self.hart))) {
-            Ok(_) => {
-                // it is safe to close access to confidential memory here because we transition in the
-                // finite state machine to nodes that will lead to the execution of a hypervisor.
-                unsafe { MemoryPartitioner::enable_hypervisor_memory_view() };
-                NonConfidentialFlow::create(self.hart)
-            }
+            Ok(_) => NonConfidentialFlow::create(self.hart),
             Err(error) => self.exit_to_confidential_vm(error.into_confidential_transformation()),
         }
     }
@@ -88,7 +81,10 @@ impl<'a> ConfidentialFlow<'a> {
     }
 
     pub fn confidential_vm_id(&'a self) -> ConfidentialVmId {
-        self.hart.confidential_hart().confidential_vm_id()
+        self.hart
+            .confidential_hart()
+            .confidential_vm_id()
+            .expect("Bug: found dummy hart instead of a confidential hart")
     }
 
     pub fn set_pending_request(self, request: PendingRequest) -> Self {

security-monitor/src/confidential_flow/handlers/share_page_result.rs

@@ -26,7 +26,7 @@ pub fn handle(share_page_result: SharePageResult, confidential_flow: Confidentia
 
     let confidential_vm_id = confidential_flow.confidential_vm_id();
     let transformation = ControlData::try_confidential_vm_mut(confidential_vm_id, |mut cvm| {
-        cvm.root_page_table_mut().map_shared_page(shared_page)
+        cvm.memory_protector_mut().map_shared_page(shared_page)
     })
     .and_then(|_| Ok(ExposeToConfidentialVm::SbiResult(SbiResult::success(0))))
     .unwrap_or_else(|error| error.into_confidential_transformation());

security-monitor/src/core/control_data/confidential_hart.rs

@@ -15,18 +15,20 @@ use crate::error::Error;
 /// (registers/CSRs) is by calling the constructor or applying a transformation.
 #[repr(C)]
 pub struct ConfidentialHart {
+    // if there is no confidential vm id assigned to this hart then it means that this confidential hart
+    // is a dummy one. A dummy virtual hart means that the confidential_hart is not associated with any
+    // confidential VM
+    confidential_vm_id: Option<ConfidentialVmId>,
     // Safety: Careful, HardwareHart and ConfidentialHart must both start with the HartState element because based on
     // this we automatically calculate offsets of registers' and CSRs' for the asm code.
     confidential_hart_state: HartState,
     pending_request: Option<PendingRequest>,
-    // a dummy virtual hart means that the confidential_hart is not associated with any confidential VM
-    dummy: bool,
 }
 
 impl ConfidentialHart {
     pub fn dummy(id: usize) -> Self {
         let confidential_hart_state = HartState::empty(id);
-        Self { confidential_hart_state, pending_request: None, dummy: true }
+        Self { confidential_hart_state, pending_request: None, confidential_vm_id: None }
     }
 
     pub fn from_vm_hart_reset(id: usize, from: &HartState) -> Self {
@@ -41,7 +43,7 @@ impl ConfidentialHart {
         confidential_hart_state.medeleg = 0b1011001111111111;
         confidential_hart_state.hedeleg = confidential_hart_state.medeleg;
 
-        Self { confidential_hart_state, pending_request: None, dummy: false }
+        Self { confidential_hart_state, pending_request: None, confidential_vm_id: None }
     }
 
     pub fn from_vm_hart(id: usize, from: &HartState) -> Self {
@@ -63,8 +65,12 @@ impl ConfidentialHart {
         confidential_hart
     }
 
-    pub fn confidential_vm_id(&self) -> ConfidentialVmId {
-        ConfidentialVmId::new(riscv::register::hgatp::Hgatp::from(self.confidential_hart_state.hgatp).vmid())
+    pub fn set_confidential_vm_id(&mut self, confidential_vm_id: ConfidentialVmId) {
+        self.confidential_vm_id = Some(confidential_vm_id);
+    }
+
+    pub fn confidential_vm_id(&self) -> Option<ConfidentialVmId> {
+        self.confidential_vm_id
     }
 
     pub(super) fn confidential_hart_id(&self) -> usize {
@@ -76,7 +82,7 @@ impl ConfidentialHart {
     }
 
     pub fn is_dummy(&self) -> bool {
-        self.dummy
+        self.confidential_vm_id.is_none()
     }
 
     pub fn set_pending_request(&mut self, request: PendingRequest) -> Result<(), Error> {
@@ -88,14 +94,6 @@ impl ConfidentialHart {
 
 // functions to inject information to a confidential VM.
 impl ConfidentialHart {
-    pub fn hgatp(&self) -> usize {
-        self.confidential_hart_state.hgatp
-    }
-
-    pub fn set_hgatp(&mut self, hgatp: usize) {
-        self.confidential_hart_state.hgatp = hgatp;
-    }
-
     pub fn apply(&mut self, transformation: ExposeToConfidentialVm) -> usize {
         match transformation {
             ExposeToConfidentialVm::SbiResult(v) => self.apply_sbi_result(v),

security-monitor/src/core/control_data/confidential_vm.rs

@@ -2,10 +2,9 @@
 // SPDX-FileContributor: Wojciech Ozga <woz@zurich.ibm.com>, IBM Research - Zurich
 // SPDX-License-Identifier: Apache-2.0
 use crate::core::control_data::{ConfidentialHart, HardwareHart};
-use crate::core::memory_partitioner::RootPageTable;
+use crate::core::memory_protector::ConfidentialVmMemoryProtector;
 use crate::error::Error;
 use alloc::vec::Vec;
-use riscv::register::hgatp::Hgatp;
 
 const MAX_HASH_SIZE: usize = 512; // 512b for SHA-512
 
@@ -25,39 +24,57 @@ pub struct ConfidentialVm {
     id: ConfidentialVmId,
     _measurements: [Measurement; 4],
     confidential_harts: Vec<ConfidentialHart>,
-    root_page_table: RootPageTable,
+    memory_protector: ConfidentialVmMemoryProtector,
 }
 
 impl ConfidentialVm {
     pub fn new(
-        id: ConfidentialVmId, mut confidential_harts: Vec<ConfidentialHart>, root_page_table: RootPageTable,
+        id: ConfidentialVmId, mut confidential_harts: Vec<ConfidentialHart>,
+        mut memory_protector: ConfidentialVmMemoryProtector,
     ) -> Self {
-        let hgatp = Hgatp::new(root_page_table.address(), root_page_table.paging_system().hgatp_mode(), id.0);
-        confidential_harts.iter_mut().for_each(|confidential_hart| confidential_hart.set_hgatp(hgatp.bits()));
-        Self { id, _measurements: [Measurement::empty(); 4], confidential_harts, root_page_table }
+        memory_protector.set_confidential_vm_id(id);
+        confidential_harts.iter_mut().for_each(|confidential_hart| confidential_hart.set_confidential_vm_id(id));
+        Self { id, _measurements: [Measurement::empty(); 4], confidential_harts, memory_protector }
     }
 
-    pub fn root_page_table_mut(&mut self) -> &mut RootPageTable {
-        &mut self.root_page_table
+    pub fn memory_protector_mut(&mut self) -> &mut ConfidentialVmMemoryProtector {
+        &mut self.memory_protector
     }
 
+    /// Steals a confidential hart from the confidential VM and assigns it to the hardware hart. The
+    /// hardware memory isolation mechanism is reconfigured to enforce memory access control for the
+    /// confidential VM. Error is returns if the confidential VM's virtual hart has been already stolen.
+    ///
+    /// Guarantees:
+    /// * If confidential hart is assigned to the hardware hart, then the hardware hart is configured to enforce memory
+    ///   access control of the confidential VM.
     pub fn steal_confidential_hart(
         &mut self, confidential_hart_id: usize, hardware_hart: &mut HardwareHart,
     ) -> Result<(), Error> {
         let confidential_hart = self.confidential_harts.get(confidential_hart_id).ok_or(Error::InvalidHartId())?;
-        // The hypervisor might try to schedule the same confidential_hart on different harts. We detect it because
-        // after a confidential_hart is scheduled for the first time, its token is stolen and the ConfidentialVM is left
-        // with a dummy confidential_hart.
+        // The hypervisor might try to schedule the same confidential hart on different physical harts. We detect it
+        // because after a confidential_hart is scheduled for the first time, its token is stolen and the
+        // ConfidentialVM is left with a dummy confidential_hart. A dummy confidential hart is a hart not associated
+        // with any confidential vm.
         assure_not!(confidential_hart.is_dummy(), Error::RunningVHart())?;
         core::mem::swap(&mut hardware_hart.confidential_hart, &mut self.confidential_harts[confidential_hart_id]);
+        // It is safe to invoke below unsafe code because at this point we are in the confidential flow part of the
+        // finite state machine and the virtual hart is assigned to the hardware hart. We must reconfigure the hardware
+        // memory isolation mechanism to enforce that the confidential virtual machine has access only to the memory
+        // regions it owns.
+        unsafe { self.memory_protector.enable() };
         Ok(())
     }
 
     pub fn return_confidential_hart(&mut self, hardware_hart: &mut HardwareHart) {
         assert!(!hardware_hart.confidential_hart.is_dummy());
-        assert!(self.id == hardware_hart.confidential_hart().confidential_vm_id());
+        assert!(Some(self.id) == hardware_hart.confidential_hart().confidential_vm_id());
         let confidential_hart_id = hardware_hart.confidential_hart.confidential_hart_id();
         core::mem::swap(&mut hardware_hart.confidential_hart, &mut self.confidential_harts[confidential_hart_id]);
+        // It is safe to reconfigure the memory access control configuration to enable access to memory regions owned
+        // by the hypervisor because we are now transitioning into the non-confidential flow part of the finite state
+        // machine where the hardware hart is associated with a dummy virtual hart.
+        unsafe { hardware_hart.enable_hypervisor_memory_protector() };
     }
 
     pub fn is_running(&self) -> bool {

security-monitor/src/core/control_data/hardware_hart.rs

@@ -3,9 +3,10 @@
 // SPDX-License-Identifier: Apache-2.0
 use crate::core::control_data::ConfidentialHart;
 use crate::core::hart::{GpRegister, HartState};
+use crate::core::memory_protector::HypervisorMemoryProtector;
 use crate::core::memory_tracker::{Allocated, Page, UnAllocated};
 use crate::core::transformations::{
-    EsmRequest, ExposeToHypervisor, GuestLoadPageFaultRequest, GuestLoadPageFaultResult, InterruptRequest,
+    ConvertToConfidentialVm, ExposeToHypervisor, GuestLoadPageFaultRequest, GuestLoadPageFaultResult, InterruptRequest,
     MmioLoadRequest, MmioStoreRequest, OpensbiRequest, ResumeRequest, SbiRequest, SbiResult, SbiVmRequest,
     SharePageResult, TerminateRequest, TrapReason,
 };
@@ -15,26 +16,30 @@ pub struct HardwareHart {
     // Careful, HardwareHart and ConfidentialHart must both start with the HartState element because based on this we
     // automatically calculate offsets of registers' and CSRs' for the asm code.
     pub(super) non_confidential_hart_state: HartState,
+    // Memory protector that configures the hardware memory isolation component to allow only memory accesses
+    // to the memory region owned by the hypervisor.
+    hypervisor_memory_protector: HypervisorMemoryProtector,
     // A page containing the stack of the code executing within the given hart.
     pub(super) stack: Page<Allocated>,
-    // the stack_address is redundant (we can learn the stack_address from the page assigned to the stack) but we need
+    // The stack_address is redundant (we can learn the stack_address from the page assigned to the stack) but we need
     // it because this is the way to expose it to assembly
     pub(super) stack_address: usize,
-    // we need to store the OpenSBI's mscratch value because OpenSBI uses mscratch to track some of its internal
+    // We need to store the OpenSBI's mscratch value because OpenSBI uses mscratch to track some of its internal
     // data structures and our security monitor also uses mscratch to keep track of the address of the hart state
     // in memory.
     previous_mscratch: usize,
-    // we keep the virtual hart that is associated with this hardware hart. The virtual hart can be 1) a dummy hart
+    // We keep the virtual hart that is associated with this hardware hart. The virtual hart can be 1) a dummy hart
     // in case there is any confidential VM's virtual hart associated to it, or 2) an confidential VM's virtual hart.
     // In the latter case, the hardware hart and confidential VM's control data swap their virtual harts (a dummy
     // hart with the confidential VM's virtual hart)
     pub(super) confidential_hart: ConfidentialHart,
 }
 
 impl HardwareHart {
-    pub fn init(id: usize, stack: Page<UnAllocated>) -> Self {
+    pub fn init(id: usize, stack: Page<UnAllocated>, hypervisor_memory_protector: HypervisorMemoryProtector) -> Self {
         Self {
             non_confidential_hart_state: HartState::empty(id),
+            hypervisor_memory_protector,
             stack_address: stack.end_address(),
             stack: stack.zeroize(),
             previous_mscratch: 0,
@@ -46,7 +51,7 @@ impl HardwareHart {
         core::ptr::addr_of!(self.non_confidential_hart_state) as usize
     }
 
-    /// calling OpenSBI handler to process the SBI call requires setting the mscratch register to a specific value which
+    /// Calling OpenSBI handler to process the SBI call requires setting the mscratch register to a specific value which
     /// we replaced during the system initialization. We store the original mscratch value expected by the OpenSBI in
     /// the previous_mscratch field.
     pub fn swap_mscratch(&mut self) {
@@ -62,6 +67,14 @@ impl HardwareHart {
     pub fn confidential_hart_mut(&mut self) -> &mut ConfidentialHart {
         &mut self.confidential_hart
     }
+
+    pub fn hypervisor_memory_protector(&self) -> &HypervisorMemoryProtector {
+        &self.hypervisor_memory_protector
+    }
+
+    pub unsafe fn enable_hypervisor_memory_protector(&self) {
+        self.hypervisor_memory_protector.enable(self.non_confidential_hart_state.hgatp)
+    }
 }
 
 impl HardwareHart {
@@ -207,8 +220,8 @@ impl HardwareHart {
         self.non_confidential_hart_state.trap_reason()
     }
 
-    pub fn esm_request(&self) -> EsmRequest {
-        EsmRequest::new(&self.non_confidential_hart_state)
+    pub fn convert_to_confidential_vm_request(&self) -> ConvertToConfidentialVm {
+        ConvertToConfidentialVm::new(&self.non_confidential_hart_state)
     }
 
     pub fn hypercall_result(&self) -> SbiResult {

security-monitor/src/core/control_data/storage.rs

@@ -2,7 +2,7 @@
 // SPDX-FileContributor: Wojciech Ozga <woz@zurich.ibm.com>, IBM Research - Zurich
 // SPDX-License-Identifier: Apache-2.0
 use crate::core::control_data::{ConfidentialHart, ConfidentialVm, ConfidentialVmId};
-use crate::core::memory_partitioner::RootPageTable;
+use crate::core::memory_protector::ConfidentialVmMemoryProtector;
 use crate::error::{Error, NOT_INITIALIZED_CONTROL_DATA};
 use alloc::collections::BTreeMap;
 use alloc::vec::Vec;
@@ -28,7 +28,7 @@ impl ControlData {
     /// to it. This identifier is not secret and reflects the number of confidential VMs that have been created up to
     /// now. The maximum allowed number of confidential VMs created is limited by the size of the `usize` type.
     pub fn store_confidential_vm(
-        confidential_harts: Vec<ConfidentialHart>, root_page_table: RootPageTable,
+        confidential_harts: Vec<ConfidentialHart>, memory_protector: ConfidentialVmMemoryProtector,
     ) -> Result<ConfidentialVmId, Error> {
         Self::try_write(|control_data| {
             control_data
@@ -39,7 +39,7 @@ impl ControlData {
                 .unwrap_or(Some(0))
                 .and_then(|max_id| {
                     let id = ConfidentialVmId::new(max_id);
-                    let confidential_vm = ConfidentialVm::new(id, confidential_harts, root_page_table);
+                    let confidential_vm = ConfidentialVm::new(id, confidential_harts, memory_protector);
                     control_data.confidential_vms.insert(id, Mutex::new(confidential_vm));
                     Some(id)
                 })

security-monitor/src/core/heap/mod.rs

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: 2023 IBM Corporation
 // SPDX-FileContributor: Wojciech Ozga <woz@zurich.ibm.com>, IBM Research - Zurich
 // SPDX-License-Identifier: Apache-2.0
-use crate::core::memory_partitioner::ConfidentialMemoryAddress;
+use crate::core::memory_layout::ConfidentialMemoryAddress;
 use allocator::MemoryAllocator;
 
 mod allocator;