crates/systemd-socket: src/lib.rs comparison

comparison src/lib.rs @ 25:8e20daee41ed

Set CLOEXEC flag on the descriptors The received systemd descriptors don't have `O_CLOEXEC` set because they are received over `exec`. Thus if the process executes a child the child gets polluted with the descriptors. To prevent this, we set `O_CLOEXEC` during initialization. However this also required restructuring of the code because `libsystemd` doesn't provide temporary access to the descriptors - only permanent one. Thus we have to "validate" the descriptors eagerly. We still store the invalid ones as errors to make sure the errors get reported accurately.

author	Martin Habovstiak <martin.habovstiak@gmail.com>
date	Fri, 28 Feb 2025 21:11:19 +0100
parents	1941e9d9819c
children	0feab4f4c2ce

comparison

equal deleted inserted replaced

-:1941e9d9819c
+:8e20daee41ed
 }
 Ok(())
 }
 }
+type StoredSocket = Result<Socket, ()>;
 // No source we can't keep the mutex locked
 impl std::error::Error for Error {}
 pub(crate) unsafe fn init(protected: bool) -> Result<(), InitError> {
 SYSTEMD_SOCKETS.get_or_try_init(|| SystemdSockets::new(protected, true).map(Ok)).map(drop)
 }
-pub(crate) fn take(name: &str) -> Result<Option<FileDescriptor>, Error> {
+pub(crate) fn take(name: &str) -> Result<Option<StoredSocket>, Error> {
 let sockets = SYSTEMD_SOCKETS.get_or_init(|| SystemdSockets::new_protected(false).map_err(Mutex::new));
 match sockets {
 Ok(sockets) => Ok(sockets.take(name)),
 Err(error) => Err(Error(error))
 }
 Self::LibSystemd(error) => error.source(),
 }
 }
 }
-struct SystemdSockets(std::sync::Mutex<std::collections::HashMap<String, FileDescriptor>>);
+pub(crate) enum Socket {
+TcpListener(std::net::TcpListener),
+}
+impl std::convert::TryFrom<FileDescriptor> for Socket {
+type Error = ();
+fn try_from(value: FileDescriptor) -> Result<Self, Self::Error> {
+use libsystemd::activation::IsType;
+use std::os::unix::io::{FromRawFd, IntoRawFd, AsRawFd};
+fn set_cloexec(fd: std::os::unix::io::RawFd) {
+// SAFETY: The function is a harmless syscall
+let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) };
+if flags != -1 && flags & libc::FD_CLOEXEC == 0 {
+// We ignore errors, since the FD is still usable
+// SAFETY: socket is definitely a valid file descriptor and setting CLOEXEC is
+// a sound operation.
+unsafe {
+libc::fcntl(fd, libc::F_SETFD, flags | libc::FD_CLOEXEC);
+}
+}
+}
+if value.is_inet() {
+// SAFETY: FileDescriptor is obtained from systemd, so it should be valid.
+let socket = unsafe { std::net::TcpListener::from_raw_fd(value.into_raw_fd()) };
+set_cloexec(socket.as_raw_fd());
+Ok(Socket::TcpListener(socket))
+} else {
+// We still need to make the filedescriptor harmless.
+set_cloexec(value.into_raw_fd());
+Err(())
+}
+}
+}
+struct SystemdSockets(std::sync::Mutex<std::collections::HashMap<String, StoredSocket>>);
 impl SystemdSockets {
 fn new_protected(explicit: bool) -> Result<Self, InitError> {
 unsafe { Self::new(true, explicit) }
 }
 unsafe fn new(protected: bool, explicit: bool) -> Result<Self, InitError> {
+use std::convert::TryFrom;
 if explicit {
 if std::env::var_os("LISTEN_PID").is_none() && std::env::var_os("LISTEN_FDS").is_none() && std::env::var_os("LISTEN_FDNAMES").is_none() {
 // Systemd is not used - make the map empty
 return Ok(SystemdSockets(Mutex::new(Default::default())));
 }
 }
 if protected { Self::check_single_thread()? }
 // MUST BE true FOR SAFETY!!!
-let map = libsystemd::activation::receive_descriptors_with_names(/*unset env = */ protected)?.into_iter().map(|(fd, name)| (name, fd)).collect();
+let map = libsystemd::activation::receive_descriptors_with_names(/*unset env = */ protected)?.into_iter().map(|(fd, name)| {
+(name, Socket::try_from(fd))
+}).collect();
 Ok(SystemdSockets(Mutex::new(map)))
 }
 fn check_single_thread() -> Result<(), InitError> {
 use std::io::BufRead;
 line.clear();
 }
 Ok(())
 }
