From ed880f5297f8ea5f4b56e4a960f9b003676650a8 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sun, 3 Jul 2022 16:51:15 +0200 Subject: [PATCH] Handle buffered CLAP stream reads and writes `clap-validator` now tests this. --- src/wrapper/clap/util.rs | 85 +++++++++++++++++++++++++++++++++++++ src/wrapper/clap/wrapper.rs | 51 +++++++++++----------- 2 files changed, 111 insertions(+), 25 deletions(-) diff --git a/src/wrapper/clap/util.rs b/src/wrapper/clap/util.rs index 30f06918..592ca624 100644 --- a/src/wrapper/clap/util.rs +++ b/src/wrapper/clap/util.rs @@ -1,4 +1,7 @@ +use clap_sys::stream::{clap_istream, clap_ostream}; +use std::mem::MaybeUninit; use std::ops::Deref; +use std::os::raw::c_void; /// Early exit out of a function with the specified return value when one of the passed pointers is /// null. @@ -47,3 +50,85 @@ impl ClapPtr { Self { inner: ptr } } } + +/// A buffer a stream can be read into. This is needed to allow reading into uninitialized vectors +/// using slices without invoking UB. +/// +/// # Safety +/// +/// This may only be implemented by slices of `u8` and types with the same representation as `u8`. +pub unsafe trait ByteReadBuffer { + /// The length of the slice, in bytes. + fn len(&self) -> usize; + + /// Get a pointer to the start of the stream. + fn as_mut_ptr(&mut self) -> *mut u8; +} + +unsafe impl ByteReadBuffer for &mut [u8] { + fn len(&self) -> usize { + // Bit of a fun one since we reuse the names of the original functions + <[u8]>::len(self) + } + + fn as_mut_ptr(&mut self) -> *mut u8 { + // Bit of a fun one since we reuse the names of the original functions + <[u8]>::as_mut_ptr(self) + } +} + +unsafe impl ByteReadBuffer for &mut [MaybeUninit] { + fn len(&self) -> usize { + <[MaybeUninit]>::len(self) + } + + fn as_mut_ptr(&mut self) -> *mut u8 { + <[MaybeUninit]>::as_mut_ptr(self) as *mut u8 + } +} + +/// Read from a stream until either the byte slice as been filled, or the stream doesn't contain any +/// data anymore. This correctly handles streams that only allow smaller, buffered reads. If the +/// stream ended before the entire slice has been filled, then this will return `false`. +pub fn read_stream(stream: &clap_istream, mut slice: impl ByteReadBuffer) -> bool { + let mut read_pos = 0; + while read_pos < slice.len() { + let bytes_read = unsafe { + (stream.read)( + stream, + slice.as_mut_ptr().add(read_pos) as *mut c_void, + (slice.len() - read_pos) as u64, + ) + }; + if bytes_read <= 0 { + return false; + } + + read_pos += bytes_read as usize; + } + + true +} + +/// Write the data from a slice to a stream until either all data has been written, or the stream +/// returns an error. This correctly handles streams that only allow smaller, buffered writes. This +/// returns `false` if the stream returns an error or doesn't allow any writes anymore. +pub fn write_stream(stream: &clap_ostream, slice: &[u8]) -> bool { + let mut write_pos = 0; + while write_pos < slice.len() { + let bytes_written = unsafe { + (stream.write)( + stream, + slice.as_ptr().add(write_pos) as *const c_void, + (slice.len() - write_pos) as u64, + ) + }; + if bytes_written <= 0 { + return false; + } + + write_pos += bytes_written as usize; + } + + true +} diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index c302aa4a..5a3d7c5e 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -82,6 +82,7 @@ use crate::plugin::{ ProcessStatus, }; use crate::util::permit_alloc; +use crate::wrapper::clap::util::{read_stream, write_stream}; use crate::wrapper::state::{self, PluginState}; use crate::wrapper::util::{hash_param_id, process_wrapper, strlcpy}; @@ -2872,19 +2873,19 @@ impl Wrapper

{ // CLAP does not provide a way to tell how much data there is left in a stream, so // we need to prepend it to our actual state data. let length_bytes = (serialized.len() as u64).to_le_bytes(); - let num_length_bytes_written = ((*stream).write)( - stream, - length_bytes.as_ptr() as *const c_void, - length_bytes.len() as u64, - ); - let num_bytes_written = ((*stream).write)( - stream, - serialized.as_ptr() as *const c_void, - serialized.len() as u64, - ); + if !write_stream(&*stream, &length_bytes) { + nih_debug_assert_failure!( + "Error or end of stream while writing the state length to the stream." + ); + return false; + } + if !write_stream(&*stream, &serialized) { + nih_debug_assert_failure!( + "Error or end of stream while writing the state buffer to the stream." + ); + return false; + } - nih_debug_assert_eq!(num_length_bytes_written as usize, length_bytes.len()); - nih_debug_assert_eq!(num_bytes_written as usize, serialized.len()); true } Err(err) => { @@ -2903,22 +2904,22 @@ impl Wrapper

{ // CLAP does not have a way to tell how much data there is left in a stream, so we've // prepended the size in front of our JSON state - let mut length_bytes = [0; 8]; - let num_length_bytes_read = ((*stream).read)( - stream, - length_bytes.as_mut_ptr() as *mut c_void, - length_bytes.len() as u64, - ); - nih_debug_assert_eq!(num_length_bytes_read as usize, length_bytes.len()); + let mut length_bytes = [0u8; 8]; + if !read_stream(&*stream, length_bytes.as_mut_slice()) { + nih_debug_assert_failure!( + "Error or end of stream while reading the state length from the stream." + ); + return false; + } let length = u64::from_le_bytes(length_bytes); let mut read_buffer: Vec = Vec::with_capacity(length as usize); - let num_bytes_read = ((*stream).read)( - stream, - read_buffer.as_mut_ptr() as *mut c_void, - length as u64, - ); - nih_debug_assert_eq!(num_bytes_read as u64, length); + if !read_stream(&*stream, read_buffer.spare_capacity_mut()) { + nih_debug_assert_failure!( + "Error or end of stream while reading the state buffer from the stream." + ); + return false; + } read_buffer.set_len(length as usize); let success = state::deserialize_json(