Skip to content

Commit

Permalink
[#61] Fix undefined behavior in FixedSizeString by removing const fro…
Browse files Browse the repository at this point in the history
…m new_unchecked
  • Loading branch information
elfenpiff committed Jan 2, 2024
1 parent c3622ca commit 7d027b3
Show file tree
Hide file tree
Showing 45 changed files with 264 additions and 236 deletions.
1 change: 1 addition & 0 deletions doc/release-notes/iceoryx2-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

### Bugfixes

* Fix undefined behavior in `FixedSizeByteString::new_unchecked` [#61](https://github.com/eclipse-iceoryx/iceoryx2/issues/61)
* Fix suffix of static config [#66](https://github.com/eclipse-iceoryx/iceoryx2/issues/66)

### Refactoring
Expand Down
15 changes: 2 additions & 13 deletions iceoryx2-bb/container/src/byte_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,23 +207,12 @@ impl<const CAPACITY: usize> FixedSizeByteString<CAPACITY> {
///
/// * `bytes` len must be smaller or equal than [`FixedSizeByteString::capacity()`]
///
pub const unsafe fn new_unchecked(bytes: &[u8]) -> Self {
pub unsafe fn new_unchecked(bytes: &[u8]) -> Self {
if CAPACITY < bytes.len() {
panic!("Insufficient capacity to store bytes.");
}

let mut new_self = Self::new();
new_self.len = bytes.len();
std::ptr::copy(
bytes.as_ptr(),
new_self.data.as_ptr() as *mut u8,
bytes.len(),
);

let zero = 0u8;
std::ptr::copy(&zero, new_self.data.as_ptr().add(bytes.len()) as *mut u8, 1);

new_self
Self::from_bytes_truncated(bytes)
}

/// Creates a new [`FixedSizeByteString`] from a byte slice
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/container/src/semantic_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ macro_rules! semantic_string {
}

impl $string_name {
pub const unsafe fn new_unchecked(bytes: &[u8]) -> Self {
pub unsafe fn new_unchecked(bytes: &[u8]) -> Self {
Self {
value: iceoryx2_bb_container::byte_string::FixedSizeByteString::new_unchecked(bytes),
}
Expand Down
17 changes: 11 additions & 6 deletions iceoryx2-bb/posix/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ pub const ADAPTIVE_WAIT_INITIAL_WAITING_TIME: Duration = Duration::from_micros(1
pub const ADAPTIVE_WAIT_FINAL_WAITING_TIME: Duration = Duration::from_millis(10);

// directories
pub const TEMP_DIRECTORY: Path =
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEMP_DIRECTORY) };
pub const TEST_DIRECTORY: Path =
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEST_DIRECTORY) };
pub const SHARED_MEMORY_DIRECTORY: Path =
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::SHARED_MEMORY_DIRECTORY) };
pub fn temp_directory() -> Path {
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEMP_DIRECTORY) }
}

pub fn test_directory() -> Path {
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEST_DIRECTORY) }
}

pub fn shared_memory_directory() -> Path {
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::SHARED_MEMORY_DIRECTORY) }
}

// TODO unable to verify?
pub const ACL_LIST_CAPACITY: u32 = 25;
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/directory_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl TestFixture {
}

