From 7dfaf7d67982ed74b114191e36df0003fca49275 Mon Sep 17 00:00:00 2001 From: Vasili Novikov Date: Wed, 6 Sep 2023 09:57:21 +0200 Subject: [PATCH] Make PositionMonitor safe by using checked overflowing operations --- intel-sgx/enclave-runner/src/usercalls/mod.rs | 3 ++- ipc-queue/src/lib.rs | 2 +- ipc-queue/src/position.rs | 16 +++++++++++----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/intel-sgx/enclave-runner/src/usercalls/mod.rs b/intel-sgx/enclave-runner/src/usercalls/mod.rs index 2f2e3c00..438567ff 100644 --- a/intel-sgx/enclave-runner/src/usercalls/mod.rs +++ b/intel-sgx/enclave-runner/src/usercalls/mod.rs @@ -31,7 +31,8 @@ use tokio::io::{AsyncRead, AsyncWrite}; use tokio::stream::Stream as TokioStream; use tokio::sync::{broadcast, mpsc as async_mpsc, oneshot, Semaphore}; use fortanix_sgx_abi::*; -use ipc_queue::{self, DescriptorGuard, Identified, QueueEvent, WritePosition}; +use ipc_queue::{self, DescriptorGuard, Identified, QueueEvent}; +use ipc_queue::position::WritePosition; use sgxs::loader::Tcs as SgxsTcs; use crate::loader::{EnclavePanic, ErasedTcs}; diff --git a/ipc-queue/src/lib.rs b/ipc-queue/src/lib.rs index e21ffbbe..279b7800 100644 --- a/ipc-queue/src/lib.rs +++ b/ipc-queue/src/lib.rs @@ -29,7 +29,7 @@ use { mod fifo; mod interface_sync; mod interface_async; -mod position; +pub mod position; #[cfg(test)] mod test_support; diff --git a/ipc-queue/src/position.rs b/ipc-queue/src/position.rs index 928a8ef1..e70461e3 100644 --- a/ipc-queue/src/position.rs +++ b/ipc-queue/src/position.rs @@ -13,8 +13,8 @@ use std::sync::atomic::Ordering; /// read to/from the queue. This is useful in case we want to know whether or /// not a particular value written to the queue has been read. pub struct PositionMonitor { - read_epoch: Arc, - fifo: Fifo, + pub(crate) read_epoch: Arc, + pub(crate) fifo: Fifo, } /// A read position in a queue. @@ -27,7 +27,10 @@ impl PositionMonitor { pub fn read_position(&self) -> ReadPosition { let current = self.fifo.current_offsets(Ordering::Relaxed); let read_epoch = self.read_epoch.load(Ordering::Relaxed); - ReadPosition(((read_epoch as u64) << 32) | (current.read_offset() as u64)) + let read_epoch_shifted = read_epoch + .checked_shl(32) + .expect("Reading from position of over 2^32 (2 to the power of 32). This is unsupported."); + ReadPosition(read_epoch_shifted | (current.read_offset() as u64)) } pub fn write_position(&self) -> WritePosition { @@ -36,7 +39,10 @@ impl PositionMonitor { if current.read_high_bit() != current.write_high_bit() { write_epoch += 1; } - WritePosition(((write_epoch as u64) << 32) | (current.write_offset() as u64)) + let write_epoch_shifted = write_epoch + .checked_shl(32) + .expect("Writing to position of over 2^32 (2 to the power of 32). This is unsupported."); + WritePosition(write_epoch_shifted | (current.write_offset() as u64)) } } @@ -62,4 +68,4 @@ impl ReadPosition { } true } -} +} \ No newline at end of file