kernel/mm/guestmem: make write_u8() unsafe

write_u8() can write arbitrary byte to abritrary address. Even though
it already catches writes to unmapped memory, it doesn't prevent a
malicious user to write at a dangerous place if it can control the
parameters. It should be made unsafe.

Signed-off-by: Thomas Leroy <thomas.leroy@suse.com>

kernel/src/debug/gdbstub.rs

@@ -394,13 +394,22 @@ pub mod svsm_gdbstub {
             // mapping
 
             let Ok(phys) = this_cpu().get_pgtable().phys_addr(addr) else {
-                // The virtual address is not one that SVSM has mapped. Try safely
-                // writing it to the original virtual address
-                return write_u8(addr, value);
+                // The virtual address is not one that SVSM has mapped.
+                // Try safely writing it to the original virtual address
+                // SAFETY: it is up to the user to ensure that the address we
+                // are writing a breakpoint to is valid.
+                return unsafe { write_u8(addr, value) };
             };
 
             let guard = PerCPUPageMappingGuard::create_4k(phys.page_align())?;
-            write_u8(guard.virt_addr() + phys.page_offset(), value)
+            let dst = guard
+                .virt_addr()
+                .checked_add(phys.page_offset())
+                .ok_or(SvsmError::InvalidAddress)?;
+
+            // SAFETY: guard is a new mapped page, non controllable by user.
+            // We also checked that the destination address didn't overflow.
+            unsafe { write_u8(dst, value) }
         }
     }
 
@@ -532,9 +541,11 @@ pub mod svsm_gdbstub {
         ) -> gdbstub::target::TargetResult<(), Self> {
             let start_addr = VirtAddr::from(start_addr);
             for (off, src) in data.iter().enumerate() {
-                if write_u8(start_addr + off, *src).is_err() {
-                    return Err(TargetError::NonFatal);
-                }
+                let dst = start_addr.checked_add(off).ok_or(TargetError::NonFatal)?;
+
+                // SAFETY: We trust the caller of this trait method to provide a valid address.
+                // We only cheked that start_adddr + off didn't overflow.
+                unsafe { write_u8(dst, *src).map_err(|_| TargetError::NonFatal)? }
             }
             Ok(())
         }

kernel/src/mm/guestmem.rs

@@ -39,9 +39,20 @@ pub fn read_u8(v: VirtAddr) -> Result<u8, SvsmError> {
     }
 }
 
+/// Writes 1 byte at a virtual address.
+///
+/// # Safety
+///
+/// The caller must verify not to corrupt arbitrary memory, as this function
+/// doesn't make any checks in that regard.
+///
+/// # Returns
+///
+/// Returns an error if the specified address is not mapped or is not mapped
+/// with the appropriate write permissions.
 #[allow(dead_code)]
 #[inline]
-pub fn write_u8(v: VirtAddr, val: u8) -> Result<(), SvsmError> {
+pub unsafe fn write_u8(v: VirtAddr, val: u8) -> Result<(), SvsmError> {
     let mut rcx: u64;
 
     unsafe {
@@ -244,7 +255,10 @@ mod tests {
         let test_address = VirtAddr::from(test_buffer.as_mut_ptr());
         let data_to_write = 0x42;
 
-        write_u8(test_address, data_to_write).unwrap();
+        // SAFETY: test_address points to the virtual address of test_buffer.
+        unsafe {
+            write_u8(test_address, data_to_write).unwrap();
+        }
 
         assert_eq!(test_buffer[0], data_to_write);
     }