Skip to content

Commit 717f593

Browse files
committed
fix(snapshot): add error handling in PCI transport restore code
There are a few places in the PCI transport snapshot restore code where Vecs (serialised in snapshot) are converted to fixed-size arrays (used at runtime). If a bad snapshot was supplied as input with wrong-sized vecs, Firecracker would panic. Add some error handling to prevent such panics. Signed-off-by: James Curtis <jxcurtis@amazon.co.uk>
1 parent 5d697c0 commit 717f593

2 files changed

Lines changed: 46 additions & 9 deletions

File tree

src/vmm/src/devices/virtio/transport/pci/device.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ use crate::devices::virtio::transport::pci::common_config::{
3232
use crate::devices::virtio::transport::pci::device_status::*;
3333
use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType};
3434
use crate::logger::{debug, error, warn};
35-
use crate::pci::configuration::{PciCapability, PciConfiguration, PciConfigurationState};
35+
use crate::pci::configuration::{
36+
PciCapability, PciConfiguration, PciConfigurationError, PciConfigurationState,
37+
};
3638
use crate::pci::msix::{MsixCap, MsixConfig, MsixConfigState};
3739
use crate::pci::{
3840
BarReprogrammingParams, DeviceRelocationError, PciCapabilityId, PciClassCode, PciDevice,
@@ -239,6 +241,8 @@ pub enum VirtioPciDeviceError {
239241
CreateVirtioPciDevice(#[from] DeviceRelocationError),
240242
/// Error creating MSI configuration: {0}
241243
Msi(#[from] InterruptError),
244+
/// Invalid PCI configuration state: {0}
245+
PciConfiguration(#[from] PciConfigurationError),
242246
}
243247

244248
pub struct VirtioPciDevice {
@@ -412,11 +416,13 @@ impl VirtioPciDevice {
412416
let pci_config = PciConfiguration::type0_from_state(
413417
state.pci_configuration_state,
414418
Some(msix_config.clone()),
415-
);
419+
)?;
416420
let virtio_common_config = VirtioPciCommonConfig::new(state.pci_dev_state);
417421
let cap_pci_cfg_info = VirtioPciCfgCapInfo {
418422
offset: state.cap_pci_cfg_offset,
419-
cap: *VirtioPciCfgCap::from_slice(&state.cap_pci_cfg).unwrap(),
423+
cap: *VirtioPciCfgCap::from_slice(&state.cap_pci_cfg).ok_or(
424+
PciConfigurationError::InvalidCapPciCfgLength(state.cap_pci_cfg.len()),
425+
)?,
420426
};
421427

422428
let interrupt = Arc::new(VirtioInterruptMsix::new(

src/vmm/src/pci/configuration.rs

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,19 @@ use super::msix::MsixConfig;
1515
use crate::logger::{info, warn};
1616
use crate::pci::{PciCapabilityId, PciClassCode};
1717

18+
/// Errors from restoring PCI configuration state.
19+
#[derive(Debug, thiserror::Error, displaydoc::Display)]
20+
pub enum PciConfigurationError {
21+
/// Invalid registers length: {0}
22+
InvalidRegistersLength(usize),
23+
/// Invalid writable_bits length: {0}
24+
InvalidWritableBitsLength(usize),
25+
/// Invalid bars length: {0}
26+
InvalidBarsLength(usize),
27+
/// Invalid cap_pci_cfg length: {0}
28+
InvalidCapPciCfgLength(usize),
29+
}
30+
1831
// The number of 32bit registers in the config space, 4096 bytes.
1932
const NUM_CONFIGURATION_REGISTERS: usize = 1024;
2033

@@ -129,15 +142,33 @@ impl PciConfiguration {
129142
pub fn type0_from_state(
130143
state: PciConfigurationState,
131144
msix_config: Option<Arc<Mutex<MsixConfig>>>,
132-
) -> Self {
133-
PciConfiguration {
134-
registers: state.registers.try_into().unwrap(),
135-
writable_bits: state.writable_bits.try_into().unwrap(),
136-
bars: state.bars.try_into().unwrap(),
145+
) -> Result<Self, PciConfigurationError> {
146+
let reg_len = state.registers.len();
147+
let registers = state
148+
.registers
149+
.try_into()
150+
.map_err(|_| PciConfigurationError::InvalidRegistersLength(reg_len))?;
151+
152+
let wb_len = state.writable_bits.len();
153+
let writable_bits = state
154+
.writable_bits
155+
.try_into()
156+
.map_err(|_| PciConfigurationError::InvalidWritableBitsLength(wb_len))?;
157+
158+
let bar_len = state.bars.len();
159+
let bars = state
160+
.bars
161+
.try_into()
162+
.map_err(|_| PciConfigurationError::InvalidBarsLength(bar_len))?;
163+
164+
Ok(PciConfiguration {
165+
registers,
166+
writable_bits,
167+
bars,
137168
last_capability: state.last_capability,
138169
msix_cap_reg_idx: state.msix_cap_reg_idx,
139170
msix_config,
140-
}
171+
})
141172
}
142173

143174
/// Create PCI configuration space state

0 commit comments

Comments
 (0)