Remove the remainder of host-managed stack cookies

They may not be terribly useful as we move towards a stack that the
guest is allowed to grow on its own.

Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com>

Author: syntactically

src/hyperlight_common/src/mem.rs

@@ -41,7 +41,6 @@ pub struct GuestStack {
 #[derive(Debug, Clone, Copy)]
 #[repr(C)]
 pub struct HyperlightPEB {
-    pub security_cookie_seed: u64,
     pub guest_function_dispatch_ptr: u64,
     pub input_stack: GuestMemoryRegion,
     pub output_stack: GuestMemoryRegion,

src/hyperlight_host/src/hypervisor/hyperlight_vm.rs

@@ -771,13 +771,6 @@ impl HyperlightVm {
                             });
                         }
                         None => {
-                            if !mem_mgr
-                                .check_stack_guard()
-                                .map_err(|e| RunVmError::CheckStackGuard(Box::new(e)))?
-                            {
-                                break Err(RunVmError::StackOverflow);
-                            }
-
                             break Err(RunVmError::MmioReadUnmapped(addr));
                         }
                     }
@@ -800,13 +793,6 @@ impl HyperlightVm {
                             });
                         }
                         None => {
-                            if !mem_mgr
-                                .check_stack_guard()
-                                .map_err(|e| RunVmError::CheckStackGuard(Box::new(e)))?
-                            {
-                                break Err(RunVmError::StackOverflow);
-                            }
-
                             break Err(RunVmError::MmioWriteUnmapped(addr));
                         }
                     }

src/hyperlight_host/src/mem/layout.rs

@@ -72,7 +72,6 @@ use std::fmt::Debug;
 use std::mem::{offset_of, size_of};
 
 use hyperlight_common::mem::{GuestStack, HyperlightPEB, PAGE_SIZE_USIZE};
-use rand::{RngCore, rng};
 use tracing::{Span, instrument};
 
 #[cfg(feature = "init-paging")]
