From f6018bc30d642aea972f39ba996f486ecd1e7ce2 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Thu, 20 Feb 2025 17:33:18 +0100 Subject: [PATCH 1/2] kernel/mm: add GuestPtr::read_ref() `GuestPtr::read()` returns `T` allocated on the stack. This could be a problem for large objects, so let's add `GuestPtr::read_ref()` which allows us to read `GuestPtr` into a pre-allocated buffer passed by reference. Signed-off-by: Stefano Garzarella --- kernel/src/mm/guestmem.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/kernel/src/mm/guestmem.rs b/kernel/src/mm/guestmem.rs index debae7ab0..6edf63f31 100644 --- a/kernel/src/mm/guestmem.rs +++ b/kernel/src/mm/guestmem.rs @@ -255,6 +255,20 @@ impl GuestPtr { unsafe { do_movsb(&buf, self.ptr) } } + /// # Safety + /// + /// The caller must verify not to read arbitrary memory, as this function + /// doesn't make any checks in that regard. + /// + /// # Returns + /// + /// Returns an error if the specified address is not mapped. + #[inline] + pub unsafe fn read_ref(&self, buf: &mut T) -> Result<(), SvsmError> { + // SAFETY: demanded to the caller + unsafe { do_movsb(self.ptr, buf) } + } + /// # Safety /// /// The caller must verify not to corrupt arbitrary memory, as this function From 40d73d582a52023b88f3df32ae7de14511ac9fe1 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Tue, 28 Jan 2025 14:30:48 +0100 Subject: [PATCH 2/2] kernel/vtpm: fix potential UB when accessing guest buffer Creating a mutable reference to the guest buffer, could have been an UB because the guest could potentially modify it. So, let's copy the guest buffer to an internal one and then copy it back to give the result to the guest. To avoid overflowing the stack, allocate the buffer with Vec. Signed-off-by: Stefano Garzarella --- kernel/src/protocols/vtpm.rs | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/kernel/src/protocols/vtpm.rs b/kernel/src/protocols/vtpm.rs index 66333cfb9..bf6da84b8 100644 --- a/kernel/src/protocols/vtpm.rs +++ b/kernel/src/protocols/vtpm.rs @@ -8,9 +8,9 @@ extern crate alloc; -use core::{mem::size_of, slice::from_raw_parts_mut}; +use core::mem::size_of; -use alloc::vec::Vec; +use alloc::{vec, vec::Vec}; use crate::{ address::{Address, PhysAddr}, @@ -239,10 +239,32 @@ fn vtpm_command_request(params: &RequestParams) -> Result<(), SvsmReqError> { return Err(SvsmReqError::unsupported_call()); } - let buffer = unsafe { from_raw_parts_mut(vaddr.as_mut_ptr::(), PAGE_SIZE) }; - let response_size = match cmd { - TpmPlatformCommand::SendCommand => tpm_send_command_request(buffer)?, + TpmPlatformCommand::SendCommand => { + // Let's use Vec to avoid consuming too much stack. + let mut buffer: Vec = vec![0; PAGE_SIZE]; + let buf_slice: &mut [u8; PAGE_SIZE] = buffer + .as_mut_slice() + .try_into() + .expect("Failed to initialize vTPM buffer"); + + let buf_ptr = GuestPtr::<[u8; PAGE_SIZE]>::new(vaddr); + + // SAFETY: `buf_ptr` is initialized with `vaddr` which points to + // a valid region just mapped, and its size is PAGE_SIZE. + // Since we need a mutable buffer in the next calls, to avoid UB, + // we copy the guest buffer to an internal one and then copy it back + // to give the result to the guest. + unsafe { buf_ptr.read_ref(buf_slice)? }; + + let response_size = tpm_send_command_request(buf_slice)?; + + // SAFETY: `buf_ptr` is initialized with `vaddr` which points to + // a valid region just mapped, and its size is PAGE_SIZE. + unsafe { buf_ptr.write_ref(buf_slice)? }; + + response_size + } }; // SAFETY: vaddr points to a new mapped region.