diff --git a/src/devices/src/virtio/gpu/virtio_gpu.rs b/src/devices/src/virtio/gpu/virtio_gpu.rs index 927aa54..c4a65be 100644 --- a/src/devices/src/virtio/gpu/virtio_gpu.rs +++ b/src/devices/src/virtio/gpu/virtio_gpu.rs @@ -776,13 +776,16 @@ impl VirtioGpu { RUTABAGA_MAP_ACCESS_READ => libc::PROT_READ, RUTABAGA_MAP_ACCESS_WRITE => libc::PROT_WRITE, RUTABAGA_MAP_ACCESS_RW => libc::PROT_READ | libc::PROT_WRITE, - _ => panic!("unexpected prot mode for mapping"), + _ => return Err(ErrUnspec), }; - if offset + resource.size > shm_region.size as u64 { - error!("mapping DOES NOT FIT"); - } - let addr = shm_region.host_addr + offset; + let addr = checked_blob_map_addr( + shm_region.host_addr, + offset, + resource.size, + shm_region.size as u64, + ) + .ok_or(ErrUnspec)?; debug!( "mapping: host_addr={:x}, addr={:x}, size={}", shm_region.host_addr, addr, resource.size @@ -831,14 +834,16 @@ impl VirtioGpu { RUTABAGA_MAP_ACCESS_READ => libc::PROT_READ, RUTABAGA_MAP_ACCESS_WRITE => libc::PROT_WRITE, RUTABAGA_MAP_ACCESS_RW => libc::PROT_READ | libc::PROT_WRITE, - _ => panic!("unexpected prot mode for mapping"), + _ => return Err(ErrUnspec), }; - if offset + resource.size > shm_region.size as u64 { - error!("resource map doesn't fit in shm region"); - return Err(ErrUnspec); - } - let addr = shm_region.host_addr + offset; + let addr = checked_blob_map_addr( + shm_region.host_addr, + offset, + resource.size, + shm_region.size as u64, + ) + .ok_or(ErrUnspec)?; if let Ok(export) = self.rutabaga.export_blob(resource_id) { // SHM and DMABUF are both regular host fds whose pages can be exposed @@ -906,12 +911,13 @@ impl VirtioGpu { if let Ok(export) = self.rutabaga.export_blob(resource_id) { if export.handle_type == RUTABAGA_MEM_HANDLE_TYPE_APPLE { - if offset + resource.size > shm_region.size as u64 { - error!("mapping DOES NOT FIT"); - return Err(ErrUnspec); - } - - let guest_addr = shm_region.guest_addr + offset; + let guest_addr = checked_blob_map_addr( + shm_region.guest_addr, + offset, + resource.size, + shm_region.size as u64, + ) + .ok_or(ErrUnspec)?; debug!( "mapping: map_ptr={:x}, guest_addr={:x}, size={}", map_ptr, guest_addr, resource.size @@ -970,7 +976,8 @@ impl VirtioGpu { ) }; if ret == libc::MAP_FAILED { - panic!("UNMAP failed"); + error!("failed to unmap blob resource"); + return Err(ErrUnspec); } resource.shmem_offset = None; @@ -1014,10 +1021,42 @@ impl VirtioGpu { Ok(OkNoData) } } + +// A guest-controlled `offset` that wraps `offset + size` or `base + offset` would +// otherwise pass the size guard and place the mmap(MAP_FIXED) out of bounds. +fn checked_blob_map_addr(base: u64, offset: u64, size: u64, shm_size: u64) -> Option { + if offset.checked_add(size)? > shm_size { + return None; + } + base.checked_add(offset) +} + #[cfg(test)] mod test { use crate::virtio::gpu::protocol::VIRTIO_GPU_MAX_SCANOUTS; + #[test] + fn checked_blob_map_addr_rejects_out_of_range_and_wrapping_offsets() { + use super::checked_blob_map_addr; + + let base = 0x1_0000_u64; + let shm = 0x1_0000_u64; + + assert_eq!( + checked_blob_map_addr(base, 0x1000, 0x2000, shm), + Some(base + 0x1000) + ); + assert_eq!(checked_blob_map_addr(base, 0, shm, shm), Some(base)); + assert!(checked_blob_map_addr(base, shm, 1, shm).is_none()); + + let size = 0x1000_u64; + let wrapping_offset = u64::MAX - size + 1; + assert!(wrapping_offset.wrapping_add(size) <= shm); + assert!(checked_blob_map_addr(base, wrapping_offset, size, shm).is_none()); + + assert!(checked_blob_map_addr(u64::MAX - 5, 10, 0, u64::MAX).is_none()); + } + #[test] fn test_virtio_gpu_associated_scanouts() { use super::AssociatedScanouts;