From 92d9a53ed3eaea04d02cfb4518589a1775887f87 Mon Sep 17 00:00:00 2001 From: Dylan Reid Date: Tue, 19 May 2026 23:23:43 -0700 Subject: [PATCH] block: Avoid raw iovecs in qcow sync I/O This removes the qcow sync raw-iovec unsafe path and drops the now-unused qcow iovec scatter/gather helpers. Assisted-by: Codex:GPT-5.5 Signed-off-by: Dylan Reid --- block/src/qcow_common.rs | 111 ++------------------------------------- block/src/qcow_sync.rs | 85 ++++++++++-------------------- 2 files changed, 33 insertions(+), 163 deletions(-) diff --git a/block/src/qcow_common.rs b/block/src/qcow_common.rs index 49cae1f79..df70c7b0c 100644 --- a/block/src/qcow_common.rs +++ b/block/src/qcow_common.rs @@ -2,17 +2,17 @@ // // Copyright 2026 The Cloud Hypervisor Authors. All rights reserved. // +// Copyright (c) Meta Platforms, Inc. and affiliates. +// // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause //! Shared helpers for QCOW2 sync and async backends. //! -//! Position-independent I/O (`pread_exact`, `pwrite_all`) and iovec -//! scatter/gather helpers used by both `qcow_sync` and `qcow_async`. +//! Position-independent I/O helpers used by both `qcow_sync` and `qcow_async`. use std::alloc::{Layout, alloc_zeroed, dealloc}; -use std::cmp::min; use std::os::fd::RawFd; -use std::{io, ptr, slice}; +use std::{io, slice}; use crate::qcow::decoder::Decoder; @@ -189,109 +189,6 @@ pub fn aligned_pwrite(fd: RawFd, buf: &[u8], offset: u64, alignment: usize) -> i pwrite_all(fd, bounce.as_slice(aligned_len), aligned_offset) } -// -- iovec helper functions -- -// -// Operate on the iovec array as a flat byte stream. - -/// Copy data into iovecs starting at the given byte offset. -/// -/// # Safety -/// Caller must ensure iovecs point to valid, writable memory of sufficient size. -pub unsafe fn scatter_to_iovecs(iovecs: &[libc::iovec], start: usize, data: &[u8]) { - let mut remaining = data; - let mut pos = 0usize; - for iov in iovecs { - let iov_end = pos + iov.iov_len; - if iov_end <= start || remaining.is_empty() { - pos = iov_end; - continue; - } - let iov_start = start.saturating_sub(pos); - let available = iov.iov_len - iov_start; - let count = min(available, remaining.len()); - // SAFETY: iov_base is valid for iov_len bytes per caller contract. - unsafe { - let dst = iov.iov_base.cast::().add(iov_start); - ptr::copy_nonoverlapping(remaining.as_ptr(), dst, count); - } - remaining = &remaining[count..]; - if remaining.is_empty() { - break; - } - pos = iov_end; - } -} - -/// Zero fill iovecs starting at the given byte offset for the given length. -/// -/// # Safety -/// Caller must ensure iovecs point to valid, writable memory of sufficient size. -pub unsafe fn zero_fill_iovecs(iovecs: &[libc::iovec], start: usize, len: usize) { - let mut remaining = len; - let mut pos = 0usize; - for iov in iovecs { - let iov_end = pos + iov.iov_len; - if iov_end <= start || remaining == 0 { - pos = iov_end; - continue; - } - let iov_start = start.saturating_sub(pos); - let available = iov.iov_len - iov_start; - let count = min(available, remaining); - // SAFETY: iov_base is valid for iov_len bytes per caller contract. - unsafe { - let dst = iov.iov_base.cast::().add(iov_start); - ptr::write_bytes(dst, 0, count); - } - remaining -= count; - if remaining == 0 { - break; - } - pos = iov_end; - } -} - -/// Gather bytes from iovecs starting at the given byte offset into `dst`. -/// -/// # Safety -/// Caller must ensure iovecs point to valid, readable memory of sufficient size. -pub unsafe fn gather_from_iovecs_into(iovecs: &[libc::iovec], start: usize, dst: &mut [u8]) { - let len = dst.len(); - let mut written = 0usize; - let mut pos = 0usize; - for iov in iovecs { - let iov_end = pos + iov.iov_len; - if iov_end <= start || written == len { - pos = iov_end; - continue; - } - let iov_start = start.saturating_sub(pos); - let available = iov.iov_len - iov_start; - let count = min(available, len - written); - // SAFETY: iov_base is valid for iov_len bytes per caller contract. - unsafe { - let src = iov.iov_base.cast::().add(iov_start); - ptr::copy_nonoverlapping(src, dst.as_mut_ptr().add(written), count); - } - written += count; - if written == len { - break; - } - pos = iov_end; - } -} - -/// Gather bytes from iovecs starting at the given byte offset into a Vec. -/// -/// # Safety -/// Caller must ensure iovecs point to valid, readable memory of sufficient size. -pub unsafe fn gather_from_iovecs(iovecs: &[libc::iovec], start: usize, len: usize) -> Vec { - let mut result = vec![0u8; len]; - // SAFETY: caller guarantees iovecs are valid; result has len bytes. - unsafe { gather_from_iovecs_into(iovecs, start, &mut result) }; - result -} - #[cfg(test)] pub(crate) mod unit_tests { use std::fs::File; diff --git a/block/src/qcow_sync.rs b/block/src/qcow_sync.rs index b794a0e2c..e7a3e5a63 100644 --- a/block/src/qcow_sync.rs +++ b/block/src/qcow_sync.rs @@ -19,9 +19,8 @@ use crate::qcow::metadata::{ }; use crate::qcow::qcow_raw_file::QcowRawFile; use crate::qcow_common::{ - AlignedBuf, aligned_pread, aligned_pwrite, decompress_cluster, gather_from_iovecs, - gather_from_iovecs_into, pread_alloc, pread_exact, pwrite_all, scatter_to_iovecs, - zero_fill_iovecs, + AlignedBuf, aligned_pread, aligned_pwrite, decompress_cluster, pread_alloc, pread_exact, + pwrite_all, }; pub struct QcowSync { @@ -80,15 +79,9 @@ impl QcowSync { } } - // SAFETY: each iovec must describe writable memory that remains valid for - // this synchronous call and must be safe to turn into a mutable Rust slice. - unsafe fn read_iovecs( - &mut self, - offset: libc::off_t, - iovecs: &[libc::iovec], - ) -> AsyncIoResult { - let address = offset as u64; - let total_len: usize = iovecs.iter().map(|v| v.iov_len).sum(); + fn read_operation(&mut self, op: &mut AsyncIoOperation) -> AsyncIoResult { + let address = op.offset() as u64; + let total_len = op.total_len(); let has_backing = self.backing_file.is_some(); let mappings = self @@ -100,8 +93,8 @@ impl QcowSync { for mapping in mappings { match mapping { ClusterReadMapping::Zero { length } => { - // SAFETY: Guaranteed by read_iovecs' caller. - unsafe { zero_fill_iovecs(iovecs, buf_offset, length as usize) }; + op.fill_zeroes_at(buf_offset, length as usize) + .map_err(AsyncIoError::ReadVectored)?; buf_offset += length as usize; } ClusterReadMapping::Allocated { @@ -120,15 +113,15 @@ impl QcowSync { self.alignment, ) .map_err(AsyncIoError::ReadVectored)?; - // SAFETY: Guaranteed by read_iovecs' caller. - unsafe { scatter_to_iovecs(iovecs, buf_offset, abuf.as_slice(len)) }; + op.write_bytes_at(buf_offset, abuf.as_slice(len)) + .map_err(AsyncIoError::ReadVectored)?; } else { // No O_DIRECT, plain buffer is fine. let mut buf = vec![0u8; len]; pread_exact(self.data_file.as_raw_fd(), &mut buf, host_offset) .map_err(AsyncIoError::ReadVectored)?; - // SAFETY: Guaranteed by read_iovecs' caller. - unsafe { scatter_to_iovecs(iovecs, buf_offset, &buf) }; + op.write_bytes_at(buf_offset, &buf) + .map_err(AsyncIoError::ReadVectored)?; } buf_offset += len; } @@ -144,14 +137,11 @@ impl QcowSync { let decompressed = decompress_cluster(&compressed, self.cluster_size as usize, &*self.decoder) .map_err(AsyncIoError::ReadVectored)?; - // SAFETY: Guaranteed by read_iovecs' caller. - unsafe { - scatter_to_iovecs( - iovecs, - buf_offset, - &decompressed[cluster_offset..cluster_offset + length], - ); - } + op.write_bytes_at( + buf_offset, + &decompressed[cluster_offset..cluster_offset + length], + ) + .map_err(AsyncIoError::ReadVectored)?; buf_offset += length; } ClusterReadMapping::Backing { @@ -164,8 +154,8 @@ impl QcowSync { .unwrap() .read_at(backing_offset, &mut buf) .map_err(AsyncIoError::ReadVectored)?; - // SAFETY: Guaranteed by read_iovecs' caller. - unsafe { scatter_to_iovecs(iovecs, buf_offset, &buf) }; + op.write_bytes_at(buf_offset, &buf) + .map_err(AsyncIoError::ReadVectored)?; buf_offset += length as usize; } } @@ -174,15 +164,9 @@ impl QcowSync { Ok(total_len) } - // SAFETY: each iovec must describe readable memory that remains valid for - // this synchronous call and must be safe to turn into a shared Rust slice. - unsafe fn write_iovecs( - &mut self, - offset: libc::off_t, - iovecs: &[libc::iovec], - ) -> AsyncIoResult { - let address = offset as u64; - let total_len: usize = iovecs.iter().map(|v| v.iov_len).sum(); + fn write_operation(&mut self, op: &AsyncIoOperation) -> AsyncIoResult { + let address = op.offset() as u64; + let total_len = op.total_len(); let mut buf_offset = 0usize; while buf_offset < total_len { @@ -221,10 +205,8 @@ impl QcowSync { // O_DIRECT, gather directly into aligned buffer. let mut abuf = AlignedBuf::new(count, self.alignment) .map_err(AsyncIoError::WriteVectored)?; - // SAFETY: Guaranteed by write_iovecs' caller. - unsafe { - gather_from_iovecs_into(iovecs, buf_offset, abuf.as_mut_slice(count)); - } + op.read_bytes_at(buf_offset, abuf.as_mut_slice(count)) + .map_err(AsyncIoError::WriteVectored)?; aligned_pwrite( self.data_file.as_raw_fd(), abuf.as_slice(count), @@ -234,8 +216,9 @@ impl QcowSync { .map_err(AsyncIoError::WriteVectored)?; } else { // No O_DIRECT, plain buffer is fine. - // SAFETY: Guaranteed by write_iovecs' caller. - let buf = unsafe { gather_from_iovecs(iovecs, buf_offset, count) }; + let mut buf = vec![0u8; count]; + op.read_bytes_at(buf_offset, &mut buf) + .map_err(AsyncIoError::WriteVectored)?; pwrite_all(self.data_file.as_raw_fd(), &buf, host_offset) .map_err(AsyncIoError::WriteVectored)?; } @@ -253,22 +236,12 @@ impl AsyncIo for QcowSync { &self.eventfd } - fn submit_data_operation(&mut self, op: AsyncIoOperation) -> AsyncIoResult<()> { - let offset = op.offset(); + fn submit_data_operation(&mut self, mut op: AsyncIoOperation) -> AsyncIoResult<()> { let is_read = op.is_read(); - let iovecs = op.iovecs(); let total_len = if is_read { - // SAFETY: AsyncIoOperation keeps the iovec target alive for this - // synchronous call. Host-memory operations also satisfy the - // aliasing requirement above; guest-memory-backed iovecs remain a - // temporary unsound qcow sync case deferred to a later fix. - unsafe { self.read_iovecs(offset, iovecs)? } + self.read_operation(&mut op)? } else { - // SAFETY: AsyncIoOperation keeps the iovec target alive for this - // synchronous call. Host-memory operations also satisfy the - // aliasing requirement above; guest-memory-backed iovecs remain a - // temporary unsound qcow sync case deferred to a later fix. - unsafe { self.write_iovecs(offset, iovecs)? } + self.write_operation(&op)? }; self.completion_list .push_back(AsyncIoCompletion::from_operation(op, total_len as i32));