From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D1DC770BBB for ; Fri, 25 Jun 2021 11:21:04 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AF5A7161D1 for ; Fri, 25 Jun 2021 11:21:04 +0200 (CEST) Received: from dev7.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6A9C21607D for ; Fri, 25 Jun 2021 11:20:58 +0200 (CEST) Received: by dev7.proxmox.com (Postfix, from userid 0) id 1481080F60; Fri, 25 Jun 2021 11:20:52 +0200 (CEST) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Fri, 25 Jun 2021 11:20:50 +0200 Message-Id: <20210625092050.2329182-11-dietmar@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210625092050.2329182-1-dietmar@proxmox.com> References: <20210625092050.2329182-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.592 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS 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. [memcom.rs, openid.rs, cache.data, user.rs, tools.rs] Subject: [pbs-devel] [PATCH proxmox-backup v3 10/10] fix CachedUserInfo by using a shared memory version counter 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: , X-List-Received-Date: Fri, 25 Jun 2021 09:21:04 -0000 --- src/api2/access/openid.rs | 2 - src/config/cached_user_info.rs | 13 ++- src/config/user.rs | 6 ++ src/tools.rs | 3 + src/tools/memcom.rs | 159 +++++++++++++++++++++++++++++++++ 5 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 src/tools/memcom.rs diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs index 52ec4311..27e3c08c 100644 --- a/src/api2/access/openid.rs +++ b/src/api2/access/openid.rs @@ -120,8 +120,6 @@ pub fn openid_login( } config.set_data(user.userid.as_str(), "user", &user)?; user::save_config(&config)?; - // fixme: replace sleep with shared memory change notification - std::thread::sleep(std::time::Duration::new(6, 0)); } else { bail!("user account '{}' missing, disabled or expired.", user_id); } diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs index 6cb64162..a4c4604f 100644 --- a/src/config/cached_user_info.rs +++ b/src/config/cached_user_info.rs @@ -12,6 +12,7 @@ use proxmox::tools::time::epoch_i64; use super::acl::{AclTree, ROLE_NAMES, ROLE_ADMIN}; use super::user::{ApiToken, User}; use crate::api2::types::{Authid, Userid}; +use crate::tools::Memcom; /// Cache User/Group/Token/Acl configuration data for fast permission tests pub struct CachedUserInfo { @@ -22,11 +23,12 @@ pub struct CachedUserInfo { struct ConfigCache { data: Option>, last_update: i64, + last_user_cache_generation: usize, } lazy_static! { static ref CACHED_CONFIG: RwLock = RwLock::new( - ConfigCache { data: None, last_update: 0 } + ConfigCache { data: None, last_update: 0, last_user_cache_generation: 0 } ); } @@ -35,9 +37,15 @@ impl CachedUserInfo { /// Returns a cached instance (up to 5 seconds old). pub fn new() -> Result, Error> { let now = epoch_i64(); + + let memcom = Memcom::new()?; + let user_cache_generation = memcom.user_cache_generation(); + { // limit scope let cache = CACHED_CONFIG.read().unwrap(); - if (now - cache.last_update) < 5 { + if (user_cache_generation == cache.last_user_cache_generation) && + ((now - cache.last_update) < 5) + { if let Some(ref config) = cache.data { return Ok(config.clone()); } @@ -51,6 +59,7 @@ impl CachedUserInfo { let mut cache = CACHED_CONFIG.write().unwrap(); cache.last_update = now; + cache.last_user_cache_generation = user_cache_generation; cache.data = Some(config.clone()); Ok(config) diff --git a/src/config/user.rs b/src/config/user.rs index 28e81876..bdec5fc1 100644 --- a/src/config/user.rs +++ b/src/config/user.rs @@ -18,6 +18,7 @@ use proxmox::api::{ use proxmox::tools::{fs::replace_file, fs::CreateOptions}; use crate::api2::types::*; +use crate::tools::Memcom; lazy_static! { pub static ref CONFIG: SectionConfig = init(); @@ -270,6 +271,11 @@ pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { replace_file(USER_CFG_FILENAME, raw.as_bytes(), options)?; + // increase user cache generation + // We use this in CachedUserInfo + let memcom = Memcom::new()?; + memcom.increase_user_cache_generation(); + Ok(()) } diff --git a/src/tools.rs b/src/tools.rs index 59599339..3b0bf087 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -38,6 +38,9 @@ pub mod format; pub mod fs; pub mod fuse_loop; +mod memcom; +pub use memcom::Memcom; + pub mod json; pub mod logrotate; pub mod loopdev; diff --git a/src/tools/memcom.rs b/src/tools/memcom.rs new file mode 100644 index 00000000..139a789a --- /dev/null +++ b/src/tools/memcom.rs @@ -0,0 +1,159 @@ +//! Memory based communication channel between proxy & daemon for things such as cache +//! invalidation. + +use std::ffi::CString; +use std::io; +use std::os::unix::io::AsRawFd; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; + +use anyhow::{bail, format_err, Error}; +use nix::errno::Errno; +use nix::fcntl::OFlag; +use nix::sys::mman::{MapFlags, ProtFlags}; +use nix::sys::stat::Mode; +use once_cell::sync::OnceCell; + +use proxmox::sys::error::SysError; +use proxmox::tools::fd::Fd; +use proxmox::tools::mmap::Mmap; + +/// In-memory communication channel. +pub struct Memcom { + mmap: Mmap, +} + +#[repr(C)] +struct Head { + // User (user.cfg) cache generation/version. + user_cache_generation: AtomicUsize, +} + +static INSTANCE: OnceCell> = OnceCell::new(); + +const MEMCOM_FILE_PATH: &str = rundir!("/proxmox-backup-memcom"); + +impl Memcom { + + /// Open the memory based communication channel singleton. + pub fn new() -> Result, Error> { + INSTANCE.get_or_try_init(Self::open).map(Arc::clone) + } + + // Actual work of `new`: + fn open() -> Result, Error> { + let fd = match open_existing() { + Ok(fd) => fd, + Err(err) if err.not_found() => create_new()?, + Err(err) => bail!("failed to open {} - {}", MEMCOM_FILE_PATH, err), + }; + + let mmap = unsafe { + Mmap::::map_fd( + fd.as_raw_fd(), + 0, + 4096, + ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, + MapFlags::MAP_SHARED | MapFlags::MAP_NORESERVE | MapFlags::MAP_POPULATE, + )? + }; + + Ok(Arc::new(Self { mmap })) + } + + // Shortcut to get the mapped `Head` as a `Head`. + fn head(&self) -> &Head { + unsafe { &*(self.mmap.as_ptr() as *const u8 as *const Head) } + } + + /// Returns the user cache generation number. + pub fn user_cache_generation(&self) -> usize { + self.head().user_cache_generation.load(Ordering::Acquire) + } + + /// Increase the user cache generation number. + pub fn increase_user_cache_generation(&self) { + self.head() + .user_cache_generation + .fetch_add(1, Ordering::AcqRel); + } +} + +/// The fast path opens an existing file. +fn open_existing() -> Result { + Fd::open(MEMCOM_FILE_PATH, OFlag::O_RDWR | OFlag::O_CLOEXEC, Mode::empty()) +} + +/// Since we need to initialize the file, we also need a solid slow path where we create the file. +/// In order to make sure the next user's `open()` vs `mmap()` race against our `truncate()` call, +/// we create it in a temporary location and rotate it in place. +fn create_new() -> Result { + // create a temporary file: + let temp_file_name = format!("{}.{}", MEMCOM_FILE_PATH, unsafe { libc::getpid() }); + let fd = Fd::open( + temp_file_name.as_str(), + OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_RDWR | OFlag::O_CLOEXEC, + Mode::from_bits_truncate(0o660), + ).map_err(|err| { + format_err!( + "failed to create new in-memory communication file at {} - {}", + temp_file_name, + err + ) + })?; + + // let it be a page in size, it'll be initialized to zero by the kernel + nix::unistd::ftruncate(fd.as_raw_fd(), 4096) + .map_err(|err| format_err!("failed to set size of {} - {}", temp_file_name, err))?; + + // if this is the pbs-daemon (running as root) rather than the proxy (running as backup user), + // make sure the backup user can access the file: + if let Ok(backup_user) = crate::backup::backup_user() { + match nix::unistd::fchown(fd.as_raw_fd(), None, Some(backup_user.gid)) { + Ok(()) => (), + Err(err) if err.is_errno(Errno::EPERM) => { + // we're not the daemon (root), so the file is already owned by the backup user + } + Err(err) => bail!( + "failed to set group to 'backup' for {} - {}", + temp_file_name, + err + ), + } + } + + // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against + // the initialization, the first one wins! + // TODO: nicer `renameat2()` wrapper in `proxmox::sys`? + let c_file_name = CString::new(temp_file_name.as_bytes()).unwrap(); + let new_path = CString::new(MEMCOM_FILE_PATH).unwrap(); + let rc = unsafe { + libc::renameat2( + -1, + c_file_name.as_ptr(), + -1, + new_path.as_ptr(), + libc::RENAME_NOREPLACE, + ) + }; + if rc == 0 { + return Ok(fd); + } + let err = io::Error::last_os_error(); + + // if another process has already raced ahead and created the file, let's just open theirs + // instead: + if err.kind() == io::ErrorKind::AlreadyExists { + // someone beat us to it: + drop(fd); + return open_existing().map_err(Error::from); + } + + // for any other errors, just bail out + bail!( + "failed to move file at {} into place at {} - {}", + temp_file_name, + MEMCOM_FILE_PATH, + err + ); +} -- 2.30.2