-fn take(&self, name: &str) -> Option<FileDescriptor> {
+fn take(&self, name: &str) -> Option<StoredSocket> {
 // MUST remove THE SOCKET FOR SAFETY!!!
 self.0.lock().expect("poisoned mutex").remove(name)
 }
 }
 }
 }
 #[cfg(all(target_os = "linux", feature = "enable_systemd"))]
 fn get_systemd(socket_name: String, prefixed: bool) -> Result<(std::net::TcpListener, SocketAddrInner), BindError> {
-use libsystemd::activation::IsType;
+use systemd_sockets::Socket;
-use std::os::unix::io::{FromRawFd, IntoRawFd};
 let real_systemd_name = if prefixed {
 &socket_name[SYSTEMD_PREFIX.len()..]
 } else {
 &socket_name
 };
 let socket = systemd_sockets::take(real_systemd_name).map_err(BindErrorInner::ReceiveDescriptors)?;
-// Safety: The environment variable is unset, so that no other calls can get the
+// match instead of combinators to avoid cloning socket_name
-// descriptors. The descriptors are taken from the map, not cloned, so they can't
+match socket {
-// be duplicated.
+Some(Ok(Socket::TcpListener(socket))) => Ok((socket, SocketAddrInner::Systemd(socket_name))),
-unsafe {
+Some(_) => Err(BindErrorInner::NotInetSocket(socket_name).into()),
-// match instead of combinators to avoid cloning socket_name
+None => Err(BindErrorInner::MissingDescriptor(socket_name).into())
-match socket {
-Some(socket) if socket.is_inet() => Ok((std::net::TcpListener::from_raw_fd(socket.into_raw_fd()), SocketAddrInner::Systemd(socket_name))),
-Some(_) => Err(BindErrorInner::NotInetSocket(socket_name).into()),
-None => Err(BindErrorInner::MissingDescriptor(socket_name).into())
-}
 }
 }
 // This approach makes the rest of the code much simpler as it doesn't require sprinkling it
 // with #[cfg(all(target_os = "linux", feature = "enable_systemd"))] yet still statically guarantees it won't execute.
 /// systemd socket but at that time there may be other threads running and error reporting also
 /// faces some restrictions. This function provides better control over the initialization point
 /// and returns a more idiomatic error type.
 ///
 /// You should generally call this at around the top of `main`, where no threads were created yet.
+/// While technically, you may spawn a thread and call this function after that thread terminated,
+/// this has the additional problem that the descriptors are still around, so if that thread (or the
+/// current one!) forks and execs the descriptors will leak into the child.
 #[inline]
 pub fn init() -> Result<(), error::InitError> {
 #[cfg(all(target_os = "linux", feature = "enable_systemd"))]
 {
 // Calling with true is always sound
 ///
 /// If for any reason you're unable to call `init` in a single thread at around the top of `main`
 /// (and this should be almost never) you may call this method if you've ensured that no other part
 /// of your codebase is operating on systemd-provided file descriptors stored in the environment
 /// variables.
+///
+/// Note however that doing so uncovers another problem: if another thread forks and execs the
+/// systemd file descriptors will get passed into that program! In such case you somehow need to
+/// clean up the file descriptors yourself.
 pub unsafe fn init_unprotected() -> Result<(), error::InitError> {
 #[cfg(all(target_os = "linux", feature = "enable_systemd"))]
 {
 systemd_sockets::init(false).map_err(error::InitError)
 }

Mercurial > crates > systemd-socket

comparison src/lib.rs @ 25:8e20daee41ed