From 0547659e2c141e35642bd2df5e77373eeb32f83c Mon Sep 17 00:00:00 2001 From: Dominik Csapak Date: Fri, 29 Nov 2024 15:28:00 +0100 Subject: [PATCH] sys: fs: set CLOEXEC when creating temp files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In general we want all open files to have set CLOEXEC since our reloading mechanism can basically fork at any moment and we don't want newer daemons to carry around old file descriptors, especially lock files. Since `make_tmp_file` is called by many things (e.g. open_file_locked, logrotate, rrd), set O_CLOEXEC with mkostemp. This fixes issues with leftover file descriptors e.g. tape backups not working because of lingering locks after a reload, or having deleted rrd files open. Signed-off-by: Dominik Csapak Reviewed-by: Fabian Grünbichler --- proxmox-sys/src/fs/file.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs index fbfc0b58..74b9e74e 100644 --- a/proxmox-sys/src/fs/file.rs +++ b/proxmox-sys/src/fs/file.rs @@ -116,6 +116,29 @@ pub fn file_read_firstline>(path: P) -> Result { read_firstline(path).map_err(|err| format_err!("unable to read {path:?} - {err}")) } +#[inline] +/// Creates a tmpfile like [`nix::unistd::mkstemp`], but with [`nix::fctnl::Oflag`] set. +/// +/// Note that some flags are masked out since they can produce an error, see mkostemp(2) for details. +// code is mostly copied from nix mkstemp +fn mkostemp( + template: &P, + oflag: OFlag, +) -> nix::Result<(std::os::fd::RawFd, PathBuf)> { + use std::os::unix::ffi::OsStringExt; + let mut path = template.with_nix_path(|path| path.to_bytes_with_nul().to_owned())?; + let p = path.as_mut_ptr().cast(); + + let flags = OFlag::intersection(OFlag::O_APPEND | OFlag::O_CLOEXEC | OFlag::O_SYNC, oflag); + + let fd = unsafe { libc::mkostemp(p, flags.bits()) }; + let last = path.pop(); // drop the trailing nul + debug_assert!(last == Some(b'\0')); + let pathname = std::ffi::OsString::from_vec(path); + Errno::result(fd)?; + Ok((fd, PathBuf::from(pathname))) +} + /// Takes a Path and CreateOptions, creates a tmpfile from it and returns /// a RawFd and PathBuf for it pub fn make_tmp_file>( @@ -127,7 +150,7 @@ pub fn make_tmp_file>( // use mkstemp here, because it works with different processes, threads, even tokio tasks let mut template = path.to_owned(); template.set_extension("tmp_XXXXXX"); - let (mut file, tmp_path) = match unistd::mkstemp(&template) { + let (mut file, tmp_path) = match mkostemp(&template, OFlag::O_CLOEXEC) { Ok((fd, path)) => (unsafe { File::from_raw_fd(fd) }, path), Err(err) => bail!("mkstemp {:?} failed: {}", template, err), };