kata-containers/cloud-hypervisor
1
0
Fork 0
mirror of https://github.com/cloud-hypervisor/cloud-hypervisor.git synced 2026-08-05 02:19:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

block: vhdx: Use zerocopy for structs

Browse Source 
Replace the use of unsafe struct casting with zerocopy trait derivation.
This fixes a Rust UB where the struct was being filled with a slice of
length greater than the size of the struct.

As a compromise the guid handling was changed to handle the uuids as
opaque bytes as they are mixed endian. This has no impact on the
functionality as they are only used for comparison and has the positive
impact of reducing some of the uuid handling complexity.

Assisted-by: Claude:Opus-4.8
Signed-off-by: Rob Bradford <rbradford@meta.com>
This commit is contained in:
Rob Bradford
2026-07-01 09:45:08 +01:00
parent f5f7b092e1
commit f62e2615a9
5 changed files with 140 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
block/Cargo.toml
View File

@@ -32,6 +32,7 @@ vm-memory = { workspace = true, features = [
] }
vm-virtio = { path = "../vm-virtio" }
vmm-sys-util = { workspace = true }
zerocopy = { workspace = true, features = ["derive"] }
zstd = "0.13"
[dev-dependencies]

139
block/src/formats/vhdx/internal/header.rs
View File

@@ -4,12 +4,13 @@
use std::collections::btree_map::BTreeMap;
use std::os::unix::fs::FileExt;
use std::{io, result, slice};
use std::{io, result};
use byteorder::{ByteOrder, LittleEndian};
use remain::sorted;
use thiserror::Error;
use uuid::Uuid;
use zerocopy::{FromBytes, Immutable, IntoBytes};
use crate::aligned_file::AlignedFile;
@@ -28,8 +29,13 @@ const REGION_SIZE: u64 = 64 * 1024; // Each region size is 64 KiB
const REGION_ENTRY_REQUIRED: u32 = 1;
const BAT_GUID: &str = "2DC27766-F623-4200-9D64-115E9BFD4A08"; // BAT GUID
const MDR_GUID: &str = "8B7CA206-4790-4B9A-B8FE-575F050F886E"; // Metadata GUID
// VHDX stores GUIDs using little-endian GUID byte order.
const BAT_GUID: [u8; 16] = [
0x66, 0x77, 0xc2, 0x2d, 0x23, 0xf6, 0x00, 0x42, 0x9d, 0x64, 0x11, 0x5e, 0x9b, 0xfd, 0x4a, 0x08,
];
const MDR_GUID: [u8; 16] = [
0x06, 0xa2, 0x7c, 0x8b, 0x90, 0x47, 0x9a, 0x4b, 0xb8, 0xfe, 0x57, 0x5f, 0x05, 0x0f, 0x88, 0x6e,
];
#[sorted]
#[derive(Error, Debug)]
@@ -48,8 +54,6 @@ pub enum VhdxHeaderError {
InvalidHeaderSign,
#[error("Not a valid VHDx region")]
InvalidRegionSign,
#[error("Couldn't parse Uuid for region entry {0}")]
InvalidUuid(#[source] uuid::Error),
#[error("Not a VHDx file")]
InvalidVHDXSign,
#[error("No valid header found")]
@@ -101,7 +105,7 @@ impl FileTypeIdentifier {
}
#[repr(C, packed)]
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes, Immutable, IntoBytes)]
pub struct Header {
pub signature: u32,
pub checksum: u32,
@@ -124,8 +128,7 @@ impl Header {
f.read_exact_at(&mut buffer, start)
.map_err(VhdxHeaderError::ReadHeader)?;
// SAFETY: buffer is of correct size and has been successfully filled.
let header: Header = unsafe { *(buffer.as_ptr().cast()) };
let header = Header::read_from_prefix(&buffer).unwrap().0;
if header.signature != HEADER_SIGN {
return Err(VhdxHeaderError::InvalidHeaderSign);
}
@@ -138,32 +141,26 @@ impl Header {
Ok(header)
}
/// Converts the header structure into a buffer
fn write_to_buffer(&self, buffer: &mut [u8; HEADER_SIZE as usize]) {
// SAFETY: self is a valid header.
let reference =
unsafe { slice::from_raw_parts((&raw const *self).cast(), HEADER_SIZE as usize) };
*buffer = reference.try_into().unwrap();
}
/// Creates and returns new updated header from the provided current header
fn update_header(
f: &AlignedFile,
current_header: &Header,
change_data_guid: bool,
mut file_write_guid: u128,
file_write_guid: u128,
start: u64,
) -> Result<Header> {
let mut buffer = [0u8; HEADER_SIZE as usize];
let mut data_write_guid = current_header.data_write_guid;
let data_write_guid = if change_data_guid {
Uuid::new_v4().as_u128()
} else {
current_header.data_write_guid
};
if change_data_guid {
data_write_guid = Uuid::new_v4().as_u128();
}
if file_write_guid == 0 {
file_write_guid = current_header.file_write_guid;
}
let file_write_guid = if file_write_guid == 0 {
current_header.file_write_guid
} else {
file_write_guid
};
let mut new_header = Header {
signature: current_header.signature,
@@ -178,9 +175,9 @@ impl Header {
log_offset: current_header.log_offset,
};
new_header.write_to_buffer(&mut buffer);
new_header.write_to_prefix(&mut buffer).unwrap();
new_header.checksum = calculate_checksum(&mut buffer, size_of::<u32>());
new_header.write_to_buffer(&mut buffer);
new_header.write_to_prefix(&mut buffer).unwrap();
f.write_all_at(&buffer, start)
.map_err(VhdxHeaderError::WriteHeader)?;
@@ -190,7 +187,7 @@ impl Header {
}
#[repr(C, packed)]
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes)]
struct RegionTableHeader {
pub signature: u32,
pub checksum: u32,
@@ -207,8 +204,7 @@ impl RegionTableHeader {
f.read_exact_at(&mut buffer, start)
.map_err(VhdxHeaderError::ReadRegionTableHeader)?;
// SAFETY: buffer is of correct size and has been successfully filled.
let region_table_header: RegionTableHeader = unsafe { *(buffer.as_ptr().cast()) };
let region_table_header = RegionTableHeader::read_from_prefix(&buffer).unwrap().0;
if region_table_header.signature != REGION_SIGN {
return Err(VhdxHeaderError::InvalidRegionSign);
}
@@ -261,8 +257,10 @@ impl RegionInfo {
.map_err(VhdxHeaderError::ReadRegionTableEntries)?;
for _ in 0..entry_count {
let entry =
RegionTableEntry::new(&buffer[offset..offset + size_of::<RegionTableEntry>()])?;
let entry = RegionTableEntry::read_from_bytes(
&buffer[offset..offset + size_of::<RegionTableEntry>()],
)
.unwrap();
offset += size_of::<RegionTableEntry>();
let start = entry.file_offset;
@@ -276,7 +274,7 @@ impl RegionInfo {
region_entries.insert(entry.file_offset, entry.file_offset + entry.length as u64);
if entry.guid == Uuid::parse_str(BAT_GUID).map_err(VhdxHeaderError::InvalidUuid)? {
if entry.guid == BAT_GUID {
if bat_entry.is_none() {
bat_entry = Some(entry);
continue;
@@ -284,7 +282,7 @@ impl RegionInfo {
return Err(VhdxHeaderError::DuplicateBATEntry);
}
if entry.guid == Uuid::parse_str(MDR_GUID).map_err(VhdxHeaderError::InvalidUuid)? {
if entry.guid == MDR_GUID {
if mdr_entry.is_none() {
mdr_entry = Some(entry);
continue;
@@ -318,28 +316,14 @@ impl RegionInfo {
}
#[repr(C, packed)]
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, FromBytes)]
pub struct RegionTableEntry {
pub guid: Uuid,
guid: [u8; 16],
pub file_offset: u64,
pub length: u32,
pub required: u32,
}
impl RegionTableEntry {
/// Reads one Region Entry from a Region Table index that starts from 0
pub fn new(buffer: &[u8]) -> Result<RegionTableEntry> {
assert!(buffer.len() == size_of::<RegionTableEntry>());
// SAFETY: the assertion above makes sure the buffer size is correct.
let mut region_table_entry: RegionTableEntry = unsafe { *(buffer.as_ptr().cast()) };
let uuid = super::uuid_from_guid(buffer);
region_table_entry.guid = uuid;
Ok(region_table_entry)
}
}
enum HeaderNo {
First,
Second,
@@ -464,13 +448,49 @@ mod tests {
use std::os::unix::fs::FileExt;
use vmm_sys_util::tempfile::TempFile;
use zerocopy::{FromBytes, IntoBytes};
use super::{
BAT_GUID, MDR_GUID, REGION_TABLE_1_START, RegionInfo, RegionTableHeader, VhdxHeaderError,
ranges_overlap,
BAT_GUID, HEADER_SIGN, Header, MDR_GUID, REGION_TABLE_1_START, RegionInfo,
RegionTableHeader, VhdxHeaderError, ranges_overlap,
};
use crate::aligned_file::AlignedFile;
#[test]
fn test_header_bytes_round_trip() {
let header = Header {
signature: HEADER_SIGN,
checksum: 0x1122_3344,
sequence_number: 0x0102_0304_0506_0708,
file_write_guid: 0x0f0e_0d0c_0b0a_0908_0706_0504_0302_0100,
data_write_guid: 0x1f1e_1d1c_1b1a_1918_1716_1514_1312_1110,
log_guid: 0x2f2e_2d2c_2b2a_2928_2726_2524_2322_2120,
log_version: 0xabcd,
version: 0x0001,
log_length: 0x0010_0000,
log_offset: 0x0000_0100_0000_0000,
};
let bytes = header.as_bytes();
assert_eq!(&bytes[0..4], &header.signature.to_le_bytes()[..]);
assert_eq!(&bytes[8..16], &header.sequence_number.to_le_bytes()[..]);
assert_eq!(&bytes[16..32], &header.file_write_guid.to_le_bytes()[..]);
assert_eq!(&bytes[64..66], &header.log_version.to_le_bytes()[..]);
assert_eq!(&bytes[72..80], &header.log_offset.to_le_bytes()[..]);
let parsed = Header::read_from_bytes(bytes).unwrap();
assert_eq!({ parsed.signature }, { header.signature });
assert_eq!({ parsed.checksum }, { header.checksum });
assert_eq!({ parsed.sequence_number }, { header.sequence_number });
assert_eq!({ parsed.file_write_guid }, { header.file_write_guid });
assert_eq!({ parsed.data_write_guid }, { header.data_write_guid });
assert_eq!({ parsed.log_guid }, { header.log_guid });
assert_eq!({ parsed.log_version }, { header.log_version });
assert_eq!({ parsed.version }, { header.version });
assert_eq!({ parsed.log_length }, { header.log_length });
assert_eq!({ parsed.log_offset }, { header.log_offset });
}
#[test]
fn test_ranges_overlap() {
// (new [start,end), existing [s,e), expected overlap)
@@ -502,18 +522,11 @@ mod tests {
}
}
/// Builds the 32-byte on-disk region table entry for `guid_str` describing
/// the region `[file_offset, file_offset + length)`. The GUID encoding is
/// the inverse of `uuid_from_guid` (first three fields big-endian, last
/// eight bytes verbatim).
fn region_entry(guid_str: &str, file_offset: u64, length: u32) -> [u8; 32] {
/// Builds the 32-byte on-disk region table entry for `guid` describing
/// the region `[file_offset, file_offset + length)`.
fn region_entry(guid: [u8; 16], file_offset: u64, length: u32) -> [u8; 32] {
let mut e = [0u8; 32];
let uuid = uuid::Uuid::parse_str(guid_str).unwrap();
let (d1, d2, d3, d4) = uuid.to_fields_le();
e[0..4].copy_from_slice(&d1.to_be_bytes());
e[4..6].copy_from_slice(&d2.to_be_bytes());
e[6..8].copy_from_slice(&d3.to_be_bytes());
e[8..16].copy_from_slice(d4);
e[0..16].copy_from_slice(&guid);
e[16..24].copy_from_slice(&file_offset.to_le_bytes());
e[24..28].copy_from_slice(&length.to_le_bytes());
// `required` (e[28..32]) left zero.

75
block/src/formats/vhdx/internal/metadata.rs
View File

@@ -8,7 +8,7 @@ use std::{io, result};
use byteorder::{ByteOrder, LittleEndian};
use remain::sorted;
use thiserror::Error;
use uuid::Uuid;
use zerocopy::FromBytes;
use super::header::RegionTableEntry;
use crate::aligned_file::AlignedFile;
@@ -28,12 +28,24 @@ const MAX_SECTORS_PER_BLOCK: u64 = 1 << 23;
const BLOCK_HAS_PARENT: u32 = 0x02; // Has a parent or a backing file
// GUID for known metadata items
const METADATA_FILE_PARAMETER: &str = "CAA16737-FA36-4D43-B3B6-33F0AA44E76B";
const METADATA_VIRTUAL_DISK_SIZE: &str = "2FA54224-CD1B-4876-B211-5DBED83BF4B8";
const METADATA_VIRTUAL_DISK_ID: &str = "BECA12AB-B2E6-4523-93EF-C309E000C746";
const METADATA_LOGICAL_SECTOR_SIZE: &str = "8141BF1D-A96F-4709-BA47-F233A8FAAB5F";
const METADATA_PHYSICAL_SECTOR_SIZE: &str = "CDA348C7-445D-4471-9CC9-E9885251C556";
const METADATA_PARENT_LOCATOR: &str = "A8D35F2D-B30B-454D-ABF7-D3D84834AB0C";
const METADATA_FILE_PARAMETER: [u8; 16] = [
0x37, 0x67, 0xa1, 0xca, 0x36, 0xfa, 0x43, 0x4d, 0xb3, 0xb6, 0x33, 0xf0, 0xaa, 0x44, 0xe7, 0x6b,
];
const METADATA_VIRTUAL_DISK_SIZE: [u8; 16] = [
0x24, 0x42, 0xa5, 0x2f, 0x1b, 0xcd, 0x76, 0x48, 0xb2, 0x11, 0x5d, 0xbe, 0xd8, 0x3b, 0xf4, 0xb8,
];
const METADATA_VIRTUAL_DISK_ID: [u8; 16] = [
0xab, 0x12, 0xca, 0xbe, 0xe6, 0xb2, 0x23, 0x45, 0x93, 0xef, 0xc3, 0x09, 0xe0, 0x00, 0xc7, 0x46,
];
const METADATA_LOGICAL_SECTOR_SIZE: [u8; 16] = [
0x1d, 0xbf, 0x41, 0x81, 0x6f, 0xa9, 0x09, 0x47, 0xba, 0x47, 0xf2, 0x33, 0xa8, 0xfa, 0xab, 0x5f,
];
const METADATA_PHYSICAL_SECTOR_SIZE: [u8; 16] = [
0xc7, 0x48, 0xa3, 0xcd, 0x5d, 0x44, 0x71, 0x44, 0x9c, 0xc9, 0xe9, 0x88, 0x52, 0x51, 0xc5, 0x56,
];
const METADATA_PARENT_LOCATOR: [u8; 16] = [
0x2d, 0x5f, 0xd3, 0xa8, 0x0b, 0xb3, 0x4d, 0x45, 0xab, 0xf7, 0xd3, 0xd8, 0x48, 0x34, 0xab, 0x0c,
];
const METADATA_FILE_PARAMETER_PRESENT: u16 = 0x01;
const METADATA_VIRTUAL_DISK_SIZE_PRESENT: u16 = 0x02;
@@ -69,8 +81,6 @@ pub enum VhdxMetadataError {
InvalidMetadataSign,
#[error("Invalid physical sector size")]
InvalidPhysicalSectorSize,
#[error("Invalid UUID")]
InvalidUuid(#[source] uuid::Error),
#[error("Invalid value")]
InvalidValue,
#[error("Not all required metadata found")]
@@ -126,10 +136,7 @@ impl DiskSpec {
let item_offset = metadata_region.file_offset + metadata_entry.offset as u64;
if metadata_entry.item_id
== Uuid::parse_str(METADATA_FILE_PARAMETER)
.map_err(VhdxMetadataError::InvalidUuid)?
{
if metadata_entry.item_id == METADATA_FILE_PARAMETER {
let mut item = [0u8; 2 * size_of::<u32>()];
f.read_exact_at(&mut item, item_offset)
.map_err(VhdxMetadataError::ReadMetadata)?;
@@ -149,30 +156,21 @@ impl DiskSpec {
disk_spec.has_parent = bits & BLOCK_HAS_PARENT != 0;
metadata_presence |= METADATA_FILE_PARAMETER_PRESENT;
} else if metadata_entry.item_id
== Uuid::parse_str(METADATA_VIRTUAL_DISK_SIZE)
.map_err(VhdxMetadataError::InvalidUuid)?
{
} else if metadata_entry.item_id == METADATA_VIRTUAL_DISK_SIZE {
let mut item = [0u8; size_of::<u64>()];
f.read_exact_at(&mut item, item_offset)
.map_err(VhdxMetadataError::ReadMetadata)?;
disk_spec.virtual_disk_size = LittleEndian::read_u64(&item);
metadata_presence |= METADATA_VIRTUAL_DISK_SIZE_PRESENT;
} else if metadata_entry.item_id
== Uuid::parse_str(METADATA_VIRTUAL_DISK_ID)
.map_err(VhdxMetadataError::InvalidUuid)?
{
} else if metadata_entry.item_id == METADATA_VIRTUAL_DISK_ID {
let mut item = [0u8; size_of::<u128>()];
f.read_exact_at(&mut item, item_offset)
.map_err(VhdxMetadataError::ReadMetadata)?;
disk_spec.disk_id = LittleEndian::read_u128(&item);
metadata_presence |= METADATA_VIRTUAL_DISK_ID_PRESENT;
} else if metadata_entry.item_id
== Uuid::parse_str(METADATA_LOGICAL_SECTOR_SIZE)
.map_err(VhdxMetadataError::InvalidUuid)?
{
} else if metadata_entry.item_id == METADATA_LOGICAL_SECTOR_SIZE {
let mut item = [0u8; size_of::<u32>()];
f.read_exact_at(&mut item, item_offset)
.map_err(VhdxMetadataError::ReadMetadata)?;
@@ -183,10 +181,7 @@ impl DiskSpec {
}
metadata_presence |= METADATA_LOGICAL_SECTOR_SIZE_PRESENT;
} else if metadata_entry.item_id
== Uuid::parse_str(METADATA_PHYSICAL_SECTOR_SIZE)
.map_err(VhdxMetadataError::InvalidUuid)?
{
} else if metadata_entry.item_id == METADATA_PHYSICAL_SECTOR_SIZE {
let mut item = [0u8; size_of::<u32>()];
f.read_exact_at(&mut item, item_offset)
.map_err(VhdxMetadataError::ReadMetadata)?;
@@ -198,10 +193,7 @@ impl DiskSpec {
}
metadata_presence |= METADATA_PHYSICAL_SECTOR_SIZE_PRESENT;
} else if metadata_entry.item_id
== Uuid::parse_str(METADATA_PARENT_LOCATOR)
.map_err(VhdxMetadataError::InvalidUuid)?
{
} else if metadata_entry.item_id == METADATA_PARENT_LOCATOR {
metadata_presence |= METADATA_PARENT_LOCATOR_PRESENT;
} else {
return Err(VhdxMetadataError::InvalidMetadataItem);
@@ -269,7 +261,7 @@ impl DiskSpec {
}
#[repr(C, packed)]
#[derive(Default, Debug, Clone, Copy)]
#[derive(Default, Debug, Clone, Copy, FromBytes)]
struct MetadataTableHeader {
signature: u64,
reserved: u16,
@@ -279,9 +271,7 @@ struct MetadataTableHeader {
impl MetadataTableHeader {
pub fn new(buffer: &[u8]) -> Result<MetadataTableHeader> {
assert!(buffer.len() == size_of::<MetadataTableHeader>());
// SAFETY: the assertion above makes sure the buffer size is correct.
let metadata_table_header: MetadataTableHeader = unsafe { *(buffer.as_ptr().cast()) };
let metadata_table_header = MetadataTableHeader::read_from_bytes(buffer).unwrap();
if metadata_table_header.signature != METADATA_SIGN {
return Err(VhdxMetadataError::InvalidMetadataSign);
@@ -300,9 +290,9 @@ impl MetadataTableHeader {
}
#[repr(C, packed)]
#[derive(Default, Debug, Clone, Copy)]
#[derive(Default, Debug, Clone, Copy, FromBytes)]
pub struct MetadataTableEntry {
item_id: Uuid,
item_id: [u8; 16],
offset: u32,
length: u32,
flag_bits: u32,
@@ -312,12 +302,7 @@ pub struct MetadataTableEntry {
impl MetadataTableEntry {
/// Parse one metadata entry from the buffer
fn new(buffer: &[u8]) -> Result<MetadataTableEntry> {
assert!(buffer.len() == size_of::<MetadataTableEntry>());
// SAFETY: the assertion above makes sure the buffer size is correct.
let mut metadata_table_entry: MetadataTableEntry = unsafe { *(buffer.as_ptr().cast()) };
let uuid = super::uuid_from_guid(buffer);
metadata_table_entry.item_id = uuid;
let metadata_table_entry = MetadataTableEntry::read_from_bytes(buffer).unwrap();
if metadata_table_entry.length > METADATA_LENGTH_MAX {
return Err(VhdxMetadataError::InvalidMetadataLength);

46
block/src/formats/vhdx/internal/mod.rs
View File

@@ -10,10 +10,8 @@ use std::io::{
use std::os::fd::{AsRawFd, RawFd};
use std::result;
use byteorder::{BigEndian, ByteOrder};
use remain::sorted;
use thiserror::Error;
use uuid::Uuid;
use self::bat::{BatEntry, VhdxBatError};
use self::header::{RegionInfo, RegionTableEntry, VhdxHeader, VhdxHeaderError};
@@ -238,18 +236,6 @@ impl AsRawFd for Vhdx {
}
}
pub(crate) fn uuid_from_guid(buf: &[u8]) -> Uuid {
// The first 3 fields of UUID are stored in Big Endian format, and
// the last 8 bytes are stored as byte array. Therefore, we read the
// first 3 fields in Big Endian format instead of Little Endian.
Uuid::from_fields_le(
BigEndian::read_u32(&buf[0..4]),
BigEndian::read_u16(&buf[4..6]),
BigEndian::read_u16(&buf[6..8]),
buf[8..16].try_into().unwrap(),
)
}
#[cfg(test)]
mod tests {
use std::fs;
@@ -310,4 +296,36 @@ mod tests {
assert_eq!(vhdx.read(&mut readback).unwrap(), readback.len());
assert_eq!(readback, data);
}
#[test]
fn header_update_survives_reopen() {
let Some(tf) = dynamic_vhdx(16) else {
eprintln!("skipping header_update_survives_reopen: qemu-img unavailable");
return;
};
let data = [0xa5u8; 512];
{
let file = fs::OpenOptions::new()
.read(true)
.write(true)
.open(tf.as_path())
.unwrap();
let mut vhdx = Vhdx::new(file, false).unwrap();
vhdx.seek(SeekFrom::Start(0)).unwrap();
assert_eq!(vhdx.write(&data).unwrap(), data.len());
vhdx.flush().unwrap();
}
let file = fs::OpenOptions::new()
.read(true)
.write(true)
.open(tf.as_path())
.unwrap();
let mut vhdx = Vhdx::new(file, false).unwrap();
let mut readback = [0u8; 512];
vhdx.seek(SeekFrom::Start(0)).unwrap();
assert_eq!(vhdx.read(&mut readback).unwrap(), readback.len());
assert_eq!(readback, data);
}
}