diff --git a/Cargo.lock b/Cargo.lock index ba21e2ee75..5a7484043d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2140,6 +2140,27 @@ dependencies = [ "xtask_fuzz", ] +[[package]] +name = "fuzz_storvsp" +version = "0.0.0" +dependencies = [ + "anyhow", + "arbitrary", + "disklayer_ram", + "guestmem", + "libfuzzer-sys", + "pal_async", + "scsi_defs", + "scsidisk", + "storvsp", + "storvsp_resources", + "vmbus_async", + "vmbus_channel", + "vmbus_ring", + "xtask_fuzz", + "zerocopy", +] + [[package]] name = "fuzz_ucs2" version = "0.0.0" @@ -5687,6 +5708,7 @@ dependencies = [ name = "scsi_defs" version = "0.0.0" dependencies = [ + "arbitrary", "bitfield-struct", "open_enum", "zerocopy", @@ -6179,6 +6201,7 @@ name = "storvsp" version = "0.0.0" dependencies = [ "anyhow", + "arbitrary", "async-trait", "criterion", "disklayer_ram", @@ -6220,6 +6243,7 @@ dependencies = [ name = "storvsp_resources" version = "0.0.0" dependencies = [ + "arbitrary", "guid", "mesh", "vm_resource", diff --git a/Cargo.toml b/Cargo.toml index b8ba9399af..a86a1cb163 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ members = [ "vm/devices/storage/disk_nvme/nvme_driver/fuzz", "vm/devices/storage/ide/fuzz", "vm/devices/storage/scsi_buffers/fuzz", + "vm/devices/storage/storvsp/fuzz", "vm/vmcore/guestmem/fuzz", "vm/x86/x86emu/fuzz", # in-guest test bins diff --git a/vm/devices/storage/scsi_defs/Cargo.toml b/vm/devices/storage/scsi_defs/Cargo.toml index 4bf61fa6e7..74d4a467cf 100644 --- a/vm/devices/storage/scsi_defs/Cargo.toml +++ b/vm/devices/storage/scsi_defs/Cargo.toml @@ -6,7 +6,12 @@ name = "scsi_defs" edition = "2021" rust-version.workspace = true +[features] +# Enable generating arbitrary values of types useful for fuzzing. +arbitrary = ["dep:arbitrary"] + [dependencies] +arbitrary = { workspace = true, optional = true, features = ["derive"] } zerocopy.workspace = true bitfield-struct.workspace = true open_enum.workspace = true diff --git a/vm/devices/storage/scsi_defs/src/lib.rs b/vm/devices/storage/scsi_defs/src/lib.rs index 3d02f257a7..0aa0678340 100644 --- a/vm/devices/storage/scsi_defs/src/lib.rs +++ b/vm/devices/storage/scsi_defs/src/lib.rs @@ -713,6 +713,7 @@ pub const SCSI_SENSEQ_OPERATING_DEFINITION_CHANGED: u8 = 0x02; open_enum! { #[derive(AsBytes, FromBytes, FromZeroes)] + #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum ScsiStatus: u8 { GOOD = 0x00, CHECK_CONDITION = 0x02, diff --git a/vm/devices/storage/scsi_defs/src/srb.rs b/vm/devices/storage/scsi_defs/src/srb.rs index 3f2317d869..4330100745 100644 --- a/vm/devices/storage/scsi_defs/src/srb.rs +++ b/vm/devices/storage/scsi_defs/src/srb.rs @@ -9,6 +9,7 @@ use zerocopy::FromZeroes; #[bitfield(u8)] #[derive(AsBytes, FromBytes, FromZeroes)] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct SrbStatusAndFlags { #[bits(6)] status_bits: u8, diff --git a/vm/devices/storage/storvsp/Cargo.toml b/vm/devices/storage/storvsp/Cargo.toml index db546422b4..ce446c4125 100644 --- a/vm/devices/storage/storvsp/Cargo.toml +++ b/vm/devices/storage/storvsp/Cargo.toml @@ -9,7 +9,15 @@ rust-version.workspace = true [features] ioperf = ["dep:disklayer_ram"] +# Enable generating arbitrary values of types useful for fuzzing. +arbitrary = ["dep:arbitrary"] + +# Expose some implementation details publicly, used for fuzzing. +fuzz_helpers = [] + [dependencies] +arbitrary = { workspace = true, optional = true, features = ["derive"] } + disklayer_ram = { workspace = true, optional = true } # For `ioperf` modules scsi_buffers.workspace = true scsi_core.workspace = true diff --git a/vm/devices/storage/storvsp/fuzz/Cargo.toml b/vm/devices/storage/storvsp/fuzz/Cargo.toml new file mode 100644 index 0000000000..24f1d4b033 --- /dev/null +++ b/vm/devices/storage/storvsp/fuzz/Cargo.toml @@ -0,0 +1,47 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +[package] +name = "fuzz_storvsp" +publish = false +edition = "2021" +rust-version.workspace = true + +[dependencies] +anyhow.workspace = true +arbitrary = { workspace = true, features = ["derive"] } +disklayer_ram.workspace = true +guestmem.workspace = true +pal_async.workspace = true +scsi_defs = {workspace = true, features = ["arbitrary"]} +scsidisk.workspace = true +storvsp = {workspace = true, features = ["arbitrary", "fuzz_helpers"]} +storvsp_resources = {workspace = true, features = ["arbitrary"]} +vmbus_async.workspace = true +vmbus_channel.workspace = true +vmbus_ring.workspace = true +xtask_fuzz.workspace = true +zerocopy.workspace = true + +[target.'cfg(all(target_os = "linux", target_env = "gnu"))'.dependencies] +libfuzzer-sys.workspace = true + +[package.metadata] +cargo-fuzz = true + +[package.metadata.xtask.fuzz.onefuzz-allowlist] +fuzz_storvsp = ["**/*.rs", "../src/**/*.rs"] + +[package.metadata.xtask.unused-deps] +# required for the xtask_fuzz macro, but unused_deps doesn't know that +ignored = ["libfuzzer-sys"] + +[[bin]] +name = "fuzz_storvsp" +path = "fuzz_storvsp.rs" +test = false +doc = false +doctest = false + +[lints] +workspace = true diff --git a/vm/devices/storage/storvsp/fuzz/fuzz_storvsp.rs b/vm/devices/storage/storvsp/fuzz/fuzz_storvsp.rs new file mode 100644 index 0000000000..84e1b483bf --- /dev/null +++ b/vm/devices/storage/storvsp/fuzz/fuzz_storvsp.rs @@ -0,0 +1,234 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#![cfg_attr(all(target_os = "linux", target_env = "gnu"), no_main)] + +use arbitrary::Arbitrary; +use arbitrary::Unstructured; +use guestmem::ranges::PagedRange; +use guestmem::GuestMemory; +use pal_async::DefaultPool; +use scsi_defs::Cdb10; +use scsi_defs::ScsiOp; +use std::sync::Arc; +use storvsp::protocol; +use storvsp::test_helpers::TestGuest; +use storvsp::test_helpers::TestWorker; +use storvsp::ScsiController; +use storvsp::ScsiControllerDisk; +use storvsp_resources::ScsiPath; +use vmbus_async::queue::OutgoingPacket; +use vmbus_async::queue::Queue; +use vmbus_channel::connected_async_channels; +use vmbus_ring::OutgoingPacketType; +use vmbus_ring::PAGE_SIZE; +use xtask_fuzz::fuzz_target; +use zerocopy::AsBytes; +use zerocopy::FromZeroes; + +#[derive(Arbitrary)] +enum StorvspFuzzAction { + SendReadWritePacket, + SendRawPacket(FuzzOutgoingPacketType), + ReadCompletion, +} + +#[derive(Arbitrary)] +enum FuzzOutgoingPacketType { + AnyOutgoingPacket, + GpaDirectPacket, +} + +/// Return an arbitrary byte length that can be sent in a GPA direct +/// packet. The byte length is limited to the maximum number of pages +/// that could fit into a `PagedRange` (at least with how we store the +/// list of pages in the fuzzer ...). +fn arbitrary_byte_len(u: &mut Unstructured<'_>) -> Result { + let max_byte_len = u.arbitrary_len::()? * PAGE_SIZE; + u.int_in_range(0..=max_byte_len) +} + +/// Sends a GPA direct packet (a type of vmbus packet that references guest memory, +/// the typical packet type used for SCSI requests) to storvsp. +async fn send_gpa_direct_packet( + guest: &mut TestGuest, + payload: &[&[u8]], + gpa_start: u64, + byte_len: usize, + transaction_id: u64, +) -> Result<(), anyhow::Error> { + let start_page: u64 = gpa_start / PAGE_SIZE as u64; + let end_page = start_page + .checked_add(byte_len.try_into()?) + .map(|v| v.div_ceil(PAGE_SIZE as u64)) + .ok_or(arbitrary::Error::IncorrectFormat)?; + + let gpns: Vec = (start_page..end_page).collect(); + let pages = PagedRange::new(gpa_start as usize % PAGE_SIZE, byte_len, gpns.as_slice()) + .ok_or(arbitrary::Error::IncorrectFormat)?; + + guest + .queue + .split() + .1 + .write(OutgoingPacket { + packet_type: OutgoingPacketType::GpaDirect(&[pages]), + transaction_id, + payload, + }) + .await + .map_err(|e| e.into()) +} + +/// Send a reasonably well structured read or write packet to storvsp. +/// While the fuzzer should eventually discover these paths by poking at +/// arbitrary GpaDirect packet payload, make the search more efficient by +/// generating a packet that is more likely to pass basic parsing checks. +async fn send_arbitrary_readwrite_packet( + u: &mut Unstructured<'_>, + guest: &mut TestGuest, +) -> Result<(), anyhow::Error> { + let path: ScsiPath = u.arbitrary()?; + let gpa = u.arbitrary::()?; + let byte_len = arbitrary_byte_len(u)?; + + let block: u32 = u.arbitrary()?; + let transaction_id: u64 = u.arbitrary()?; + + let packet = protocol::Packet { + operation: protocol::Operation::EXECUTE_SRB, + flags: 0, + status: protocol::NtStatus::SUCCESS, + }; + + // TODO: read6, read12, read16, write6, write12, write16, etc. (READ is read10, WRITE is write10) + let scsiop_choices = [ScsiOp::READ, ScsiOp::WRITE]; + let cdb = Cdb10 { + operation_code: *(u.choose(&scsiop_choices)?), + logical_block: block.into(), + transfer_blocks: ((byte_len / 512) as u16).into(), + ..FromZeroes::new_zeroed() + }; + + let mut scsi_req = protocol::ScsiRequest { + target_id: path.target, + path_id: path.path, + lun: path.lun, + length: protocol::SCSI_REQUEST_LEN_V2 as u16, + cdb_length: size_of::() as u8, + data_transfer_length: byte_len.try_into()?, + data_in: 1, + ..FromZeroes::new_zeroed() + }; + + scsi_req.payload[0..10].copy_from_slice(cdb.as_bytes()); + + send_gpa_direct_packet( + guest, + &[packet.as_bytes(), scsi_req.as_bytes()], + gpa, + byte_len, + transaction_id, + ) + .await +} + +fn do_fuzz(u: &mut Unstructured<'_>) -> Result<(), anyhow::Error> { + DefaultPool::run_with(|driver| async move { + let (host, guest_channel) = connected_async_channels(16 * 1024); // TODO: [use-arbitrary-input] + let guest_queue = Queue::new(guest_channel).unwrap(); + + let test_guest_mem = GuestMemory::allocate(u.int_in_range(1..=256)? * PAGE_SIZE); + let controller = ScsiController::new(); + let disk_len_sectors = u.int_in_range(1..=1048576)?; // up to 512mb in 512 byte sectors + let disk = scsidisk::SimpleScsiDisk::new( + disklayer_ram::ram_disk(disk_len_sectors * 512, false).unwrap(), + Default::default(), + ); + controller.attach(u.arbitrary()?, ScsiControllerDisk::new(Arc::new(disk)))?; + + let _test_worker = TestWorker::start( + controller, + driver.clone(), + test_guest_mem.clone(), + host, + None, + ); + + let mut guest = TestGuest { + queue: guest_queue, + transaction_id: 0, + }; + + if u.ratio(9, 10)? { + // TODO: [use-arbitrary-input] (e.g., munge the negotiation packets) + guest.perform_protocol_negotiation().await; + } + + while !u.is_empty() { + let action = u.arbitrary::()?; + match action { + StorvspFuzzAction::SendReadWritePacket => { + send_arbitrary_readwrite_packet(u, &mut guest).await?; + } + StorvspFuzzAction::SendRawPacket(packet_type) => { + match packet_type { + FuzzOutgoingPacketType::AnyOutgoingPacket => { + let packet_types = [ + OutgoingPacketType::InBandNoCompletion, + OutgoingPacketType::InBandWithCompletion, + OutgoingPacketType::Completion, + ]; + let payload = u.arbitrary::()?; + // TODO: [use-arbitrary-input] (send a byte blob of arbitrary length rather + // than a fixed-size arbitrary packet) + let packet = OutgoingPacket { + transaction_id: u.arbitrary()?, + packet_type: *u.choose(&packet_types)?, + payload: &[payload.as_bytes()], // TODO: [use-arbitrary-input] + }; + + guest.queue.split().1.write(packet).await?; + } + FuzzOutgoingPacketType::GpaDirectPacket => { + let header = u.arbitrary::()?; + let scsi_req = u.arbitrary::()?; + + send_gpa_direct_packet( + &mut guest, + &[header.as_bytes(), scsi_req.as_bytes()], + u.arbitrary()?, + arbitrary_byte_len(u)?, + u.arbitrary()?, + ) + .await? + } + } + } + StorvspFuzzAction::ReadCompletion => { + // Read completion(s) from the storvsp -> guest queue. This shouldn't + // evoke any specific storvsp behavior, but is important to eventually + // allow forward progress of various code paths. + // + // Ignore the result, since vmbus returns error if the queue is empty, + // but that's fine for the fuzzer ... + let _ = guest.queue.split().0.try_read(); + } + } + } + + Ok::<(), anyhow::Error>(()) + })?; + + Ok::<(), anyhow::Error>(()) +} + +fuzz_target!(|input: &[u8]| { + xtask_fuzz::init_tracing_if_repro(); + + let _ = do_fuzz(&mut Unstructured::new(input)); + + // Always keep the corpus, since errors are a reasonable outcome. + // A future optimization would be to reject any corpus entries that + // result in the inability to generate arbitrary data from the Unstructured... +}); diff --git a/vm/devices/storage/storvsp/src/ioperf.rs b/vm/devices/storage/storvsp/src/ioperf.rs index 0898d03e70..c1a763c57d 100644 --- a/vm/devices/storage/storvsp/src/ioperf.rs +++ b/vm/devices/storage/storvsp/src/ioperf.rs @@ -54,7 +54,7 @@ impl PerfTester { let test_guest_mem = GuestMemory::allocate(16 * 1024); let worker = TestWorker::start( - controller.state.clone(), + controller, driver, test_guest_mem.clone(), host, diff --git a/vm/devices/storage/storvsp/src/lib.rs b/vm/devices/storage/storvsp/src/lib.rs index 239e721d0c..e8e82e7e2f 100644 --- a/vm/devices/storage/storvsp/src/lib.rs +++ b/vm/devices/storage/storvsp/src/lib.rs @@ -6,10 +6,18 @@ #[cfg(feature = "ioperf")] pub mod ioperf; +#[cfg(feature = "fuzz_helpers")] +pub mod protocol; +#[cfg(feature = "fuzz_helpers")] +pub mod test_helpers; + +#[cfg(not(feature = "fuzz_helpers"))] mod protocol; +#[cfg(not(feature = "fuzz_helpers"))] +mod test_helpers; + pub mod resolver; mod save_restore; -mod test_helpers; use crate::ring::gparange::GpnList; use crate::ring::gparange::MultiPagedRangeBuf; @@ -1734,6 +1742,17 @@ mod tests { use test_with_tracing::test; use vmbus_channel::connected_async_channels; + // Discourage `Clone` for `ScsiController` outside the crate, but it is + // necessary for testing. The fuzzer also uses `TestWorker`, which needs + // a `clone` of the inner state, but is not in this crate. + impl Clone for ScsiController { + fn clone(&self) -> Self { + ScsiController { + state: self.state.clone(), + } + } + } + #[async_test] async fn test_channel_working(driver: DefaultDriver) { // set up the channels and worker @@ -1758,7 +1777,7 @@ mod tests { .unwrap(); let test_worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem.clone(), host, @@ -1813,7 +1832,7 @@ mod tests { let controller = ScsiController::new(); let _worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem, host, @@ -1884,7 +1903,7 @@ mod tests { let controller = ScsiController::new(); let _worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem, host, @@ -1934,7 +1953,7 @@ mod tests { let controller = ScsiController::new(); let worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem, host, @@ -1974,7 +1993,7 @@ mod tests { let controller = ScsiController::new(); let _worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem, host, @@ -2059,7 +2078,7 @@ mod tests { let controller = ScsiController::new(); let _worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem, host, @@ -2101,7 +2120,7 @@ mod tests { let controller = ScsiController::new(); let test_worker = TestWorker::start( - controller.state.clone(), + controller.clone(), driver.clone(), test_guest_mem.clone(), host, @@ -2282,7 +2301,7 @@ mod tests { let test_guest_mem = GuestMemory::allocate(16384); let worker = TestWorker::start( - controller.state.clone(), + controller.clone(), &driver, test_guest_mem.clone(), host, diff --git a/vm/devices/storage/storvsp/src/protocol.rs b/vm/devices/storage/storvsp/src/protocol.rs index 76a3e92370..6e0c7d414f 100644 --- a/vm/devices/storage/storvsp/src/protocol.rs +++ b/vm/devices/storage/storvsp/src/protocol.rs @@ -45,6 +45,7 @@ pub const VERSION_THRESHOLD: u16 = version(6, 2); open_enum! { #[derive(AsBytes, FromBytes, FromZeroes)] + #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum NtStatus: u32 { SUCCESS = 0x0000_0000, BUFFER_OVERFLOW = 0x8000_0005, // The data was too large to fit into the specified buffer. @@ -66,6 +67,7 @@ open_enum! { #[repr(C)] #[derive(Debug, Copy, Clone, AsBytes, FromBytes, FromZeroes)] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct Packet { // Requested operation type pub operation: Operation, @@ -77,6 +79,7 @@ pub struct Packet { open_enum! { #[derive(AsBytes, FromBytes, FromZeroes)] + #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum Operation: u32 { COMPLETE_IO = 1, REMOVE_DEVICE = 2, @@ -101,6 +104,7 @@ pub const VMSCSI_SENSE_BUFFER_SIZE: usize = 0x14; #[repr(C)] #[derive(Copy, Clone, Debug, AsBytes, FromBytes, FromZeroes)] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ScsiRequest { pub length: u16, pub srb_status: SrbStatusAndFlags, diff --git a/vm/devices/storage/storvsp/src/test_helpers.rs b/vm/devices/storage/storvsp/src/test_helpers.rs index 3f5d5a323f..12bfb8468c 100644 --- a/vm/devices/storage/storvsp/src/test_helpers.rs +++ b/vm/devices/storage/storvsp/src/test_helpers.rs @@ -13,7 +13,7 @@ use crate::InitState; use crate::PacketError; use crate::Protocol; use crate::ProtocolState; -use crate::ScsiControllerState; +use crate::ScsiController; use crate::ScsiPath; use crate::Worker; use crate::WorkerError; @@ -38,17 +38,17 @@ use vmbus_ring::PAGE_SIZE; use zerocopy::AsBytes; use zerocopy::FromZeroes; -pub(crate) struct TestWorker { +pub struct TestWorker { task: Task>, } impl TestWorker { - pub async fn teardown(self) -> Result<(), WorkerError> { + pub(crate) async fn teardown(self) -> Result<(), WorkerError> { self.task.await } pub fn start( - controller: Arc, + controller: ScsiController, spawner: impl Spawn, mem: GuestMemory, channel: RawAsyncChannel, @@ -56,7 +56,7 @@ impl TestWorker { ) -> Self { let task = spawner.spawn("test", async move { let mut worker = Worker::new( - controller.clone(), + controller.state.clone(), channel, 0, mem, @@ -164,7 +164,7 @@ pub(crate) fn parse_guest_enumerate_bus( } } -pub(crate) struct TestGuest { +pub struct TestGuest { pub queue: Queue, pub transaction_id: u64, } @@ -343,7 +343,7 @@ impl TestGuest { } // Send protocol negotiation packets for a test guest. - pub(crate) async fn perform_protocol_negotiation(&mut self) { + pub async fn perform_protocol_negotiation(&mut self) { let negotiate_packet = protocol::Packet { operation: protocol::Operation::BEGIN_INITIALIZATION, flags: 0, diff --git a/vm/devices/storage/storvsp_resources/Cargo.toml b/vm/devices/storage/storvsp_resources/Cargo.toml index ee50f6bed1..390bec70ef 100644 --- a/vm/devices/storage/storvsp_resources/Cargo.toml +++ b/vm/devices/storage/storvsp_resources/Cargo.toml @@ -6,7 +6,12 @@ name = "storvsp_resources" edition = "2021" rust-version.workspace = true +[features] +# Enable generating arbitrary values of types useful for fuzzing. +arbitrary = ["dep:arbitrary"] + [dependencies] +arbitrary = { workspace = true, optional = true, features = ["derive"] } vm_resource.workspace = true guid = { workspace = true, features = ["mesh"] } diff --git a/vm/devices/storage/storvsp_resources/src/lib.rs b/vm/devices/storage/storvsp_resources/src/lib.rs index df0b849f4c..d08b82ff3e 100644 --- a/vm/devices/storage/storvsp_resources/src/lib.rs +++ b/vm/devices/storage/storvsp_resources/src/lib.rs @@ -17,6 +17,7 @@ use vm_resource::ResourceId; /// A path at which to enumerate a SCSI logical unit. #[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Hash, Protobuf)] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ScsiPath { /// The SCSI path number. pub path: u8,