Skip to content

Commit

Permalink
bring back close() error checking
Browse files Browse the repository at this point in the history
Summary:
In stack ending at D66275420, several call sites were migrated from
RawFd with explicit `libc::close()` or `nix::unistd::close()` which
admit the possibility of failure. Now, `close()` failures are
unactionable -- you certainly cannot retry the close operation or risk
closing some other FD -- but it may be worth logging when they happen.

Bring back explicit close operations and error checking.

Test Plan: CI and eyes

Reviewed By: jasonwhite

Differential Revision: D67157487

fbshipit-source-id: 254cace779c6f993117fbaaf6e0453df6bdc70e5
  • Loading branch information
chadaustin authored and facebook-github-bot committed Dec 18, 2024
1 parent ffeb632 commit 5fa9a27
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ rust_binary(
"anyhow",
"cap-std",
"clap",
"close-err",
"libc",
"nix",
"rustix",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::os::fd::AsRawFd;

use anyhow::Context;
use anyhow::Result;
use close_err::Closable;
use libc::c_char;
use libc::c_short;
use libc::ifreq;
Expand All @@ -35,5 +36,6 @@ pub(crate) fn bring_loopback_up() -> Result<()> {
unsafe { req.ifr_ifru.ifru_flags |= libc::IFF_UP as c_short };
Errno::result(unsafe { libc::ioctl(socket.as_raw_fd(), libc::SIOCSIFFLAGS, &mut req) })
.context("while setting up flag")?;
socket.close().context("while closing socket")?;
Ok(())
}
2 changes: 1 addition & 1 deletion antlir/antlir2/antlir2_rootless/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub enum Error {
#[error("Rootless was somehow already initialized")]
AlreadyInitialized,
#[error("failed to setup userns in current process: {0}")]
Userns(nix::errno::Errno),
Userns(std::io::Error),
#[error("error reading a subid file: {0}")]
SubidRead(std::io::Error),
#[error("error using subid mapping: {0}")]
Expand Down
1 change: 1 addition & 0 deletions antlir/antlir2/antlir2_rootless/unshare_userns/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ rust_library(
],
visibility = ["//antlir/antlir2/antlir2_rootless:"],
deps = [
"close-err",
"nix",
],
)
15 changes: 8 additions & 7 deletions antlir/antlir2/antlir2_rootless/unshare_userns/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@
#![no_std]

use core::ffi::CStr;
use std::io;
use std::os::fd::AsRawFd;

use close_err::Closable;
use nix::errno::Errno;
use nix::sched::unshare;
use nix::sched::CloneFlags;
Expand All @@ -69,7 +71,6 @@ use nix::sys::wait::WaitStatus;
use nix::unistd::fork;
use nix::unistd::pipe;
use nix::unistd::ForkResult;
use nix::Result;

#[derive(Copy, Clone)]
pub struct Map<'a> {
Expand All @@ -78,7 +79,7 @@ pub struct Map<'a> {
pub len: &'a CStr,
}

pub fn unshare_userns(pid_cstr: &CStr, uid_map: &Map, gid_map: &Map) -> Result<()> {
pub fn unshare_userns(pid_cstr: &CStr, uid_map: &Map, gid_map: &Map) -> io::Result<()> {
// TODO(T181212521): do the same check in OSS
#[cfg(facebook)]
if memory::is_using_jemalloc()
Expand All @@ -93,22 +94,22 @@ pub fn unshare_userns(pid_cstr: &CStr, uid_map: &Map, gid_map: &Map) -> Result<(
match unsafe { fork() }? {
ForkResult::Parent { child } => {
unshare(CloneFlags::CLONE_NEWUSER)?;
drop(read);
drop(write);
read.close()?;
write.close()?;
let status = waitpid(child, None)?;
if status != WaitStatus::Exited(child, 0) {
return Err(Errno::EIO);
return Err(io::Error::from(Errno::EIO));
}
}
ForkResult::Child => {
drop(write);
write.close()?;
nix::unistd::read(read.as_raw_fd(), &mut [0u8])?;

match unsafe { fork() } {
Ok(ForkResult::Parent { child }) => {
let status = waitpid(child, None)?;
if status != WaitStatus::Exited(child, 0) {
return Err(Errno::EIO);
return Err(io::Error::from(Errno::EIO));
}
Ok(())
}
Expand Down

0 comments on commit 5fa9a27

Please sign in to comment.