[pbs-devel] [PATCH proxmox-backup v8 4/4] fix: api: avoid race condition in set_backup_owner

[Shannon Sterz]

when two clients change the owner of a backup store, a race condition
arose. add locking to avoid this.

Signed-off-by: Shannon Sterz <s.sterz at proxmox.com>
---
 src/api2/admin/datastore.rs | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 483e595c..3e532636 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -7,7 +7,7 @@ use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, format_err, Context, Error};
 use futures::*;
 use hyper::http::request::Parts;
 use hyper::{header, Body, Response, StatusCode};
@@ -2347,10 +2347,9 @@ pub async fn set_backup_owner(
         let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
 
         let backup_group = datastore.backup_group(ns, backup_group);
+        let owner = backup_group.get_owner()?;
 
         if owner_check_required {
-            let owner = backup_group.get_owner()?;
-
             let allowed = match (owner.is_token(), new_owner.is_token()) {
                 (true, true) => {
                     // API token to API token, owned by same user
@@ -2397,6 +2396,14 @@ pub async fn set_backup_owner(
             );
         }
 
+        let _guard = backup_group
+            .lock()
+            .with_context(|| format!("while setting the owner of group '{backup_group:?}'"))?;
+
+        if owner != backup_group.get_owner()? {
+            bail!("{owner} does not own this group anymore");
+        }
+
         backup_group.set_owner(&new_owner, true)?;
 
         Ok(())
-- 
2.39.5