Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a static buffer for panic messages #41

Merged
merged 1 commit into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions io/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ mc-sgx-util = "0.4.0"
[dev-dependencies]
once_cell = "1.16.0"
serial_test = "1.0.0"
yare = "1.0.1"
5 changes: 4 additions & 1 deletion io/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
// Copyright (c) 2022 The MobileCoin Foundation
// Copyright (c) 2022-2023 The MobileCoin Foundation

#![doc = include_str!("../README.md")]
#![deny(missing_docs, missing_debug_implementations)]
#![no_std]

mod write_buffer;

use core::ffi::c_void;
use mc_sgx_core_sys_types::sgx_status_t;
use mc_sgx_core_types::Error;
use mc_sgx_util::ResultInto;
pub use write_buffer::WriteBuffer;

/// Write the entire `buffer` into the hosts stderr sink.
///
Expand Down
157 changes: 157 additions & 0 deletions io/src/write_buffer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// Copyright (c) 2023 The MobileCoin Foundation
nick-mobilecoin marked this conversation as resolved.
Show resolved Hide resolved

//! Provides a sized buffer which implements the [`fmt::Write`] trait.

use core::fmt;

/// Byte size of [`WriteBuffer`].
///
/// Attempting to write more than this many bytes to the [`WriteBuffer`] will
/// result in an error.
pub const BUFFER_SIZE: usize = 4096;

/// A buffer which implements the [`fmt::Write`] trait.
#[derive(Debug)]
pub struct WriteBuffer {
buf: [u8; BUFFER_SIZE],
pos: usize,
}

impl WriteBuffer {
/// Create a new empty [`WriteBuffer`]
pub const fn new() -> Self {
WriteBuffer {
buf: [0; BUFFER_SIZE],
pos: 0,
}
}

/// Clear the contents in the [`WriteBuffer`]
pub fn clear(&mut self) {
self.pos = 0;
}
}

impl AsRef<str> for WriteBuffer {
fn as_ref(&self) -> &str {
// Shouldn't fail because [`Write::write_str()`] is the only public way
// to add content. [`Write::write_str()`] takes a `&str` so for this to
// fail someone must have coerced invalid UTF-8 to a string prior to
// this method invocation.
core::str::from_utf8(&self.buf[..self.pos])
.expect("`WriteBuffer` is not valid UTF-8. It should have only been given `&str`s")
}
}

impl AsRef<[u8]> for WriteBuffer {
fn as_ref(&self) -> &[u8] {
&self.buf[..self.pos]
}
}

impl fmt::Write for WriteBuffer {
fn write_str(&mut self, string: &str) -> fmt::Result {
let bytes = string.as_bytes();

let remaining = &mut self.buf[self.pos..];
if remaining.len() < bytes.len() {
return Err(fmt::Error);
}

let new_contents = &mut remaining[..bytes.len()];
new_contents.copy_from_slice(bytes);

self.pos += bytes.len();

Ok(())
}
}

#[cfg(test)]
mod test {
extern crate std;

use super::*;
use core::fmt::Write;
use std::vec;
use yare::parameterized;

#[test]
fn new_write_buffer_is_empty() {
let buffer = WriteBuffer::new();

let contents: &str = buffer.as_ref();
assert_eq!(contents, "");
}

#[parameterized(
what = {&["what"], "what"},
hello = {&["hello"], "hello"},
good_bye = {&["good", " ", "bye"], "good bye"},
i_like_long_strings = {&["i", " ", "like", " ", "long", " ", "strings"], "i like long strings"},
no_spaces = {&["no", "spaces"], "nospaces"},
)]
fn write_buffer_contains_the_written_contents(messages: &[&str], expected: &str) {
let mut buffer = WriteBuffer::new();

for message in messages {
buffer
.write_str(message)
.expect("Shouldn't fail to write message");
}

let contents: &str = buffer.as_ref();
assert_eq!(contents, expected);
}

#[parameterized(
why_not = {b"why not"},
you_bet = {b"you_bet"},
)]
fn write_buffer_as_bytes(message: &[u8]) {
let mut buffer = WriteBuffer::new();
let message_str = core::str::from_utf8(message).expect("Message should be valid UTF-8");

buffer
.write_str(message_str)
.expect("Shouldn't fail to write message");

let contents: &[u8] = buffer.as_ref();
assert_eq!(contents, message);
}

#[test]
fn write_buffer_can_hold_4096_bytes() {
let mut buffer = WriteBuffer::new();
// 66 == 'B'
let mut message = vec![66u8; BUFFER_SIZE - 1];
let message_str = core::str::from_utf8(&message).expect("Message should be valid UTF-8");

buffer
.write_str(message_str)
.expect("Shouldn't fail to write message");
buffer
.write_str("C")
.expect("Shouldn't fail to write last byte");

let contents: &[u8] = buffer.as_ref();
message.push(67);
assert_eq!(contents, message);
}

#[test]
fn write_buffer_errors_at_4097_bytes() {
let mut buffer = WriteBuffer::new();
let message = [66u8; BUFFER_SIZE - 1];
let message_str = core::str::from_utf8(&message).expect("Message should be valid UTF-8");

buffer
.write_str(message_str)
.expect("Shouldn't fail to write message");
buffer
.write_str("C")
.expect("Shouldn't fail to write last byte");

assert!(buffer.write_str("D").is_err());
}
}
1 change: 1 addition & 0 deletions panic/log/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ keywords = ["sgx", "no-std", "panic"]

[dependencies]
mc-sgx-io = { path = "../../io", version = "0.1.0" }
mc-sgx-sync = { path = "../../sync", version = "0.1.0" }

# This is a crate that can only be built for an SGX target, so it's not part of
# the root workspace. Because of this limitation we must re-iterate the
Expand Down
38 changes: 32 additions & 6 deletions panic/log/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,41 @@
#![deny(missing_docs, missing_debug_implementations)]
#![no_std]

extern crate alloc;
use alloc::string::ToString;
use core::fmt::Write;
use core::panic::PanicInfo;
use mc_sgx_io::WriteBuffer;
use mc_sgx_sync::Mutex;

/// A buffer for building up the panic message.
/// We avoid allocating when handling the panic as failure to allocate could be
/// the cause of the panic.
static MESSAGE_BUFFER: Mutex<WriteBuffer> = Mutex::new(WriteBuffer::new());

#[panic_handler]
fn panic(info: &PanicInfo) -> ! {
// Ignore the result, we're already panicking we can't really do much if
// `stderr_write_all()` fails
let _ = mc_sgx_io::stderr_write_all(info.to_string().as_bytes());

log_panic_info(info);
loop {}
nick-mobilecoin marked this conversation as resolved.
Show resolved Hide resolved
}

/// Log information during a panic
///
/// If for some reason the `info` exceeds the size of the [`MESSAGE_BUFFER`]
/// then this will log a default message.
///
/// # Arguments:
/// * `info` - The panic information to log
fn log_panic_info(info: &PanicInfo) {
if let Ok(mut buffer) = MESSAGE_BUFFER.lock() {
buffer.clear();
let message = match write!(buffer, "{info}") {
Ok(()) => buffer.as_ref(),
_ => "Failed to format panic log info.",
};

// Ignore the result, we're already panicking we can't really do much if
// `stderr_write_all()` fails
let _ = mc_sgx_io::stderr_write_all(message.as_bytes());
} else {
let _ = mc_sgx_io::stderr_write_all(b"Mutex for panic logging has been poisoned.");
}
}