[pbs-devel] [PATCH proxmox-backup] change tape drive lock path

[pbs-devel] [PATCH proxmox-backup] change tape drive lock path

[Dietmar Maurer]

New kernel has stricter checks on tmpfs with stick-bit on directories, so some
commands (i.e. proxmox-tape changer status) fails when executed as root, because
permission checks fails when locking the drive.

This patch move the drive locks to /run/proxmox-backup/drive-lock.

Note: This is incompatible to old locking mechmanism, so users may not
run tape backups during update (or running backup can fail).
---

Lock file permissions are still wrong if the user runs "proxmox-tape
changer status" as root and the lock file does not exist already.

 src/bin/proxmox-backup-api.rs |  1 +
 src/tape/drive/mod.rs         |  6 +++---
 src/tape/mod.rs               | 18 ++++++++++++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index 25ed030a..a8fbbadb 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -45,6 +45,7 @@ async fn run() -> Result<(), Error> {
     proxmox_backup::tape::create_tape_status_dir()?;
     proxmox_backup::tape::create_drive_state_dir()?;
     proxmox_backup::tape::create_changer_state_dir()?;
+    proxmox_backup::tape::create_drive_lock_dir()?;
 
     if let Err(err) = generate_auth_key() {
         bail!("unable to generate auth key - {}", err);
diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index 8010d576..fb4b6f47 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -568,7 +568,7 @@ pub fn get_tape_device_state(
     config: &SectionConfigData,
     drive: &str,
 ) -> Result<Option<String>, Error> {
-    let path = format!("/run/proxmox-backup/drive-state/{}", drive);
+    let path = format!("{}/{}", crate::tape::DRIVE_STATE_DIR, drive);
     let state = file_read_optional_string(path)?;
 
     let device_path = tape_device_path(config, drive)?;
@@ -612,7 +612,7 @@ fn lock_device_path(device_path: &str) -> Result<DeviceLockGuard, TapeLockError>
 
     let lock_name = crate::tools::systemd::escape_unit(device_path, true);
 
-    let mut path = std::path::PathBuf::from("/var/lock");
+    let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
     path.push(lock_name);
 
     let timeout = std::time::Duration::new(10, 0);
@@ -637,7 +637,7 @@ fn test_device_path_lock(device_path: &str) -> Result<bool, Error> {
 
     let lock_name = crate::tools::systemd::escape_unit(device_path, true);
 
-    let mut path = std::path::PathBuf::from("/var/lock");
+    let mut path = std::path::PathBuf::from(crate::tape::DRIVE_LOCK_DIR);
     path.push(lock_name);
 
     let timeout = std::time::Duration::new(0, 0);
diff --git a/src/tape/mod.rs b/src/tape/mod.rs
index 5248d21b..8190e141 100644
--- a/src/tape/mod.rs
+++ b/src/tape/mod.rs
@@ -48,6 +48,9 @@ pub use pool_writer::*;
 /// Directory path where we store all tape status information
 pub const TAPE_STATUS_DIR: &str = "/var/lib/proxmox-backup/tape";
 
+/// Directory path where we store drive lock file
+pub const DRIVE_LOCK_DIR: &str = concat!(PROXMOX_BACKUP_RUN_DIR_M!(), "/drive-lock");
+
 /// Directory path where we store temporary drive state
 pub const DRIVE_STATE_DIR: &str = concat!(PROXMOX_BACKUP_RUN_DIR_M!(), "/drive-state");
 
@@ -78,6 +81,21 @@ pub fn create_tape_status_dir() -> Result<(), Error> {
     Ok(())
 }
 
+/// Create drive lock dir with correct permission
+pub fn create_drive_lock_dir() -> Result<(), Error> {
+    let backup_user = crate::backup::backup_user()?;
+    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0750);
+    let options = CreateOptions::new()
+        .perm(mode)
+        .owner(backup_user.uid)
+        .group(backup_user.gid);
+
+    create_path(DRIVE_LOCK_DIR, None, Some(options))
+        .map_err(|err: Error| format_err!("unable to create drive state dir - {}", err))?;
+
+    Ok(())
+}
+
 /// Create drive state dir with correct permission
 pub fn create_drive_state_dir() -> Result<(), Error> {
     let backup_user = crate::backup::backup_user()?;
-- 
2.30.2

[pbs-devel] applied: [PATCH proxmox-backup] change tape drive lock path

[Thomas Lamprecht]

On 12.07.21 17:23, Dietmar Maurer wrote:
> New kernel has stricter checks on tmpfs with stick-bit on directories, so some
> commands (i.e. proxmox-tape changer status) fails when executed as root, because
> permission checks fails when locking the drive.
> 
> This patch move the drive locks to /run/proxmox-backup/drive-lock.
> 
> Note: This is incompatible to old locking mechmanism, so users may not
> run tape backups during update (or running backup can fail).
> ---
> 
> Lock file permissions are still wrong if the user runs "proxmox-tape
> changer status" as root and the lock file does not exist already.
> 
>  src/bin/proxmox-backup-api.rs |  1 +
>  src/tape/drive/mod.rs         |  6 +++---
>  src/tape/mod.rs               | 18 ++++++++++++++++++
>  3 files changed, 22 insertions(+), 3 deletions(-)
> 
>

applied, thanks!