From bc144baf0e5bd0662799ab6e716f6585bdbf7fe3 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 8 Nov 2021 13:09:12 +0900 Subject: [PATCH] Handle qcow image backed by a composite disk This CL fixes a bug that was introduced by [1] where max_nesting_depth is used to prevent unbounded nesting of qcow images. When a qcow image is backed by a composite disk, the composite disk is parsed twice: (1) once before the qcow header is created, and (2) once again after the composite disk is written to the header and the header is parsed. The max_nesting_depth was set correctly for (1), but was set to 1 for (2). Since a composite disk inherently is nested, max_nesting_depth drops to 0 and it causes an error. This CL fixes the bug by respecting max_nesting_depth also for the case (2). Bug: N/A Test: launch cuttlefish [1] https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3146214 Change-Id: Ic2d30df6c76a0c1965e222960e0094fe847b1097 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3267734 Auto-Submit: Jiyong Park Tested-by: kokoro Reviewed-by: Andrew Walbran Reviewed-by: Daniel Verkamp Commit-Queue: Keiichi Watanabe --- disk/src/qcow/mod.rs | 52 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/disk/src/qcow/mod.rs b/disk/src/qcow/mod.rs index daacca2ced..ba98e05da7 100644 --- a/disk/src/qcow/mod.rs +++ b/disk/src/qcow/mod.rs @@ -564,7 +564,7 @@ impl QcowFile { /// Creates a new QcowFile at the given path. pub fn new(file: File, virtual_size: u64) -> Result { let header = QcowHeader::create_for_size_and_path(virtual_size, None)?; - QcowFile::new_from_header(file, header) + QcowFile::new_from_header(file, header, 1) } /// Creates a new QcowFile at the given path. @@ -584,16 +584,20 @@ impl QcowFile { .map_err(|e| Error::BackingFileOpen(Box::new(e)))?; let size = backing_file.get_len().map_err(Error::BackingFileIo)?; let header = QcowHeader::create_for_size_and_path(size, Some(backing_file_name))?; - let mut result = QcowFile::new_from_header(file, header)?; + let mut result = QcowFile::new_from_header(file, header, backing_file_max_nesting_depth)?; result.backing_file = Some(backing_file); Ok(result) } - fn new_from_header(mut file: File, header: QcowHeader) -> Result { + fn new_from_header( + mut file: File, + header: QcowHeader, + max_nesting_depth: u32, + ) -> Result { file.seek(SeekFrom::Start(0)).map_err(Error::SeekingFile)?; header.write_to(&mut file)?; - let mut qcow = Self::from(file, 1)?; + let mut qcow = Self::from(file, max_nesting_depth)?; // Set the refcount for each refcount table cluster. let cluster_size = 0x01u64 << qcow.header.cluster_bits; @@ -1765,8 +1769,9 @@ mod tests { use super::*; use crate::MAX_NESTING_DEPTH; use base::WriteZeroes; + use std::fs::OpenOptions; use std::io::{Read, Seek, SeekFrom, Write}; - use tempfile::tempfile; + use tempfile::{tempfile, TempDir}; fn valid_header() -> Vec { vec![ @@ -2691,4 +2696,41 @@ mod tests { .expect("Failed to rebuild recounts."); }); } + + #[test] + fn nested_qcow() { + let tmp_dir = TempDir::new().unwrap(); + + // A file `backing` is backing a qcow file `qcow.l1`, which in turn is backing another + // qcow file. + let backing_file_path = tmp_dir.path().join("backing"); + let _backing_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(&backing_file_path) + .unwrap(); + + let level1_qcow_file_path = tmp_dir.path().join("qcow.l1"); + let level1_qcow_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(&level1_qcow_file_path) + .unwrap(); + let _level1_qcow_file = QcowFile::new_from_backing( + level1_qcow_file, + &backing_file_path.to_str().unwrap(), + 1000, /* allow deep nesting */ + ) + .unwrap(); + + let level2_qcow_file = tempfile().unwrap(); + let _level2_qcow_file = QcowFile::new_from_backing( + level2_qcow_file, + &level1_qcow_file_path.to_str().unwrap(), + 1000, /* allow deep nesting */ + ) + .expect("failed to create level2 qcow file"); + } }