diff --git a/block/src/formats/qcow/internal/metadata.rs b/block/src/formats/qcow/internal/metadata.rs index 40898e08a..65dd673ce 100644 --- a/block/src/formats/qcow/internal/metadata.rs +++ b/block/src/formats/qcow/internal/metadata.rs @@ -418,8 +418,7 @@ impl QcowState { })) } else { let cluster_addr = l2_entry_std_cluster_addr(l2_entry); - let cluster_size = self.raw_file.cluster_size(); - if cluster_addr & (cluster_size - 1) != 0 { + if !self.is_refcount_addressable_cluster_offset(cluster_addr) { // Fall through to write lock path which sets the corrupt bit return Ok(None); } @@ -476,11 +475,7 @@ impl QcowState { }) } else { let cluster_addr = l2_entry_std_cluster_addr(l2_entry); - let cluster_size = self.raw_file.cluster_size(); - if cluster_addr & (cluster_size - 1) != 0 { - self.set_corrupt_bit_best_effort(); - return Err(io::Error::from_raw_os_error(EIO)); - } + self.reject_invalid_cluster_offset(cluster_addr)?; let intra_offset = self.raw_file.cluster_offset(address); Ok(ClusterReadMapping::Allocated { offset: cluster_addr + intra_offset, @@ -567,12 +562,8 @@ impl QcowState { self.update_cluster_addr(l1_index, l2_index, cluster_addr, &mut set_refcounts)?; cluster_addr } else { - // Already allocated - validate alignment let cluster_addr = l2_entry_std_cluster_addr(l2_entry); - if cluster_addr & (self.raw_file.cluster_size() - 1) != 0 { - self.set_corrupt_bit_best_effort(); - return Err(io::Error::from_raw_os_error(EIO)); - } + self.reject_invalid_cluster_offset(cluster_addr)?; cluster_addr }; @@ -597,16 +588,26 @@ impl QcowState { (address / self.raw_file.cluster_size()) % self.l2_entries } + fn is_refcount_addressable_cluster_offset(&self, cluster_addr: u64) -> bool { + cluster_addr & (self.raw_file.cluster_size() - 1) == 0 + && cluster_addr <= self.refcounts.max_valid_cluster_offset() + } + + fn reject_invalid_cluster_offset(&mut self, cluster_addr: u64) -> io::Result<()> { + if self.is_refcount_addressable_cluster_offset(cluster_addr) { + Ok(()) + } else { + self.set_corrupt_bit_best_effort(); + Err(io::Error::from_raw_os_error(EIO)) + } + } + // -- Cache and allocation operations requiring exclusive access -- /// Populates the L2 cache for read operations without allocation. fn cache_l2_cluster(&mut self, l1_index: usize, l2_addr_disk: u64) -> io::Result<()> { if !self.l2_cache.contains_key(l1_index) { - let cluster_size = self.raw_file.cluster_size(); - if l2_addr_disk & (cluster_size - 1) != 0 { - self.set_corrupt_bit_best_effort(); - return Err(io::Error::from_raw_os_error(EIO)); - } + self.reject_invalid_cluster_offset(l2_addr_disk)?; let l2_table = VecCache::from_vec(self.raw_file.read_pointer_cluster(l2_addr_disk, None)?); let l1_table = &self.l1_table; @@ -634,11 +635,7 @@ impl QcowState { self.l1_table[l1_index] = new_addr; VecCache::new(self.l2_entries as usize) } else { - let cluster_size = self.raw_file.cluster_size(); - if l2_addr_disk & (cluster_size - 1) != 0 { - self.set_corrupt_bit_best_effort(); - return Err(io::Error::from_raw_os_error(EIO)); - } + self.reject_invalid_cluster_offset(l2_addr_disk)?; VecCache::from_vec(self.raw_file.read_pointer_cluster(l2_addr_disk, None)?) }; let l1_table = &self.l1_table; @@ -894,6 +891,7 @@ impl QcowState { } let cluster_addr = l2_entry_std_cluster_addr(l2_entry); + self.reject_invalid_cluster_offset(cluster_addr)?; let refcount = self .refcounts .get_cluster_refcount(&mut self.raw_file, cluster_addr) diff --git a/block/src/formats/qcow/worker/sync.rs b/block/src/formats/qcow/worker/sync.rs index 219d598b8..304b8b256 100644 --- a/block/src/formats/qcow/worker/sync.rs +++ b/block/src/formats/qcow/worker/sync.rs @@ -359,8 +359,10 @@ mod unit_tests { const TEST_L1_L2_ADDR_MASK: u64 = 0x00ff_ffff_ffff_fe00; const TEST_HEADER_L1_TABLE_OFFSET: u64 = 40; + const TEST_CLUSTER_USED_FLAG: u64 = 1 << 63; const TEST_COMPRESSED_FLAG: u64 = 1 << 62; const TEST_ZERO_FLAG: u64 = 1; + const TEST_OUT_OF_BOUNDS_CLUSTER: u64 = 0x0000_0001_4000_0000; fn read_be_u64_at(file: &mut File, offset: u64) -> u64 { let mut bytes = [0u8; 8]; @@ -390,11 +392,23 @@ mod unit_tests { file.sync_all().unwrap(); } + fn set_first_l2_entry(file: &mut File, l2_entry: u64) { + let l2_entry_offset = first_l2_entry_offset(file); + write_be_u64_at(file, l2_entry_offset, l2_entry); + file.sync_all().unwrap(); + } + fn first_l2_entry(file: &mut File) -> u64 { let l2_entry_offset = first_l2_entry_offset(file); read_be_u64_at(file, l2_entry_offset) } + fn qcow_header_is_corrupt(file: &File) -> bool { + let mut raw = RawFile::new(file.try_clone().unwrap(), false); + raw.seek(SeekFrom::Start(0)).unwrap(); + QcowHeader::new(&mut raw).unwrap().is_corrupt() + } + fn create_disk_with_data( file_size: u64, data: &[u8], @@ -495,6 +509,59 @@ mod unit_tests { assert_eq!(result as usize, data.len()); } + #[test] + fn test_qcow_sync_rejects_out_of_bounds_allocated_l2_entry_on_read() { + let data = vec![0x5a; 4096]; + let (temp_file, disk) = create_disk_with_data(100 * 1024 * 1024, &data, 0, true, false); + let mut file = temp_file.as_file().try_clone().unwrap(); + + set_first_l2_entry( + &mut file, + TEST_CLUSTER_USED_FLAG | TEST_OUT_OF_BOUNDS_CLUSTER, + ); + + let mut async_io = disk.create_async_io(1).unwrap(); + let err = async_io + .read_to_vec(0, OwnedIoBuffer::from_vec(vec![0u8; 512]), 1) + .expect_err("out-of-bounds allocated L2 entry must fail"); + + match err { + AsyncIoError::ReadVectored(e) => assert_eq!(e.raw_os_error(), Some(libc::EIO)), + other => panic!("unexpected error: {other:?}"), + } + assert!( + qcow_header_is_corrupt(&file), + "out-of-bounds allocated L2 entry should set the corrupt bit" + ); + } + + #[test] + fn test_qcow_sync_rejects_out_of_bounds_allocated_l2_entry_on_write() { + let data = vec![0x5a; 4096]; + let (temp_file, disk) = create_disk_with_data(100 * 1024 * 1024, &data, 0, true, false); + let mut file = temp_file.as_file().try_clone().unwrap(); + + set_first_l2_entry( + &mut file, + TEST_CLUSTER_USED_FLAG | TEST_OUT_OF_BOUNDS_CLUSTER, + ); + + let mut async_io = disk.create_async_io(1).unwrap(); + let overwrite = vec![0x11u8; 512]; + let err = async_io + .write_from_vec(0, OwnedIoBuffer::from_vec(overwrite), 1) + .expect_err("out-of-bounds allocated L2 entry must fail"); + + match err { + AsyncIoError::WriteVectored(e) => assert_eq!(e.raw_os_error(), Some(libc::EIO)), + other => panic!("unexpected error: {other:?}"), + } + assert!( + qcow_header_is_corrupt(&file), + "out-of-bounds allocated L2 entry should set the corrupt bit" + ); + } + #[test] fn test_qcow_async_punch_hole_completion() { let data = vec![0xDD; 128 * 1024];