@@ -101,7 +100,6 @@ pub(crate) struct SandboxMemoryLayout {
     /// The following fields are offsets to the actual PEB struct fields.
     /// They are used when writing the PEB struct itself
     peb_offset: usize,
-    peb_security_cookie_seed_offset: usize,
     peb_guest_dispatch_function_ptr_offset: usize, // set by guest in guest entrypoint
     peb_input_data_offset: usize,
     peb_output_data_offset: usize,
@@ -149,10 +147,6 @@ impl Debug for SandboxMemoryLayout {
             .field("PEB Address", &format_args!("{:#x}", self.peb_address))
             .field("PEB Offset", &format_args!("{:#x}", self.peb_offset))
             .field("Code Size", &format_args!("{:#x}", self.code_size))
-            .field(
-                "Security Cookie Seed Offset",
-                &format_args!("{:#x}", self.peb_security_cookie_seed_offset),
-            )
             .field(
                 "Guest Dispatch Function Pointer Offset",
                 &format_args!("{:#x}", self.peb_guest_dispatch_function_ptr_offset),
@@ -245,8 +239,6 @@ impl SandboxMemoryLayout {
         let guest_code_offset = 0;
         // The following offsets are to the fields of the PEB struct itself!
         let peb_offset = code_size.next_multiple_of(PAGE_SIZE_USIZE);
-        let peb_security_cookie_seed_offset =
-            peb_offset + offset_of!(HyperlightPEB, security_cookie_seed);
         let peb_guest_dispatch_function_ptr_offset =
             peb_offset + offset_of!(HyperlightPEB, guest_function_dispatch_ptr);
         let peb_input_data_offset = peb_offset + offset_of!(HyperlightPEB, input_stack);
@@ -281,7 +273,6 @@ impl SandboxMemoryLayout {
             peb_offset,
             stack_size: stack_size_rounded,
             heap_size,
-            peb_security_cookie_seed_offset,
             peb_guest_dispatch_function_ptr_offset,
             peb_input_data_offset,
             peb_output_data_offset,
@@ -692,11 +683,6 @@ impl SandboxMemoryLayout {
 
         // Start of setting up the PEB. The following are in the order of the PEB fields
 
-        // Set up the security cookie seed
-        let mut security_cookie_seed = [0u8; 8];
-        rng().fill_bytes(&mut security_cookie_seed);
-        shared_mem.copy_from_slice(&security_cookie_seed, self.peb_security_cookie_seed_offset)?;
-
         // Skip guest_dispatch_function_ptr_offset because it is set by the guest
 
         // Skip code, is set when loading binary

src/hyperlight_host/src/mem/mgr.rs

@@ -13,9 +13,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-#[cfg(feature = "init-paging")]
-use std::cmp::Ordering;
-
 use flatbuffers::FlatBufferBuilder;
 use hyperlight_common::flatbuffer_wrappers::function_call::{
     FunctionCall, validate_guest_function_call_buffer,
@@ -33,9 +30,6 @@ use super::shared_mem::{ExclusiveSharedMemory, GuestSharedMemory, HostSharedMemo
 use crate::sandbox::snapshot::Snapshot;
 use crate::{Result, new_error};
 
-/// The size of stack guard cookies
-pub(crate) const STACK_COOKIE_LEN: usize = 16;
-
 /// A struct that is responsible for laying out and managing the memory
 /// for a given `Sandbox`.
 #[derive(Clone)]
@@ -52,8 +46,6 @@ pub(crate) struct SandboxMemoryManager<S> {
     pub(crate) entrypoint_offset: Option<Offset>,
     /// How many memory regions were mapped after sandbox creation
     pub(crate) mapped_rgns: u64,
-    /// Stack cookie for stack guard verification
-    pub(crate) stack_cookie: [u8; STACK_COOKIE_LEN],
     /// Buffer for accumulating guest abort messages
     pub(crate) abort_buffer: Vec<u8>,
 }
@@ -160,7 +152,6 @@ where
         scratch_mem: S,
         load_addr: RawPtr,
         entrypoint_offset: Option<Offset>,
-        stack_cookie: [u8; STACK_COOKIE_LEN],
     ) -> Self {
         Self {
             layout,
@@ -169,17 +160,10 @@ where
             load_addr,
             entrypoint_offset,
             mapped_rgns: 0,
-            stack_cookie,
             abort_buffer: Vec::new(),
         }
     }
 
-    /// Get the stack cookie
-    #[instrument(skip_all, parent = Span::current(), level= "Trace")]
-    pub(crate) fn get_stack_cookie(&self) -> &[u8; STACK_COOKIE_LEN] {
-        &self.stack_cookie
-    }
-
     /// Get mutable access to the abort buffer
     pub(crate) fn get_abort_buffer_mut(&mut self) -> &mut Vec<u8> {
         &mut self.abort_buffer
@@ -217,7 +201,6 @@ impl SandboxMemoryManager<ExclusiveSharedMemory> {
         shared_mem.copy_from_slice(s.memory(), 0)?;
         let scratch_mem = ExclusiveSharedMemory::new(s.layout().get_scratch_size())?;
         let load_addr: RawPtr = RawPtr::try_from(layout.get_guest_code_address())?;
-        let stack_cookie = rand::random::<[u8; STACK_COOKIE_LEN]>();
         let entrypoint_gva = s.preinitialise();
         let entrypoint_offset = entrypoint_gva.map(|x| (x - u64::from(&load_addr)).into());
         Ok(Self::new(
@@ -226,7 +209,6 @@ impl SandboxMemoryManager<ExclusiveSharedMemory> {
             scratch_mem,
             load_addr,
             entrypoint_offset,
-            stack_cookie,
         ))
     }
 
@@ -266,7 +248,6 @@ impl SandboxMemoryManager<ExclusiveSharedMemory> {
             load_addr: self.load_addr.clone(),
             entrypoint_offset: self.entrypoint_offset,
             mapped_rgns: self.mapped_rgns,
-            stack_cookie: self.stack_cookie,
             abort_buffer: self.abort_buffer,
         };
         let guest_mgr = SandboxMemoryManager {
@@ -276,7 +257,6 @@ impl SandboxMemoryManager<ExclusiveSharedMemory> {
             load_addr: self.load_addr.clone(),
             entrypoint_offset: self.entrypoint_offset,
             mapped_rgns: self.mapped_rgns,
-            stack_cookie: self.stack_cookie,
             abort_buffer: Vec::new(), // Guest doesn't need abort buffer
         };
         host_mgr.update_scratch_bookkeeping(
@@ -287,33 +267,6 @@ impl SandboxMemoryManager<ExclusiveSharedMemory> {
 }
 
 impl SandboxMemoryManager<HostSharedMemory> {
-    /// Check the stack guard of the memory in `shared_mem`, using
-    /// `layout` to calculate its location.
-    ///
-    /// Return `true`
-    /// if `shared_mem` could be accessed properly and the guard
-    /// matches `cookie`. If it could be accessed properly and the
-    /// guard doesn't match `cookie`, return `false`. Otherwise, return
-    /// a descriptive error.
-    ///
-    /// This method could be an associated function instead. See
-    /// documentation at the bottom `set_stack_guard` for description
-    /// of why it isn't.
-    #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
-    #[cfg(feature = "init-paging")]
-    pub(crate) fn check_stack_guard(&self) -> Result<bool> {
-        let expected = self.stack_cookie;
-        let offset = self.layout.get_top_of_user_stack_offset();
-        let actual: [u8; STACK_COOKIE_LEN] = self.shared_mem.read(offset)?;
-        let cmp_res = expected.iter().cmp(actual.iter());
-        Ok(cmp_res == Ordering::Equal)
-    }
-
-    #[cfg(not(feature = "init-paging"))]
-    pub(crate) fn check_stack_guard(&self) -> Result<bool> {
-        Ok(true)
-    }
-
     /// Get the address of the dispatch function in memory
     #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
     pub(crate) fn get_pointer_to_dispatch_function(&self) -> Result<u64> {

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

@@ -649,8 +649,6 @@ impl MultiUseSandbox {
                 return Err(error);
             }
 
-            self.mem_mgr.check_stack_guard()?;
-
             let guest_result = self.mem_mgr.get_guest_function_call_result()?.into_inner();
 
             match guest_result {
@@ -817,9 +815,7 @@ impl Callable for MultiUseSandbox {
 
 impl std::fmt::Debug for MultiUseSandbox {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("MultiUseSandbox")
-            .field("stack_guard", &self.mem_mgr.get_stack_cookie())
-            .finish()
+        f.debug_struct("MultiUseSandbox").finish()
     }
 }
 

src/hyperlight_host/src/sandbox/snapshot.rs

@@ -575,7 +575,6 @@ mod tests {
             scratch_mem,
             0.into(),
             None,
-            [0u8; 16],
         );
         let (mgr, _) = mgr.build().unwrap();
         (mgr, pt_base as u64)