sys: open directories with O_CLOEXEC

Factor out the open-flags to use for directories and add the CLOEXEC
flag to ensure that open FDs do not get passed to any child process.

A prominent cases where this can happen is the proxmox-daemon reload
code, which re-execs itself in a forked child-process and thus gets
all FDs passed.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
 [ TL: expand doc-comment and reword commit message to point at actual
   thing this fixes (exec not daemon reload) ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Dominik Csapak 2024-11-29 15:28:01 +01:00 committed by Thomas Lamprecht
parent 0547659e2c
commit 787c2e03d7
2 changed files with 12 additions and 7 deletions

View File

@ -24,7 +24,7 @@ pub fn change_cloexec(fd: RawFd, on: bool) -> Result<(), anyhow::Error> {
} }
pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> { pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> {
open(".", OFlag::O_DIRECTORY, stat::Mode::empty()) open(".", crate::fs::DIR_FLAGS, stat::Mode::empty())
} }
pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error> pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error>

View File

@ -14,6 +14,11 @@ use proxmox_lang::try_block;
use crate::fs::{fchown, CreateOptions}; use crate::fs::{fchown, CreateOptions};
/// The default list of [`OFlag`]'s we want to use when opening directories. Besides ensuring that
/// the FD indeed points to a directory we also must ensure that it gets closed on exec to avoid
/// leaking a open FD to a child process.
pub(crate) const DIR_FLAGS: OFlag = OFlag::O_DIRECTORY.union(OFlag::O_CLOEXEC);
/// Creates directory at the provided path with specified ownership. /// Creates directory at the provided path with specified ownership.
/// ///
/// Errors if the directory already exists. /// Errors if the directory already exists.
@ -66,7 +71,7 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
Err(err) => bail!("unable to create directory {path:?} - {err}",), Err(err) => bail!("unable to create directory {path:?} - {err}",),
} }
let fd = nix::fcntl::open(path, OFlag::O_DIRECTORY, stat::Mode::empty()) let fd = nix::fcntl::open(path, DIR_FLAGS, stat::Mode::empty())
.map(|fd| unsafe { OwnedFd::from_raw_fd(fd) }) .map(|fd| unsafe { OwnedFd::from_raw_fd(fd) })
.map_err(|err| format_err!("unable to open created directory {path:?} - {err}"))?; .map_err(|err| format_err!("unable to open created directory {path:?} - {err}"))?;
// umask defaults to 022 so make sure the mode is fully honowed: // umask defaults to 022 so make sure the mode is fully honowed:
@ -120,7 +125,7 @@ fn create_path_do(
Some(Component::Prefix(_)) => bail!("illegal prefix path component encountered"), Some(Component::Prefix(_)) => bail!("illegal prefix path component encountered"),
Some(Component::RootDir) => { Some(Component::RootDir) => {
let _ = iter.next(); let _ = iter.next();
crate::fd::open(c"/", OFlag::O_DIRECTORY, stat::Mode::empty())? crate::fd::open(c"/", DIR_FLAGS, stat::Mode::empty())?
} }
Some(Component::CurDir) => { Some(Component::CurDir) => {
let _ = iter.next(); let _ = iter.next();
@ -128,7 +133,7 @@ fn create_path_do(
} }
Some(Component::ParentDir) => { Some(Component::ParentDir) => {
let _ = iter.next(); let _ = iter.next();
crate::fd::open(c"..", OFlag::O_DIRECTORY, stat::Mode::empty())? crate::fd::open(c"..", DIR_FLAGS, stat::Mode::empty())?
} }
Some(Component::Normal(_)) => { Some(Component::Normal(_)) => {
// simply do not advance the iterator, heavy lifting happens in create_path_at_do() // simply do not advance the iterator, heavy lifting happens in create_path_at_do()
@ -154,7 +159,7 @@ fn create_path_at_do(
None => return Ok(created), None => return Ok(created),
Some(Component::ParentDir) => { Some(Component::ParentDir) => {
at = crate::fd::openat(&at, c"..", OFlag::O_DIRECTORY, stat::Mode::empty())?; at = crate::fd::openat(&at, c"..", DIR_FLAGS, stat::Mode::empty())?;
} }
Some(Component::Normal(path)) => { Some(Component::Normal(path)) => {
@ -175,7 +180,7 @@ fn create_path_at_do(
Err(e) => return Err(e.into()), Err(e) => return Err(e.into()),
Ok(_) => true, Ok(_) => true,
}; };
at = crate::fd::openat(&at, path, OFlag::O_DIRECTORY, stat::Mode::empty())?; at = crate::fd::openat(&at, path, DIR_FLAGS, stat::Mode::empty())?;
if let (true, Some(opts)) = (created, opts) { if let (true, Some(opts)) = (created, opts) {
if opts.owner.is_some() || opts.group.is_some() { if opts.owner.is_some() || opts.group.is_some() {
@ -222,7 +227,7 @@ pub fn make_tmp_dir<P: AsRef<Path>>(
if let Some(options) = options { if let Some(options) = options {
if let Err(err) = try_block!({ if let Err(err) = try_block!({
let mut fd = crate::fd::open(&path, OFlag::O_DIRECTORY, stat::Mode::empty())?; let mut fd = crate::fd::open(&path, DIR_FLAGS, stat::Mode::empty())?;
options.apply_to(&mut fd, &path)?; options.apply_to(&mut fd, &path)?;
Ok::<(), Error>(()) Ok::<(), Error>(())
}) { }) {