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

Change Logger to use an atomic pointer internally #978

Merged
merged 2 commits into from Oct 27, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,9 @@
had to add `&BootServices` as additional parameter.
- `BootServices::free_pages` and `BootServices::free_pool` are now `unsafe` to
call, since it is possible to trigger UB by freeing memory that is still in use.
- `Logger` no longer requires exterior mutability. `Logger::new` is now `const`,
takes no arguments, and creates the logger in a disabled state. Call
`Logger::set_output` to enable it.

## uefi-macros - [Unreleased]

Expand Down
17 changes: 5 additions & 12 deletions uefi-services/src/lib.rs
Expand Up @@ -54,7 +54,7 @@ static mut SYSTEM_TABLE: Option<SystemTable<Boot>> = None;

/// Global logger object
#[cfg(feature = "logger")]
static mut LOGGER: Option<uefi::logger::Logger> = None;
static LOGGER: uefi::logger::Logger = uefi::logger::Logger::new();

/// Obtains a pointer to the system table.
///
Expand Down Expand Up @@ -161,16 +161,11 @@ macro_rules! println {
/// disable() on exit from UEFI boot services.
#[cfg(feature = "logger")]
unsafe fn init_logger(st: &mut SystemTable<Boot>) {
let stdout = st.stdout();

// Construct the logger.
let logger = {
LOGGER = Some(uefi::logger::Logger::new(stdout));
LOGGER.as_ref().unwrap()
};
// Connect the logger to stdout.
LOGGER.set_output(st.stdout());

// Set the logger.
log::set_logger(logger).unwrap(); // Can only fail if already initialized.
log::set_logger(&LOGGER).unwrap(); // Can only fail if already initialized.

// Set logger max level to level specified by log features
log::set_max_level(log::STATIC_MAX_LEVEL);
Expand All @@ -188,9 +183,7 @@ unsafe extern "efiapi" fn exit_boot_services(_e: Event, _ctx: Option<NonNull<c_v
SYSTEM_TABLE = None;

#[cfg(feature = "logger")]
if let Some(ref mut logger) = LOGGER {
logger.disable();
}
LOGGER.disable();

uefi::allocator::exit_boot_services();
}
Expand Down
57 changes: 41 additions & 16 deletions uefi/src/logger.rs
Expand Up @@ -15,7 +15,8 @@
use crate::proto::console::text::Output;

use core::fmt::{self, Write};
use core::ptr::NonNull;
use core::ptr;
use core::sync::atomic::{AtomicPtr, Ordering};

/// Logging implementation which writes to a UEFI output stream.
///
Expand All @@ -24,39 +25,63 @@ use core::ptr::NonNull;
/// undefined behaviour from inadvertent logging.
#[derive(Debug)]
pub struct Logger {
writer: Option<NonNull<Output>>,
writer: AtomicPtr<Output>,
}

impl Logger {
/// Creates a new logger.
///
/// You must arrange for the `disable` method to be called or for this logger
/// to be otherwise discarded before boot services are exited.
/// The logger is initially disabled. Call [`set_output`] to enable it.
///
/// # Safety
///
/// Undefined behaviour may occur if this logger is still active after the
/// application has exited the boot services stage.
pub unsafe fn new(output: &mut Output) -> Self {
/// [`set_output`]: Self::set_output
#[must_use]
pub const fn new() -> Self {
Logger {
writer: NonNull::new(output as *const _ as *mut _),
writer: AtomicPtr::new(ptr::null_mut()),
}
}

/// Disable the logger
pub fn disable(&mut self) {
self.writer = None;
/// Get the output pointer (may be null).
#[must_use]
fn output(&self) -> *mut Output {
self.writer.load(Ordering::Acquire)
}

/// Set the [`Output`] to which the logger will write.
///
/// If a null pointer is passed for `output`, this method is equivalent to
/// calling [`disable`].
///
/// # Safety
///
/// The `output` pointer must either be null or point to a valid [`Output`]
/// object. That object must remain valid until the logger is either
/// disabled, or `set_output` is called with a different `output`.
///
/// You must arrange for the [`disable`] method to be called or for this
/// logger to be otherwise discarded before boot services are exited.
///
/// [`disable`]: Self::disable
pub unsafe fn set_output(&self, output: *mut Output) {
self.writer.store(output, Ordering::Release);
}

/// Disable the logger.
pub fn disable(&self) {
unsafe { self.set_output(ptr::null_mut()) }
}
}

impl log::Log for Logger {
fn enabled(&self, _metadata: &log::Metadata) -> bool {
self.writer.is_some()
!self.output().is_null()
}

fn log(&self, record: &log::Record) {
if let Some(mut ptr) = self.writer {
let writer = unsafe { ptr.as_mut() };
let output = self.output();

if !output.is_null() {
let writer = unsafe { &mut *output };
let result = DecoratedLog::write(
writer,
record.level(),
Expand Down