[pbs-devel] [PATCH proxmox-backup v3 10/10] fix CachedUserInfo by using a shared memory version counter

[Dietmar Maurer <dietmar@proxmox.com>]

---
 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<Arc<CachedUserInfo>>,
     last_update: i64,
+    last_user_cache_generation: usize,
 }
 
 lazy_static! {
     static ref CACHED_CONFIG: RwLock<ConfigCache> = 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<Arc<Self>, 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<u8>,
+}
+
+#[repr(C)]
+struct Head {
+    // User (user.cfg) cache generation/version.
+    user_cache_generation: AtomicUsize,
+}
+
+static INSTANCE: OnceCell<Arc<Memcom>>  = OnceCell::new();
+
+const MEMCOM_FILE_PATH: &str = rundir!("/proxmox-backup-memcom");
+
+impl Memcom {
+    
+    /// Open the memory based communication channel singleton.
+    pub fn new() ->  Result<Arc<Self>, Error>  {
+        INSTANCE.get_or_try_init(Self::open).map(Arc::clone)
+    }
+
+    // Actual work of `new`:
+    fn open() ->  Result<Arc<Self>, 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::<u8>::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, nix::Error>  {
+      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<Fd, Error>  {
+    // 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