fn generate_directory_name(&mut self) -> Path {
let mut directory = TEST_DIRECTORY;
let mut directory = test_directory();
directory.push(PATH_SEPARATOR).unwrap();
directory.push_bytes(b"dir_tests_").unwrap();
directory
Expand All @@ -107,7 +107,7 @@ impl TestFixture {

#[test]
fn directory_temp_directory_does_exist() {
assert_that!(Directory::does_exist(&TEST_DIRECTORY).unwrap(), eq true);
assert_that!(Directory::does_exist(&test_directory()).unwrap(), eq true);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/posix/tests/file_descriptor_set_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn generate_socket_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/posix/tests/file_descriptor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ trait GenericTestBuilder {

impl GenericTestBuilder for File {
fn sut() -> Self {
let name = FilePath::from_path_and_file(&TEST_DIRECTORY, &generate_name()).unwrap();
let name = FilePath::from_path_and_file(&test_directory(), &generate_name()).unwrap();

let file_content = [170u8; 2048];

Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/file_lock_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

use iceoryx2_bb_container::semantic_string::SemanticString;
use iceoryx2_bb_posix::config::*;
use iceoryx2_bb_posix::config;
use iceoryx2_bb_posix::file::*;
use iceoryx2_bb_posix::file_lock::*;
use iceoryx2_bb_posix::process::*;
Expand All @@ -38,7 +38,7 @@ fn generate_file_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&config::test_directory(), &file).unwrap()
}

const TIMEOUT: Duration = Duration::from_millis(10);
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/posix/tests/file_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn generate_file_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

struct TestFixture {
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/metadata_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use iceoryx2_pal_posix::posix::POSIX_SUPPORT_USERS_AND_GROUPS;
#[test]
fn metadata_reads_basic_stats_correctly() {
let file_name =
FilePath::from_path_and_file(&TEST_DIRECTORY, &FileName::new(b"meta_test").unwrap())
FilePath::from_path_and_file(&test_directory(), &FileName::new(b"meta_test").unwrap())
.unwrap();

let mut file = FileBuilder::new(&file_name)
Expand All @@ -50,7 +50,7 @@ fn metadata_reads_owner_and_permission_stats_correctly() {
test_requires!(POSIX_SUPPORT_USERS_AND_GROUPS && POSIX_SUPPORT_PERMISSIONS);

let file_name =
FilePath::from_path_and_file(&TEST_DIRECTORY, &FileName::new(b"meta_test_123").unwrap())
FilePath::from_path_and_file(&test_directory(), &FileName::new(b"meta_test_123").unwrap())
.unwrap();

let mut file = FileBuilder::new(&file_name)
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/socket_ancillary_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use iceoryx2_bb_container::semantic_string::SemanticString;
use iceoryx2_bb_elementary::unique_id::*;
use iceoryx2_bb_posix::config::*;
use iceoryx2_bb_posix::config;
use iceoryx2_bb_posix::file::*;
use iceoryx2_bb_posix::file_descriptor::*;
use iceoryx2_bb_posix::process::ProcessId;
Expand All @@ -29,7 +29,7 @@ fn generate_file_name() -> FilePath {
file.push_bytes(UniqueId::new().value().to_string().as_bytes())
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&config::test_directory(), &file).unwrap()
}

struct TestFixture {
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/unix_datagram_socket_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn generate_socket_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

fn generate_file_name() -> FilePath {
Expand All @@ -57,7 +57,7 @@ fn generate_file_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

struct TestFixture {
Expand Down
10 changes: 5 additions & 5 deletions iceoryx2-cal/src/communication_channel/message_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path_hint: DEFAULT_PATH_HINT,
suffix: Channel::<()>::default_suffix(),
prefix: Channel::<()>::default_prefix(),
path_hint: Channel::<()>::default_path_hint(),
}
}
}
Expand Down Expand Up @@ -190,7 +190,7 @@ struct SharedConfiguration {
}

#[derive(Debug)]
pub struct Creator<T> {
pub struct Creator<T: Copy + Debug> {
channel_name: FileName,
enable_safe_overflow: bool,
buffer_size: usize,
Expand Down Expand Up @@ -291,7 +291,7 @@ impl<T: Copy + Debug> CommunicationChannelCreator<T, Channel<T>> for Creator<T>
}

#[derive(Debug)]
pub struct Connector<T> {
pub struct Connector<T: Copy + Debug> {
channel_name: FileName,
config: Configuration,
_phantom_data: PhantomData<T>,
Expand Down
15 changes: 5 additions & 10 deletions iceoryx2-cal/src/communication_channel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ pub mod unix_datagram;

use std::fmt::Debug;

use iceoryx2_bb_posix::config::TEMP_DIRECTORY;
use iceoryx2_bb_system_types::file_name::FileName;
use iceoryx2_bb_system_types::path::Path;

Expand All @@ -81,15 +80,6 @@ use crate::named_concept::{NamedConcept, NamedConceptBuilder, NamedConceptMgmt};
/// The buffer size which the receiver has at least by default
pub const DEFAULT_RECEIVER_BUFFER_SIZE: usize = 8;

/// The default suffix of every communication channel
pub const DEFAULT_SUFFIX: FileName = unsafe { FileName::new_unchecked(b".com") };

/// The default prefix of every communication channel
pub const DEFAULT_PREFIX: FileName = unsafe { FileName::new_unchecked(b"iox2_") };

/// The default path hint for every communication channel
pub const DEFAULT_PATH_HINT: Path = TEMP_DIRECTORY;

/// Describes failures when sending data
#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq)]
pub enum CommunicationChannelSendError {
Expand Down Expand Up @@ -207,4 +197,9 @@ pub trait CommunicationChannel<T>: Sized + Debug + NamedConceptMgmt {
fn has_configurable_buffer_size() -> bool {
false
}

/// The default suffix of every communication channel
fn default_suffix() -> FileName {
unsafe { FileName::new_unchecked(b".com") }
}
}
6 changes: 3 additions & 3 deletions iceoryx2-cal/src/communication_channel/posix_shared_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
path_hint: DEFAULT_PATH_HINT,
prefix: DEFAULT_PREFIX,
suffix: Channel::default_suffix(),
path_hint: Channel::default_path_hint(),
prefix: Channel::default_prefix(),
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions iceoryx2-cal/src/communication_channel/process_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path_hint: DEFAULT_PATH_HINT,
suffix: Channel::default_suffix(),
prefix: Channel::default_prefix(),
path_hint: Channel::default_path_hint(),
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions iceoryx2-cal/src/communication_channel/unix_datagram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path_hint: DEFAULT_PATH_HINT,
suffix: Channel::<()>::default_suffix(),
prefix: Channel::<()>::default_prefix(),
path_hint: Channel::<()>::default_path_hint(),
}
}
}
Expand Down Expand Up @@ -165,7 +165,7 @@ impl<T: Copy + Debug> CommunicationChannel<T> for Channel<T> {
}

#[derive(Debug)]
pub struct Creator<T> {
pub struct Creator<T: Copy + Debug> {
channel_name: FileName,
enable_safe_overflow: bool,
buffer_size: usize,
Expand Down Expand Up @@ -240,7 +240,7 @@ impl<T: Copy + Debug> CommunicationChannelCreator<T, Channel<T>> for Creator<T>
}

#[derive(Debug)]
pub struct Connector<T> {
pub struct Connector<T: Copy + Debug> {
channel_name: FileName,
config: Configuration,
_phantom_data: PhantomData<T>,
Expand Down
16 changes: 5 additions & 11 deletions iceoryx2-cal/src/dynamic_storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,13 @@
use std::fmt::Debug;

use iceoryx2_bb_memory::bump_allocator::BumpAllocator;
use iceoryx2_bb_posix::config::TEMP_DIRECTORY;
use iceoryx2_bb_system_types::file_name::FileName;
use iceoryx2_bb_system_types::path::Path;

use crate::static_storage::file::{NamedConcept, NamedConceptBuilder, NamedConceptMgmt};

pub mod posix_shared_memory;
pub mod process_local;

/// The default suffix of every dynamic storage
pub const DEFAULT_SUFFIX: FileName = unsafe { FileName::new_unchecked(b".dyn") };

/// The default prefix of every dynamic storage
pub const DEFAULT_PREFIX: FileName = unsafe { FileName::new_unchecked(b"iox2_") };

/// The default path hint for every dynamic storage
pub const DEFAULT_PATH_HINT: Path = TEMP_DIRECTORY;

/// Describes failures when creating a new [`DynamicStorage`]
#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq)]
pub enum DynamicStorageCreateError {
Expand Down Expand Up @@ -155,4 +144,9 @@ pub trait DynamicStorage<T: Send + Sync>: Sized + Debug + NamedConceptMgmt + Nam
/// can be accessed by multiple processes concurrently therefore it must be constant or
/// thread-safe.
fn get(&self) -> &T;

/// The default suffix of every dynamic storage
fn default_suffix() -> FileName {
unsafe { FileName::new_unchecked(b".dyn") }
}
}
10 changes: 5 additions & 5 deletions iceoryx2-cal/src/dynamic_storage/posix_shared_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ const IS_INITIALIZED_STATE_VALUE: u64 = 0xbeefaffedeadbeef;

/// The builder of [`Storage`].
#[derive(Debug)]
pub struct Builder<T: Debug> {
pub struct Builder<T: Send + Sync + Debug> {
storage_name: FileName,
supplementary_size: usize,
has_ownership: bool,
config: Configuration,
_phantom_data: PhantomData<T>,
}

#[derive(Clone, Debug)]
#[derive(Debug, Clone)]
pub struct Configuration {
suffix: FileName,
prefix: FileName,
Expand All @@ -83,9 +83,9 @@ struct Data<T: Send + Sync + Debug> {
impl Default for Configuration {
fn default() -> Self {
Self {
path: DEFAULT_PATH_HINT,
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path: Storage::<()>::default_path_hint(),
suffix: Storage::<()>::default_suffix(),
prefix: Storage::<()>::default_prefix(),
}
}
}
Expand Down
Loading

0 comments on commit 7d027b3

Please sign in to comment.