RFC: storvsp: new fuzzer

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"
@@ -5670,6 +5691,7 @@ dependencies = [
 name = "scsi_defs"
 version = "0.0.0"
 dependencies = [
+ "arbitrary",
  "bitfield-struct",
  "open_enum",
  "zerocopy",
@@ -6162,6 +6184,7 @@ name = "storvsp"
 version = "0.0.0"
 dependencies = [
  "anyhow",
+ "arbitrary",
  "async-trait",
  "criterion",
  "disklayer_ram",
@@ -6203,6 +6226,7 @@ dependencies = [
 name = "storvsp_resources"
 version = "0.0.0"
 dependencies = [
+ "arbitrary",
  "guid",
  "mesh",
  "vm_resource",

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

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

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,

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,

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

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

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<usize, arbitrary::Error> {
+    let max_byte_len = u.arbitrary_len::<u64>()? * 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<u64> = (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::<u64>()?;
+    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::<Cdb10>() 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::<StorvspFuzzAction>()?;
+            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::<protocol::Packet>()?;
+                            // 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::<protocol::Packet>()?;
+                            let scsi_req = u.arbitrary::<protocol::ScsiRequest>()?;
+
+                            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...
+});

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,