Skip to content

Commit

Permalink
kernel/vtpm: fix potential UB when accessing guest buffer
Browse files Browse the repository at this point in the history
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 <cclaudio@linux.ibm.com>
Suggested-by: Carlos López <carlos.lopezr4096@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
  • Loading branch information
stefano-garzarella committed Jan 28, 2025
1 parent 389b3cb commit 15633c6
Showing 1 changed file with 20 additions and 4 deletions.
24 changes: 20 additions & 4 deletions kernel/src/protocols/vtpm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

Expand Down Expand Up @@ -239,20 +241,34 @@ fn vtpm_command_request(params: &RequestParams) -> Result<(), SvsmReqError> {
return Err(SvsmReqError::unsupported_call());
}

let buffer = unsafe { from_raw_parts_mut(vaddr.as_mut_ptr::<u8>(), 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::<u8>(), 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::<u32>() goes to the folowing page, it should
// not be a problem since the end of the requested region is
// (paddr + PAGE_SIZE), which requests another page. So
// write(response_size) can only happen on valid memory, mapped
// by PerCPUPageMappingGuard::create().
unsafe {
GuestPtr::<u32>::new(vaddr).write(response_size)?;
GuestPtr::<u32>::new(vaddr).write(response_size?)?;
}

Ok(())
Expand Down

0 comments on commit 15633c6

Please sign in to comment.