Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit acaf7ac1 authored by Andrew Walbran's avatar Andrew Walbran
Browse files

Add safety comments and TODO. 

The way sigaction is currently used is unsound.

Bug: 290018030
Bug: 292218119
Test: m rust
Change-Id: Ib38b115b3b1e2de0a8b4a612d56112268d22985f
parent 4875d0fe

system/gd/rust/facade/src/main.rs

+10 −2
Original line number Diff line number Diff line
@@ -13,7 +13,8 @@ use std::sync::{Arc, Mutex}; 
use tokio::runtime::Runtime;



fn main() {

    let sigint = install_sigint();

    // SAFETY: There is no signal handler installed before this.

    let sigint = unsafe { install_sigint() };

    bt_common::init_logging();

    let rt = Arc::new(Runtime::new().unwrap());

    rt.block_on(async_main(Arc::clone(&rt), sigint));
@@ -70,7 +71,10 @@ async fn async_main(rt: Arc<Runtime>, mut sigint: mpsc::UnboundedReceiver<()>) { 
}



// TODO: remove as this is a temporary nix-based hack to catch SIGINT

fn install_sigint() -> mpsc::UnboundedReceiver<()> {

/// # Safety

///

/// The old signal handler, if any, must be installed correctly.

unsafe fn install_sigint() -> mpsc::UnboundedReceiver<()> {

    let (tx, rx) = mpsc::unbounded();

    *SIGINT_TX.lock().unwrap() = Some(tx);
@@ -79,6 +83,10 @@ fn install_sigint() -> mpsc::UnboundedReceiver<()> { 
        signal::SaFlags::empty(),

        signal::SigSet::empty(),

    );

    // SAFETY: The caller guarantees that the old signal handler was installed correctly.

    // TODO(b/292218119): Make sure `handle_sigint` only makes system calls that are safe for signal

    // handlers, and only accesses global state through atomics. In particular, it must not take any

    // shared locks.

    unsafe {

        signal::sigaction(signal::SIGINT, &sig_action).unwrap();

    }

system/gd/rust/topshim/facade/src/main.rs

+10 −2
Original line number Diff line number Diff line
@@ -36,7 +36,8 @@ use bluetooth_core_rs_for_facade::*; 
use bt_shim::*;



fn main() {

    let sigint = install_sigint();

    // SAFETY: There is no signal handler installed before this.

    let sigint = unsafe { install_sigint() };

    bt_common::init_logging();

    let rt = Arc::new(Runtime::new().unwrap());

    rt.block_on(async_main(Arc::clone(&rt), sigint));
@@ -123,7 +124,10 @@ async fn async_main(rt: Arc<Runtime>, mut sigint: mpsc::UnboundedReceiver<()>) { 
}



// TODO: remove as this is a temporary nix-based hack to catch SIGINT

fn install_sigint() -> mpsc::UnboundedReceiver<()> {

/// # Safety

///

/// The old signal handler, if any, must be installed correctly.

unsafe fn install_sigint() -> mpsc::UnboundedReceiver<()> {

    let (tx, rx) = mpsc::unbounded();

    *SIGINT_TX.lock().unwrap() = Some(tx);
@@ -132,6 +136,10 @@ fn install_sigint() -> mpsc::UnboundedReceiver<()> { 
        signal::SaFlags::empty(),

        signal::SigSet::empty(),

    );

    // SAFETY: The caller guarantees that the old signal handler was installed correctly.

    // TODO(b/292218119): Make sure `handle_sigint` only makes system calls that are safe for signal

    // handlers, and only accesses global state through atomics. In particular, it must not take any

    // shared locks.

    unsafe {

        signal::sigaction(signal::SIGINT, &sig_action).unwrap();

    }