From 15633c6fbf8204f32e20308d02d3d565d45f8074 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Tue, 28 Jan 2025 14:30:48 +0100 Subject: [PATCH] kernel/vtpm: fix potential UB when accessing guest buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a mutable reference to the guest buffer, could have been an UB because the guest could potentially modify it. So, let's revoke buffer access to the guest before creating the mutable reference with `from_raw_parts_mut()`. Restore buffer access to the guest, after the reference to the buffer is dropped (internal block in the match). Suggested-by: Claudio Carvalho Suggested-by: Carlos López Signed-off-by: Stefano Garzarella --- kernel/src/protocols/vtpm.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/kernel/src/protocols/vtpm.rs b/kernel/src/protocols/vtpm.rs index 66333cfb9..6876563ed 100644 --- a/kernel/src/protocols/vtpm.rs +++ b/kernel/src/protocols/vtpm.rs @@ -14,9 +14,11 @@ use alloc::vec::Vec; use crate::{ address::{Address, PhysAddr}, + cpu::flush_tlb_global_sync, mm::{valid_phys_address, GuestPtr, PerCPUPageMappingGuard}, protocols::{errors::SvsmReqError, RequestParams}, - types::PAGE_SIZE, + sev::utils::{rmp_grant_guest_access, rmp_revoke_guest_access}, + types::{PageSize, PAGE_SIZE}, vtpm::{vtpm_get_locked, TcgTpmSimulatorInterface, VtpmProtocolInterface}, }; @@ -239,12 +241,26 @@ 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) }; + // Make sure the guest can't make modifications to the buffer + rmp_revoke_guest_access(vaddr, PageSize::Regular)?; + // TLB flush needed to propagate new permissions + flush_tlb_global_sync(); let response_size = match cmd { - TpmPlatformCommand::SendCommand => tpm_send_command_request(buffer)?, + TpmPlatformCommand::SendCommand => { + // SAFETY: vaddr is just mapped, we are sure the guest can't modify it + // because we revoked the access, and its size is PAGE_SIZE + let buffer = unsafe { from_raw_parts_mut(vaddr.as_mut_ptr::(), PAGE_SIZE) }; + + tpm_send_command_request(buffer) + } }; + // Allow guest to access the buffer again + rmp_grant_guest_access(vaddr, PageSize::Regular)?; + // TLB flush needed to propagate new permissions + flush_tlb_global_sync(); + // SAFETY: vaddr points to a new mapped region. // if paddr + sizeof::() goes to the folowing page, it should // not be a problem since the end of the requested region is @@ -252,7 +268,7 @@ fn vtpm_command_request(params: &RequestParams) -> Result<(), SvsmReqError> { // write(response_size) can only happen on valid memory, mapped // by PerCPUPageMappingGuard::create(). unsafe { - GuestPtr::::new(vaddr).write(response_size)?; + GuestPtr::::new(vaddr).write(response_size?)?; } Ok(())