From 04e50855b39c7ce9548eb0a34b40d11ad8f91644 Mon Sep 17 00:00:00 2001 From: Shannon Sterz Date: Wed, 26 Mar 2025 12:44:14 +0100 Subject: [PATCH] fix: api: avoid race condition in set_backup_owner when two clients change the owner of a backup store, a race condition arose. add locking to avoid this. Signed-off-by: Shannon Sterz Acked-by: Wolfgang Bumiller --- 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(())