From de06f1577789a831b797d83ac020b748c0b96548 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Wed, 15 Jul 2020 15:45:08 -0700 Subject: [PATCH] devices: pci: enforce minimum valid BAR sizes PCI BARs have a minimum size based on the number of bits reserved for the type bitfield, which is stored in the low bits of each BAR. The minimum size is derived from the number of bits used to store the type: 2 bits (4-byte minimum) for I/O bars and 4 bits (16-byte minimum) for memory BARs. Previously, since the minimum size was not enforced, callers could create invalid configurations that would allow the guest to overwrite the type bits of the BARs. Luckily, this only happened in the unit tests, which are fixed in this change. BUG=None TEST=cargo test -p devices Change-Id: I30d65485254b49655c9ad7bec51e43533fe2c01d Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2300692 Reviewed-by: Dylan Reid Tested-by: kokoro Commit-Queue: Daniel Verkamp --- devices/src/pci/pci_configuration.rs | 82 +++++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 7 deletions(-) diff --git a/devices/src/pci/pci_configuration.rs b/devices/src/pci/pci_configuration.rs index ebfa2a64e5..50f1b9719e 100644 --- a/devices/src/pci/pci_configuration.rs +++ b/devices/src/pci/pci_configuration.rs @@ -15,7 +15,9 @@ const STATUS_REG: usize = 1; const STATUS_REG_CAPABILITIES_USED_MASK: u32 = 0x0010_0000; const BAR0_REG: usize = 4; const BAR_IO_ADDR_MASK: u32 = 0xffff_fffc; +const BAR_IO_MIN_SIZE: u64 = 4; const BAR_MEM_ADDR_MASK: u32 = 0xffff_fff0; +const BAR_MEM_MIN_SIZE: u64 = 16; const NUM_BAR_REGS: usize = 6; const CAPABILITY_LIST_HEAD_OFFSET: usize = 0x34; const FIRST_CAPABILITY_OFFSET: usize = 0x40; @@ -199,7 +201,7 @@ pub struct PciBarConfiguration { prefetchable: PciBarPrefetchable, } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum Error { BarAddressInvalid(u64, u64), BarAlignmentInvalid(u64, u64), @@ -370,6 +372,10 @@ impl PciConfiguration { /// (i.e, region size must be power of two, register not already used). Returns 'None' on /// failure all, `Some(BarIndex)` on success. pub fn add_pci_bar(&mut self, config: &PciBarConfiguration) -> Result { + if config.reg_idx >= NUM_BAR_REGS { + return Err(Error::BarInvalid(config.reg_idx)); + } + if self.bar_used[config.reg_idx] { return Err(Error::BarInUse(config.reg_idx)); } @@ -378,8 +384,14 @@ impl PciConfiguration { return Err(Error::BarSizeInvalid(config.size)); } - if config.reg_idx >= NUM_BAR_REGS { - return Err(Error::BarInvalid(config.reg_idx)); + let min_size = if config.region_type == PciBarRegionType::IORegion { + BAR_IO_MIN_SIZE + } else { + BAR_MEM_MIN_SIZE + }; + + if config.size < min_size { + return Err(Error::BarSizeInvalid(config.size)); } if config.addr % config.size != 0 { @@ -711,7 +723,7 @@ mod tests { cfg.add_pci_bar( &PciBarConfiguration::new( 0, - 0x4, + 0x10, PciBarRegionType::Memory64BitRegion, PciBarPrefetchable::NotPrefetchable, ) @@ -725,7 +737,7 @@ mod tests { ); assert_eq!(cfg.get_bar_addr(0), 0x01234567_89ABCDE0); assert_eq!(cfg.writable_bits[BAR0_REG + 1], 0xFFFFFFFF); - assert_eq!(cfg.writable_bits[BAR0_REG + 0], 0xFFFFFFFC); + assert_eq!(cfg.writable_bits[BAR0_REG + 0], 0xFFFFFFF0); } #[test] @@ -744,7 +756,7 @@ mod tests { cfg.add_pci_bar( &PciBarConfiguration::new( 0, - 0x4, + 0x10, PciBarRegionType::Memory32BitRegion, PciBarPrefetchable::NotPrefetchable, ) @@ -757,7 +769,7 @@ mod tests { Some(PciBarRegionType::Memory32BitRegion) ); assert_eq!(cfg.get_bar_addr(0), 0x12345670); - assert_eq!(cfg.writable_bits[BAR0_REG], 0xFFFFFFFC); + assert_eq!(cfg.writable_bits[BAR0_REG], 0xFFFFFFF0); } #[test] @@ -788,4 +800,60 @@ mod tests { assert_eq!(cfg.get_bar_addr(0), 0x1230); assert_eq!(cfg.writable_bits[BAR0_REG], 0xFFFFFFFC); } + + #[test] + fn add_pci_bar_invalid_size() { + let mut cfg = PciConfiguration::new( + 0x1234, + 0x5678, + PciClassCode::MultimediaController, + &PciMultimediaSubclass::AudioController, + Some(&TestPI::Test), + PciHeaderType::Device, + 0xABCD, + 0x2468, + ); + + // I/O BAR with size 2 (too small) + assert_eq!( + cfg.add_pci_bar( + &PciBarConfiguration::new( + 0, + 0x2, + PciBarRegionType::IORegion, + PciBarPrefetchable::NotPrefetchable, + ) + .set_address(0x1230), + ), + Err(Error::BarSizeInvalid(0x2)) + ); + + // I/O BAR with size 3 (not a power of 2) + assert_eq!( + cfg.add_pci_bar( + &PciBarConfiguration::new( + 0, + 0x3, + PciBarRegionType::IORegion, + PciBarPrefetchable::NotPrefetchable, + ) + .set_address(0x1230), + ), + Err(Error::BarSizeInvalid(0x3)) + ); + + // Memory BAR with size 8 (too small) + assert_eq!( + cfg.add_pci_bar( + &PciBarConfiguration::new( + 0, + 0x8, + PciBarRegionType::Memory32BitRegion, + PciBarPrefetchable::NotPrefetchable, + ) + .set_address(0x12345670), + ), + Err(Error::BarSizeInvalid(0x8)) + ); + } }