Skip to content
Open
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
35 changes: 21 additions & 14 deletions library/std/src/sys/net/connection/uefi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,26 @@ pub struct TcpStream {
}

impl TcpStream {
fn new(inner: tcp::Tcp) -> Self {
Self {
inner,
read_timeout: Arc::new(Mutex::new(None)),
write_timeout: Arc::new(Mutex::new(None)),
}
}

pub fn connect<A: ToSocketAddrs>(addr: A) -> io::Result<TcpStream> {
return each_addr(addr, inner);

fn inner(addr: &SocketAddr) -> io::Result<TcpStream> {
let inner = tcp::Tcp::connect(addr, None)?;
Ok(TcpStream {
inner,
read_timeout: Arc::new(Mutex::new(None)),
write_timeout: Arc::new(Mutex::new(None)),
})
Ok(TcpStream::new(inner))
}
}

pub fn connect_timeout(addr: &SocketAddr, timeout: Duration) -> io::Result<TcpStream> {
let inner = tcp::Tcp::connect(addr, Some(timeout))?;
Ok(Self {
inner,
read_timeout: Arc::new(Mutex::new(None)),
write_timeout: Arc::new(Mutex::new(None)),
})
Ok(Self::new(inner))
}

pub fn set_read_timeout(&self, t: Option<Duration>) -> io::Result<()> {
Expand Down Expand Up @@ -150,16 +150,23 @@ pub struct TcpListener {
}

impl TcpListener {
pub fn bind<A: ToSocketAddrs>(_: A) -> io::Result<TcpListener> {
unsupported()
pub fn bind<A: ToSocketAddrs>(addr: A) -> io::Result<TcpListener> {
return each_addr(addr, inner);

fn inner(addr: &SocketAddr) -> io::Result<TcpListener> {
let inner = tcp::Tcp::bind(addr)?;
Ok(TcpListener { inner })
}
}

pub fn socket_addr(&self) -> io::Result<SocketAddr> {
unsupported()
self.inner.socket_addr()
}

pub fn accept(&self) -> io::Result<(TcpStream, SocketAddr)> {
unsupported()
let tcp = self.inner.accept()?;
let addr = tcp.peer_addr()?;
Ok((TcpStream::new(tcp), addr))
}

pub fn duplicate(&self) -> io::Result<TcpListener> {
Expand Down
19 changes: 18 additions & 1 deletion library/std/src/sys/net/connection/uefi/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,24 @@ impl Tcp {
temp.connect(timeout)?;
Ok(Tcp::V4(temp))
}
SocketAddr::V6(_) => todo!(),
SocketAddr::V6(_) => unsupported(),
}
}

pub(crate) fn bind(addr: &SocketAddr) -> io::Result<Self> {
match addr {
SocketAddr::V4(x) => {
let temp = tcp4::Tcp4::new()?;
temp.configure(false, None, Some(x))?;
Ok(Tcp::V4(temp))
}
SocketAddr::V6(_) => unsupported(),
}
}

pub(crate) fn accept(&self) -> io::Result<Self> {
match self {
Self::V4(client) => client.accept().map(Tcp::V4),
}
}

Expand Down
78 changes: 63 additions & 15 deletions library/std/src/sys/net/connection/uefi/tcp4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use r_efi::protocols::tcp4;

use crate::io;
use crate::net::SocketAddrV4;
use crate::ptr::NonNull;
use crate::ptr::{self, NonNull};
use crate::sync::atomic::{AtomicBool, Ordering};
use crate::sys::pal::helpers;
use crate::time::{Duration, Instant};
Expand All @@ -12,20 +12,21 @@ const TYPE_OF_SERVICE: u8 = 8;
const TIME_TO_LIVE: u8 = 255;

pub(crate) struct Tcp4 {
handle: NonNull<crate::ffi::c_void>,
protocol: NonNull<tcp4::Protocol>,
flag: AtomicBool,
#[expect(dead_code)]
service_binding: helpers::ServiceProtocol,
}

const DEFAULT_ADDR: efi::Ipv4Address = efi::Ipv4Address { addr: [0u8; 4] };

impl Tcp4 {
pub(crate) fn new() -> io::Result<Self> {
let service_binding = helpers::ServiceProtocol::open(tcp4::SERVICE_BINDING_PROTOCOL_GUID)?;
let protocol = helpers::open_protocol(service_binding.child_handle(), tcp4::PROTOCOL_GUID)?;
let (service_binding, handle) =
helpers::ServiceProtocol::open(tcp4::SERVICE_BINDING_PROTOCOL_GUID)?;
let protocol = helpers::open_protocol(handle, tcp4::PROTOCOL_GUID)?;

Ok(Self { service_binding, protocol, flag: AtomicBool::new(false) })
Ok(Self { service_binding, handle, protocol, flag: AtomicBool::new(false) })
}

pub(crate) fn configure(
Expand All @@ -42,11 +43,14 @@ impl Tcp4 {
(DEFAULT_ADDR, 0)
};

// FIXME: Remove when passive connections with proper subnet handling are added
assert!(station_address.is_none());
let use_default_address = efi::Boolean::TRUE;
let (station_address, station_port) = (DEFAULT_ADDR, 0);
let subnet_mask = helpers::ipv4_to_r_efi(crate::net::Ipv4Addr::new(0, 0, 0, 0));
let use_default_address: r_efi::efi::Boolean = station_address.is_none().into();
let (station_address, station_port) = if let Some(x) = station_address {
(helpers::ipv4_to_r_efi(*x.ip()), x.port())
} else {
(DEFAULT_ADDR, 0)
};
let subnet_mask = crate::net::Ipv4Addr::new(255, 255, 255, 0);
let subnet_mask = helpers::ipv4_to_r_efi(subnet_mask);

let mut config_data = tcp4::ConfigData {
type_of_service: TYPE_OF_SERVICE,
Expand All @@ -60,7 +64,7 @@ impl Tcp4 {
station_port,
subnet_mask,
},
control_option: crate::ptr::null_mut(),
control_option: ptr::null_mut(),
};

let r = unsafe { ((*protocol).configure)(protocol, &mut config_data) };
Expand All @@ -74,17 +78,55 @@ impl Tcp4 {
let r = unsafe {
((*protocol).get_mode_data)(
protocol,
crate::ptr::null_mut(),
ptr::null_mut(),
&mut config_data,
crate::ptr::null_mut(),
crate::ptr::null_mut(),
crate::ptr::null_mut(),
ptr::null_mut(),
ptr::null_mut(),
ptr::null_mut(),
)
};

if r.is_error() { Err(io::Error::from_raw_os_error(r.as_usize())) } else { Ok(config_data) }
}

pub(crate) fn accept(&self) -> io::Result<Self> {
let evt = unsafe { self.create_evt() }?;
let completion_token =
tcp4::CompletionToken { event: evt.as_ptr(), status: Status::SUCCESS };
let mut listen_token =
tcp4::ListenToken { completion_token, new_child_handle: ptr::null_mut() };

let protocol = self.protocol.as_ptr();
let r = unsafe { ((*protocol).accept)(protocol, &mut listen_token) };
if r.is_error() {
return Err(io::Error::from_raw_os_error(r.as_usize()));
}

unsafe { self.wait_or_cancel(None, &mut listen_token.completion_token) }?;

if completion_token.status.is_error() {
Err(io::Error::from_raw_os_error(completion_token.status.as_usize()))
} else {
// EDK2 internals seem to assume a single ServiceBinding Protocol for TCP4 and TCP6, and
// thus does not use any service binding protocol data in destroying child sockets. It
// does seem to suggest that we need to cleanup even the protocols created by accept. To
// be on the safe side with other implementations, we will be using the same service
// binding protocol as the parent TCP4 handle.
//
// https://github.com/tianocore/edk2/blob/f80580f56b267c96f16f985dbf707b2f96947da4/NetworkPkg/TcpDxe/TcpDriver.c#L938

let handle = NonNull::new(listen_token.new_child_handle).unwrap();
let protocol = helpers::open_protocol(handle, tcp4::PROTOCOL_GUID)?;

Ok(Self {
handle,
service_binding: self.service_binding,
protocol,
flag: AtomicBool::new(false),
})
}
}

pub(crate) fn connect(&self, timeout: Option<Duration>) -> io::Result<()> {
let evt = unsafe { self.create_evt() }?;
let completion_token =
Expand Down Expand Up @@ -263,6 +305,12 @@ impl Tcp4 {
}
}

impl Drop for Tcp4 {
fn drop(&mut self) {
let _ = self.service_binding.destroy_child(self.handle);
}
}

extern "efiapi" fn toggle_atomic_flag(_: r_efi::efi::Event, ctx: *mut crate::ffi::c_void) {
let flag = unsafe { AtomicBool::from_ptr(ctx.cast()) };
flag.store(true, Ordering::Relaxed);
Expand Down
33 changes: 16 additions & 17 deletions library/std/src/sys/pal/uefi/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,14 +646,21 @@ pub(crate) fn get_device_path_from_map(map: &Path) -> io::Result<BorrowedDeviceP

/// Helper for UEFI Protocols which are created and destroyed using
/// [EFI_SERVICE_BINDING_PROTOCOL](https://uefi.org/specs/UEFI/2.11/11_Protocols_UEFI_Driver_Model.html#efi-service-binding-protocol)
///
/// SAFETY: Copying ServiceProtocol is safe as long as handle is valid. For most service binding
/// protocols, at least in edk2 implementations, the lifetime of these handles will be till UEFI is
/// active, i.e. 'static
#[derive(Clone, Copy)]
pub(crate) struct ServiceProtocol {
service_guid: r_efi::efi::Guid,
handle: NonNull<crate::ffi::c_void>,
child_handle: NonNull<crate::ffi::c_void>,
}
Comment on lines +649 to 657
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SAFETY comment shouldn't be here since duplicating this struct doesn't do anything by itself. If the handle is expected to be valid for 'static, this would be better expressed as an /// Invariant: ... comment on the handle field. Then:

  1. For unsafe calls that use handle, you can say they are valid by the invariant
  2. For places where ServiceProtocol is constructed, you can add an // INVARIANT: comment saying why they meet that invariant. And call out the assumption.

For most service binding protocols, at least in edk2 implementations, the lifetime of these handles will be till UEFI is active, i.e. 'static

Are there cases where a relevant handle here will disappear earlier that we need to worry about?


impl ServiceProtocol {
pub(crate) fn open(service_guid: r_efi::efi::Guid) -> io::Result<Self> {
/// Open a child handle on a service_binding protocol.
pub(crate) fn open(
service_guid: r_efi::efi::Guid,
) -> io::Result<(Self, NonNull<crate::ffi::c_void>)> {
Comment on lines 654 to +663
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type Handle = NonNull<crate::ffi::c_void> may be a worthwhile typedef since it's used a few times in this file and in Tcp4

let handles = locate_handles(service_guid)?;

for handle in handles {
Expand All @@ -662,17 +669,13 @@ impl ServiceProtocol {
continue;
};

return Ok(Self { service_guid, handle, child_handle });
return Ok((Self { service_guid, handle }, child_handle));
}
}

Err(io::const_error!(io::ErrorKind::NotFound, "no service binding protocol found"))
}

pub(crate) fn child_handle(&self) -> NonNull<crate::ffi::c_void> {
self.child_handle
}

fn create_child(
sbp: NonNull<service_binding::Protocol>,
) -> io::Result<NonNull<crate::ffi::c_void>> {
Comment on lines 679 to 681
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think this needs to be unsafe too for the same reason

Expand All @@ -687,17 +690,13 @@ impl ServiceProtocol {
.ok_or(const_error!(io::ErrorKind::Other, "null child handle"))
}
}
}

impl Drop for ServiceProtocol {
fn drop(&mut self) {
if let Ok(sbp) = open_protocol::<service_binding::Protocol>(self.handle, self.service_guid)
{
// SAFETY: Child handle must be allocated by the current service binding protocol.
let _ = unsafe {
((*sbp.as_ptr()).destroy_child)(sbp.as_ptr(), self.child_handle.as_ptr())
};
}
pub(crate) fn destroy_child(&self, handle: NonNull<crate::ffi::c_void>) -> io::Result<()> {
let sbp = open_protocol::<service_binding::Protocol>(self.handle, self.service_guid)?;

// SAFETY: Child handle must be allocated by the current service binding protocol.
let r = unsafe { ((*sbp.as_ptr()).destroy_child)(sbp.as_ptr(), handle.as_ptr()) };
if r.is_error() { Err(crate::io::Error::from_raw_os_error(r.as_usize())) } else { Ok(()) }
}
Comment on lines +694 to 700
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be unsafe if the precondition of the destroy_child call depends on the validity of handle.

Also, is it safe to call destroy_child with the same handle more than once?

}

Expand Down
Loading