Skip to content

Commit

Permalink
Add callback support to FileDescription
Browse files Browse the repository at this point in the history 
   - Implementing atomic reads for contiguous buffers

Signed-off-by: shamb0 <[email protected]>
  • Loading branch information
@shamb0
shamb0 committed Dec 26, 2024
1 parent 294ec0a commit fb2d16a
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 983 deletions.
85 changes: 1 addition & 84 deletions src/shims/files.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,6 @@ pub trait FileDescription: std::fmt::Debug + Any {
throw_unsup_format!("cannot read from {}", self.name());
}

/// Performs an atomic read operation on the file.
///
/// # Arguments
/// * `self_ref` - Strong reference to file description for lifetime management
/// * `communicate_allowed` - Whether external communication is permitted
/// * `op` - The I/O operation containing buffer and layout information
/// * `dest` - Destination for storing operation results
/// * `ecx` - Mutable reference to interpreter context
///
/// # Returns
/// * `Ok(())` on successful read
/// * `Err(_)` if read fails or is unsupported
fn read_atomic<'tcx>(
&self,
_self_ref: &FileDescriptionRef,
_communicate_allowed: bool,
_op: &mut IoTransferOperation<'tcx>,
_dest: &MPlaceTy<'tcx>,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
throw_unsup_format!("cannot read from {}", self.name());
}

/// Writes as much as possible from the given buffer `ptr`.
/// `len` indicates how many bytes we should try to write.
/// `dest` is where the return value should be stored: number of bytes written, or `-1` in case of error.
Expand Down Expand Up @@ -435,7 +412,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}

/// Represents an atomic I/O operation that handles data transfer between memory regions.
/// Supports both contiguous and scattered memory layouts for efficient I/O operations.
/// Supports contiguous memory layouts for efficient I/O operations.
#[derive(Clone)]
pub struct IoTransferOperation<'tcx> {
/// Intermediate buffer for atomic transfer operations.
Expand All @@ -458,8 +435,6 @@ pub struct IoTransferOperation<'tcx> {
enum IoBufferLayout {
/// Single continuous memory region for transfer.
Contiguous { address: Pointer },
/// Multiple discontinuous memory regions.
Scattered { regions: Vec<(Pointer, usize)> },
}

impl VisitProvenance for IoTransferOperation<'_> {
Expand All @@ -480,17 +455,6 @@ impl<'tcx> IoTransferOperation<'tcx> {
}
}

/// Creates a new I/O operation for scattered memory regions.
pub fn new_scattered(buffers: Vec<(Pointer, usize)>) -> Self {
let total_size = buffers.iter().map(|(_, len)| len).sum();
IoTransferOperation {
transfer_buffer: vec![0; total_size],
layout: IoBufferLayout::Scattered { regions: buffers },
total_size,
_phantom: std::marker::PhantomData,
}
}

/// Provides mutable access to the transfer buffer.
pub fn buffer_mut(&mut self) -> &mut [u8] {
&mut self.transfer_buffer
Expand Down Expand Up @@ -534,53 +498,6 @@ impl<'tcx> IoTransferOperation<'tcx> {
return ecx.set_last_error_and_return(LibcError("EIO"), dest);
}
}

IoBufferLayout::Scattered { regions } => {
let mut current_pos = 0;

for (ptr, len) in regions {
if current_pos >= bytes_processed {
break;
}

// Calculate copy size with safe arithmetic
let remaining_bytes = bytes_processed
.checked_sub(current_pos)
.expect("current_pos should never exceed bytes_read");
let copy_size = (*len).min(remaining_bytes);

// POSIX Compliance: Verify each buffer's accessibility
if ecx
.check_ptr_access(
*ptr,
Size::from_bytes(copy_size),
CheckInAllocMsg::MemoryAccessTest,
)
.report_err()
.is_err()
{
return ecx.set_last_error_and_return(LibcError("EFAULT"), dest);
}

let end_pos = current_pos
.checked_add(copy_size)
.expect("end position calculation should not overflow");

// Attempt the write operation with proper error handling
if ecx
.write_bytes_ptr(
*ptr,
self.transfer_buffer[current_pos..end_pos].iter().copied(),
)
.report_err()
.is_err()
{
return ecx.set_last_error_and_return(LibcError("EIO"), dest);
}

current_pos = end_pos;
}
}
}

