From 0ff074d2b8faad16e514b2db81c6ed0c9c2d60f8 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Wed, 24 Jul 2019 14:34:10 -0700 Subject: [PATCH] vm-allocator: Fix potential allocation errors There is one corner case which was not properly handled by the current code from our AddressAllocator. If both the address start (from the next range) and the requested region size are already aligned on the same value as "alignment", when doing the substract of the requested size + alignment, the returned address is already aligned. The problem is that we might end up overlapping with an existing range since the check between the available delta and the requested size does not take into account a full extra alignment. By substracting the requested size + alignment - 1 from the address start of the next range, we ensure this kind of corner case would not happen since the address won't be naturally aligned and after some adjustment from the function align_address(), the correct start address will be returned. Signed-off-by: Sebastien Boeuf --- vm-allocator/src/address.rs | 13 +++++++------ vm-allocator/src/system.rs | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/vm-allocator/src/address.rs b/vm-allocator/src/address.rs index bfb99f36a..30d8ec5ef 100755 --- a/vm-allocator/src/address.rs +++ b/vm-allocator/src/address.rs @@ -30,7 +30,7 @@ pub type Result = result::Result; /// # use vm_memory::{Address, GuestAddress, GuestUsize}; /// AddressAllocator::new(GuestAddress(0x1000), 0x10000).map(|mut pool| { /// assert_eq!(pool.allocate(None, 0x110, Some(0x100)), Some(GuestAddress(0x10e00))); -/// assert_eq!(pool.allocate(None, 0x100, Some(0x100)), Some(GuestAddress(0x10c00))); +/// assert_eq!(pool.allocate(None, 0x100, Some(0x100)), Some(GuestAddress(0x10d00))); /// }); /// ``` #[derive(Debug, Eq, PartialEq)] @@ -147,9 +147,10 @@ impl AddressAllocator { if let Some(size_delta) = address.checked_sub(self.align_address(prev_end_address, alignment).raw_value()) { + let adjust = if alignment > 1 { alignment - 1 } else { 0 }; if size_delta.raw_value() >= req_size { return Some( - self.align_address(address.unchecked_sub(req_size + alignment), alignment), + self.align_address(address.unchecked_sub(req_size + adjust), alignment), ); } } @@ -242,12 +243,12 @@ mod tests { let mut pool = AddressAllocator::new(GuestAddress(0x1000), 0x1000).unwrap(); assert_eq!( pool.allocate(None, 0x800, Some(0x100)), - Some(GuestAddress(0x1700)) + Some(GuestAddress(0x1800)) ); assert_eq!(pool.allocate(None, 0x900, Some(0x100)), None); assert_eq!( pool.allocate(None, 0x400, Some(0x100)), - Some(GuestAddress(0x1200)) + Some(GuestAddress(0x1400)) ); } @@ -260,11 +261,11 @@ mod tests { ); assert_eq!( pool.allocate(None, 0x100, Some(0x100)), - Some(GuestAddress(0x10c00)) + Some(GuestAddress(0x10d00)) ); assert_eq!( pool.allocate(None, 0x10, Some(0x100)), - Some(GuestAddress(0x10d00)) + Some(GuestAddress(0x10c00)) ); } diff --git a/vm-allocator/src/system.rs b/vm-allocator/src/system.rs index 224e59db9..92fdc5a5e 100755 --- a/vm-allocator/src/system.rs +++ b/vm-allocator/src/system.rs @@ -35,7 +35,7 @@ fn pagesize() -> usize { /// vec![GsiApic::new(5, 19)]).unwrap(); /// assert_eq!(allocator.allocate_irq(), Some(5)); /// assert_eq!(allocator.allocate_irq(), Some(6)); -/// assert_eq!(allocator.allocate_mmio_addresses(None, 0x1000, Some(0x1000)), Some(GuestAddress(0x1fffe000))); +/// assert_eq!(allocator.allocate_mmio_addresses(None, 0x1000, Some(0x1000)), Some(GuestAddress(0x1fff_f000))); /// /// ``` pub struct SystemAllocator {