vmm: Prevent "internal" identifiers being used by user

For devices that cannot be named by the user use the "__" prefix to
identify them as internal devices. Check that any identifiers provided
in the config do not clash with those internal names. This prevents the
user from creating a disk such as "__serial" which would then cause a
failure in unpredictable manner.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>

vmm/src/config.rs

@@ -172,6 +172,8 @@ pub enum ValidationError {
     IommuNotSupported(u16),
     // Identifier is not unique
     IdentifierNotUnique(String),
+    /// Invalid identifier
+    InvalidIdentifier(String),
 }
 
 type ValidationResult<T> = std::result::Result<T, ValidationError>;
@@ -261,6 +263,9 @@ impl fmt::Display for ValidationError {
             IdentifierNotUnique(s) => {
                 write!(f, "Identifier {} is not unique", s)
             }
+            InvalidIdentifier(s) => {
+                write!(f, "Identifier {} is invalid", s)
+            }
         }
     }
 }
@@ -2328,6 +2333,23 @@ pub struct VmConfig {
 }
 
 impl VmConfig {
+    fn validate_identifier(
+        id_list: &mut BTreeSet<String>,
+        id: &Option<String>,
+    ) -> ValidationResult<()> {
+        if let Some(id) = id.as_ref() {
+            if id.starts_with("__") {
+                return Err(ValidationError::InvalidIdentifier(id.clone()));
+            }
+
+            if !id_list.insert(id.clone()) {
+                return Err(ValidationError::IdentifierNotUnique(id.clone()));
+            }
+        }
+
+        Ok(())
+    }
+
     // Also enables virtio-iommu if the config needs it
     // Returns the list of unique identifiers provided through the
     // configuration.
@@ -2379,11 +2401,7 @@ impl VmConfig {
                 disk.validate(self)?;
                 self.iommu |= disk.iommu;
 
-                if let Some(id) = disk.id.clone() {
+                Self::validate_identifier(&mut id_list, &disk.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2395,11 +2413,7 @@ impl VmConfig {
                 net.validate(self)?;
                 self.iommu |= net.iommu;
 
-                if let Some(id) = net.id.clone() {
+                Self::validate_identifier(&mut id_list, &net.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2410,11 +2424,7 @@ impl VmConfig {
             for fs in fses {
                 fs.validate(self)?;
 
-                if let Some(id) = fs.id.clone() {
+                Self::validate_identifier(&mut id_list, &fs.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2423,11 +2433,7 @@ impl VmConfig {
                 pmem.validate(self)?;
                 self.iommu |= pmem.iommu;
 
-                if let Some(id) = pmem.id.clone() {
+                Self::validate_identifier(&mut id_list, &pmem.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2474,11 +2480,7 @@ impl VmConfig {
             for user_device in user_devices {
                 user_device.validate(self)?;
 
-                if let Some(id) = user_device.id.clone() {
+                Self::validate_identifier(&mut id_list, &user_device.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2487,11 +2489,7 @@ impl VmConfig {
                 vdpa_device.validate(self)?;
                 self.iommu |= vdpa_device.iommu;
 
-                if let Some(id) = vdpa_device.id.clone() {
+                Self::validate_identifier(&mut id_list, &vdpa_device.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2517,11 +2515,7 @@ impl VmConfig {
                 device.validate(self)?;
                 self.iommu |= device.iommu;
 
-                if let Some(id) = device.id.clone() {
+                Self::validate_identifier(&mut id_list, &device.id)?;
-                    if !id_list.insert(id.clone()) {
-                        return Err(ValidationError::IdentifierNotUnique(id));
-                    }
-                }
             }
         }
 
@@ -2529,11 +2523,7 @@ impl VmConfig {
             vsock.validate(self)?;
             self.iommu |= vsock.iommu;
 
-            if let Some(id) = vsock.id.clone() {
+            Self::validate_identifier(&mut id_list, &vsock.id)?;
-                if !id_list.insert(id.clone()) {
-                    return Err(ValidationError::IdentifierNotUnique(id));
-                }
-            }
         }
 
         if let Some(numa) = &self.numa {
@@ -2557,9 +2547,7 @@ impl VmConfig {
         if let Some(zones) = &self.memory.zones {
             for zone in zones.iter() {
                 let id = zone.id.clone();
-                if !id_list.insert(id.clone()) {
+                Self::validate_identifier(&mut id_list, &Some(id))?;
-                    return Err(ValidationError::IdentifierNotUnique(id));
-                }
             }
         }
 
@@ -2567,9 +2555,7 @@ impl VmConfig {
         if let Some(sgx_epcs) = &self.sgx_epc {
             for sgx_epc in sgx_epcs.iter() {
                 let id = sgx_epc.id.clone();
-                if !id_list.insert(id.clone()) {
+                Self::validate_identifier(&mut id_list, &Some(id))?;
-                    return Err(ValidationError::IdentifierNotUnique(id));
-                }
             }
         }
 

vmm/src/device_manager.rs

@@ -115,30 +115,28 @@ use vmm_sys_util::eventfd::EventFd;
 #[cfg(target_arch = "aarch64")]
 const MMIO_LEN: u64 = 0x1000;
 
-const VFIO_DEVICE_NAME_PREFIX: &str = "_vfio";
+// Singleton devices / devices the user cannot name
-
-const VFIO_USER_DEVICE_NAME_PREFIX: &str = "_vfio_user";
-
 #[cfg(target_arch = "x86_64")]
-const IOAPIC_DEVICE_NAME: &str = "_ioapic";
+const IOAPIC_DEVICE_NAME: &str = "__ioapic";
-
+const SERIAL_DEVICE_NAME: &str = "__serial";
-const SERIAL_DEVICE_NAME_PREFIX: &str = "_serial";
 #[cfg(target_arch = "aarch64")]
-const GPIO_DEVICE_NAME_PREFIX: &str = "_gpio";
+const GPIO_DEVICE_NAME: &str = "__gpio";
+const RNG_DEVICE_NAME: &str = "__rng";
+const IOMMU_DEVICE_NAME: &str = "__iommu";
+const BALLOON_DEVICE_NAME: &str = "__balloon";
+const CONSOLE_DEVICE_NAME: &str = "__console";
 
-const CONSOLE_DEVICE_NAME: &str = "_console";
+// Devices that the user may name and for which we generate
+// identifiers if the user doesn't give one
 const DISK_DEVICE_NAME_PREFIX: &str = "_disk";
 const FS_DEVICE_NAME_PREFIX: &str = "_fs";
-const BALLOON_DEVICE_NAME: &str = "_balloon";
 const NET_DEVICE_NAME_PREFIX: &str = "_net";
 const PMEM_DEVICE_NAME_PREFIX: &str = "_pmem";
-const RNG_DEVICE_NAME: &str = "_rng";
 const VDPA_DEVICE_NAME_PREFIX: &str = "_vdpa";
 const VSOCK_DEVICE_NAME_PREFIX: &str = "_vsock";
-const WATCHDOG_DEVICE_NAME: &str = "_watchdog";
+const WATCHDOG_DEVICE_NAME: &str = "__watchdog";
-
+const VFIO_DEVICE_NAME_PREFIX: &str = "_vfio";
-const IOMMU_DEVICE_NAME: &str = "_iommu";
+const VFIO_USER_DEVICE_NAME_PREFIX: &str = "_vfio_user";
-
 const VIRTIO_PCI_DEVICE_NAME_PREFIX: &str = "_virtio-pci";
 
 /// Errors associated with device manager
@@ -480,6 +478,9 @@ pub enum DeviceManagerError {
 
     /// Invalid identifier as it is not unique.
     IdentifierNotUnique(String),
+
+    /// Invalid identifier
+    InvalidIdentifier(String),
 }
 pub type DeviceManagerResult<T> = result::Result<T, DeviceManagerError>;
 
@@ -1565,7 +1566,7 @@ impl DeviceManager {
         );
 
         // Add a GPIO device
-        let id = String::from(GPIO_DEVICE_NAME_PREFIX);
+        let id = String::from(GPIO_DEVICE_NAME);
         let gpio_irq = self
             .address_manager
             .allocator
@@ -1655,7 +1656,7 @@ impl DeviceManager {
         // Serial is tied to IRQ #4
         let serial_irq = 4;
 
-        let id = String::from(SERIAL_DEVICE_NAME_PREFIX);
+        let id = String::from(SERIAL_DEVICE_NAME);
 
         let interrupt_group = interrupt_manager
             .create_group(LegacyIrqGroupConfig {
@@ -1701,7 +1702,7 @@ impl DeviceManager {
         interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>,
         serial_writer: Option<Box<dyn io::Write + Send>>,
     ) -> DeviceManagerResult<Arc<Mutex<Pl011>>> {
-        let id = String::from(SERIAL_DEVICE_NAME_PREFIX);
+        let id = String::from(SERIAL_DEVICE_NAME);
 
         let serial_irq = self
             .address_manager
@@ -4225,6 +4226,10 @@ impl DeviceManager {
 
     fn validate_identifier(&self, id: &Option<String>) -> DeviceManagerResult<()> {
         if let Some(id) = id {
+            if id.starts_with("__") {
+                return Err(DeviceManagerError::InvalidIdentifier(id.clone()));
+            }
+
             if self.device_tree.lock().unwrap().contains_key(id) {
                 return Err(DeviceManagerError::IdentifierNotUnique(id.clone()));
             }