interp_ok(())
Expand Down
187 changes: 2 additions & 185 deletions src/shims/unix/fd.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ use std::io;
use std::io::ErrorKind;

use rustc_abi::Size;
use rustc_middle::ty::layout::TyAndLayout;

use crate::helpers::check_min_arg_count;
use crate::shims::files::{FileDescription, IoTransferOperation};
use crate::shims::files::FileDescription;
use crate::shims::unix::linux_like::epoll::EpollReadyEvents;
use crate::shims::unix::*;
use crate::*;
Expand Down Expand Up @@ -247,10 +246,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// `usize::MAX` because it is bounded by the host's `isize`.

match offset {
None => {
let mut op = IoTransferOperation::new_contiguous(buf, count);
fd.read_atomic(&fd, communicate, &mut op, dest, this)?
}
None => fd.read(&fd, communicate, buf, count, dest, this)?,
Some(offset) => {
let Ok(offset) = u64::try_from(offset) else {
return this.set_last_error_and_return(LibcError("EINVAL"), dest);
Expand All @@ -261,185 +257,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(())
}

/// Reads data from a file descriptor into multiple buffers atomically (vectored I/O).
///
/// This implementation follows POSIX readv() semantics, reading data from the file descriptor
/// specified by `fd_num` into the buffers described by the array of iovec structures pointed
/// to by `iov_ptr`. The `iovcnt` argument specifies the number of iovec structures in the array.
///
/// # Arguments
/// * `fd_num` - The file descriptor to read from
/// * `iov_ptr` - Pointer to an array of iovec structures
/// * `iovcnt` - Number of iovec structures in the array
/// * `dest` - Destination for storing the number of bytes read
///
/// # Returns
/// * `Ok(())` - Operation completed successfully, with total bytes read stored in `dest`
/// * `Err(_)` - Operation failed with appropriate errno set
///
/// # Errors
/// * `EBADF` - `fd_num` is not a valid file descriptor
/// * `EFAULT` - Part of iovec array or buffers point outside accessible address space
/// * `EINVAL` - `iovcnt` is negative or exceeds system limit
/// * `EIO` - I/O error occurred while reading from the file descriptor
///
/// # POSIX Compliance
/// Implements standard POSIX readv() behavior:
/// * Performs reads atomically with respect to other threads
/// * Returns exact number of bytes read or -1 on error
/// * Handles partial reads and end-of-file conditions
/// * Respects system-imposed limits on total transfer size
fn readv(
&mut self,
fd_num: i32,
iov_ptr: &OpTy<'tcx>,
iovcnt: i32,
dest: &MPlaceTy<'tcx>,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();

// POSIX Compliance: Must handle negative values (EINVAL).
if iovcnt < 0 {
return this.set_last_error_and_return(LibcError("EINVAL"), dest);
}

// POSIX Compliance: Must handle zero properly.
if iovcnt == 0 {
return this.write_scalar(Scalar::from_i32(0), dest);
}

// POSIX Compliance: Check if iovcnt exceeds system limits.
// Most implementations limit this to IOV_MAX
// Common system default
// https://github.com/turbolent/w2c2/blob/d94227c22f8d78a04fbad70fa744481ca4a1912e/examples/clang/sys/include/limits.h#L50
const IOV_MAX: i32 = 1024;
if iovcnt > IOV_MAX {
trace!("readv: iovcnt exceeds IOV_MAX");
return this.set_last_error_and_return(LibcError("EINVAL"), dest);
}

// POSIX Compliance: Validate file descriptor.
let Some(fd) = this.machine.fds.get(fd_num) else {
return this.set_last_error_and_return(LibcError("EBADF"), dest);
};

// Convert iovcnt to usize for array indexing.
let iovcnt = usize::try_from(iovcnt).expect("iovcnt exceeds platform size");
let iovec_layout = this.libc_ty_layout("iovec");

// Gather iovec information.
// Pre-allocate vectors for iovec information
let mut iov_info = Vec::with_capacity(iovcnt);
let mut total_size: usize = 0;
let communicate = this.machine.communicate();

// POSIX Compliance: Validate each iovec structure.
// Must check for EFAULT (invalid buffer addresses) and EINVAL (invalid length).
for i in 0..iovcnt {
// Calculate offset to current iovec structure.
let offset = iovec_layout
.size
.bytes()
.checked_mul(i as u64)
.expect("iovec array index overflow");

// Access current iovec structure.
let current_iov = match this
.deref_pointer_and_offset_vectored(
iov_ptr,
offset,
iovec_layout,
iovcnt,
iovec_layout,
)
.report_err()
{
Ok(iov) => iov,
Err(_) => {
return this.set_last_error_and_return(LibcError("EFAULT"), dest);
}
};

// Extract and validate buffer pointer and length.
let base_field = this.project_field_named(&current_iov, "iov_base")?;
let base = this.read_pointer(&base_field)?;

let len_field = this.project_field_named(&current_iov, "iov_len")?;
let len: usize = usize::try_from(this.read_target_usize(&len_field)?).unwrap();

// Validate buffer alignment and accessibility.
if this
.check_ptr_access(base, Size::from_bytes(len), CheckInAllocMsg::MemoryAccessTest)
.report_err()
.is_err()
{
return this.set_last_error_and_return(LibcError("EFAULT"), dest);
}

// Update total size safely.
total_size = match total_size.checked_add(len) {
Some(new_size) => new_size,
None => {
return this.set_last_error_and_return(LibcError("EINVAL"), dest);
}
};

iov_info.push((base, len));
}

let mut op = IoTransferOperation::new_scattered(iov_info);
fd.read_atomic(&fd, communicate, &mut op, dest, this)?;

interp_ok(())
}

/// Dereferences a pointer to access an element within a source array, with specialized bounds checking
/// for vectored I/O operations like readv().
///
/// This function provides array-aware bounds checking that is specifically designed for situations
/// where we need to access multiple independent memory regions, such as when processing an array
/// of iovec structures. Unlike simple pointer arithmetic bounds checking, this implementation
/// understands and validates array-based access patterns.
fn deref_pointer_and_offset_vectored(
&self,
op: &impl Projectable<'tcx, Provenance>,
offset_bytes: u64,
base_layout: TyAndLayout<'tcx>,
count: usize,
value_layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, MPlaceTy<'tcx>> {
// 1. Validate the iovec array bounds.
let array_size = base_layout
.size
.bytes()
.checked_mul(count as u64)
.ok_or_else(|| err_ub_format!("iovec array size overflow"))?;

// 2. Check if our offset is within the array.
if offset_bytes >= array_size {
throw_ub_format!(
"{}",
format!(
"iovec array access out of bounds: offset {} in array of size {}",
offset_bytes, array_size
)
);
}

// 3. Ensure the iovec structure we're accessing is fully contained.
if offset_bytes.checked_add(base_layout.size.bytes()).is_none_or(|end| end > array_size) {
throw_ub_format!("iovec structure would extend past array bounds");
}

// 4. Proceed with the dereferencing.
let this = self.eval_context_ref();
let op_place = this.deref_pointer_as(op, base_layout)?;
let offset = Size::from_bytes(offset_bytes);

let value_place = op_place.offset(offset, value_layout, this)?;
interp_ok(value_place)
}

fn write(
&mut self,
fd_num: i32,
Expand Down
6 changes: 0 additions & 6 deletions src/shims/unix/foreign_items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let count = this.read_target_usize(count)?;
this.read(fd, buf, count, None, dest)?;
}
"readv" => {
let [fd, iov, iovcnt] = this.check_shim(abi, Conv::C , link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
let iovcnt = this.read_scalar(iovcnt)?.to_i32()?;
this.readv(fd, iov, iovcnt, dest)?;
}
"write" => {
let [fd, buf, n] = this.check_shim(abi, Conv::C , link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
Expand Down
Loading

0 comments on commit fb2d16a

Please sign in to comment.