From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 4B1CC1FF171 for ; Fri, 29 Nov 2024 15:28:01 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4C6A31AF3E; Fri, 29 Nov 2024 15:28:04 +0100 (CET) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Fri, 29 Nov 2024 15:28:00 +0100 Message-Id: <20241129142801.3334969-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [file.rs] Subject: [pbs-devel] [PATCH proxmox v2 1/2] sys: fs: set CLOEXEC when creating temp files X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 --- changes from v1: * introduce mkostemp helper which is similar to nix's mkstemp helper (the code is a copy of mkstemp aside from the call to libcmkostemp + the oflag handling) I did it this way, since we may be able to upstream this, have to look more closer at this though. 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), }; -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel