[lxc-devel] [lxd/master] Storage: Don't reset custom volume root permissions on mount for DIR and BTRFS

tomponline on Github lxc-bot at linuxcontainers.org
Wed May 6 08:36:32 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 600 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200506/297be543/attachment-0001.bin>
-------------- next part --------------
From 29dc3614a3367f5e229f43f09476dd32ba6035f4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 6 May 2020 09:18:32 +0100
Subject: [PATCH 1/2] test/suites/storage/volume/attach: Adds test for custom
 volume root perm persistence

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/storage_volume_attach.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/suites/storage_volume_attach.sh b/test/suites/storage_volume_attach.sh
index 80b732a54a..bdbc2f6d28 100644
--- a/test/suites/storage_volume_attach.sh
+++ b/test/suites/storage_volume_attach.sh
@@ -90,6 +90,13 @@ test_storage_volume_attach() {
   # attach second container
   lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")" testvolume c2 testvolume
 
+  # check that setting perms on the root of the custom volume persists after a reboot.
+  lxc exec c2 -- stat -c '%a' /testvolume | grep 711
+  lxc exec c2 -- chmod 0700 /testvolume
+  lxc exec c2 -- stat -c '%a' /testvolume | grep 700
+  lxc restart --force c2
+  lxc exec c2 -- stat -c '%a' /testvolume | grep 700
+
   # delete containers
   lxc delete -f c1
   lxc delete -f c2

From e913337a2196797090fa4e4e1a9ca757c4d3b5de Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 6 May 2020 09:26:32 +0100
Subject: [PATCH 2/2] lxd/storage/drivers: Fixes custom volume root mount perm
 issue for BTRFS and DIR

The mount-time EnsureMountPath() call was resetting the permission of the custom volume root path to 0711.

However for BTRFS and DIR drivers, because there is no actual mount occurring, this was resetting any permission set inside the instance.

For these drivers we now do not call EnsureMountPath if the path exists and the volumne type is custom.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_btrfs_volumes.go | 28 ++++++++++++++-------
 lxd/storage/drivers/driver_dir_volumes.go   | 25 ++++++++++++------
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 9048a01f4c..275fc5d97d 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -571,14 +571,19 @@ func (d *btrfs) GetVolumeDiskPath(vol Volume) (string, error) {
 	return genericVFSGetVolumeDiskPath(vol)
 }
 
-// MountVolume simulates mounting a volume.
+// MountVolume simulates mounting a volume. As the driver doesn't have volumes to mount it returns
+// false indicating that there is no need to issue an unmount.
 func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
-	err := vol.EnsureMountPath()
-	if err != nil {
-		return false, err
+	// Don't attempt to modify the permission of an existing custom volume root.
+	// A user inside the instance may have modified this and we don't want to reset it on restart.
+	if !shared.PathExists(vol.MountPath()) || vol.volType != VolumeTypeCustom {
+		err := vol.EnsureMountPath()
+		if err != nil {
+			return false, err
+		}
 	}
 
-	return true, nil
+	return false, nil
 }
 
 // UnmountVolume simulates unmounting a volume.
@@ -871,12 +876,17 @@ func (d *btrfs) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) e
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to avoid accidental modifications.
 func (d *btrfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
-	err := snapVol.EnsureMountPath()
-	if err != nil {
-		return false, err
+	snapPath := snapVol.MountPath()
+
+	// Don't attempt to modify the permission of an existing custom volume root.
+	// A user inside the instance may have modified this and we don't want to reset it on restart.
+	if !shared.PathExists(snapPath) || snapVol.volType != VolumeTypeCustom {
+		err := snapVol.EnsureMountPath()
+		if err != nil {
+			return false, err
+		}
 	}
 
-	snapPath := snapVol.MountPath()
 	return mountReadOnly(snapPath, snapPath)
 }
 
diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index 61a8516080..5055dd7ea4 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -301,12 +301,16 @@ func (d *dir) GetVolumeDiskPath(vol Volume) (string, error) {
 	return genericVFSGetVolumeDiskPath(vol)
 }
 
-// MountVolume simulates mounting a volume. As dir driver doesn't have volumes to mount it returns
+// MountVolume simulates mounting a volume. As the driver doesn't have volumes to mount it returns
 // false indicating that there is no need to issue an unmount.
 func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
-	err := vol.EnsureMountPath()
-	if err != nil {
-		return false, err
+	// Don't attempt to modify the permission of an existing custom volume root.
+	// A user inside the instance may have modified this and we don't want to reset it on restart.
+	if !shared.PathExists(vol.MountPath()) || vol.volType != VolumeTypeCustom {
+		err := vol.EnsureMountPath()
+		if err != nil {
+			return false, err
+		}
 	}
 
 	return false, nil
@@ -395,12 +399,17 @@ func (d *dir) DeleteVolumeSnapshot(snapVol Volume, op *operations.Operation) err
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to avoid accidental modifications.
 func (d *dir) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
-	err := snapVol.EnsureMountPath()
-	if err != nil {
-		return false, err
+	snapPath := snapVol.MountPath()
+
+	// Don't attempt to modify the permission of an existing custom volume root.
+	// A user inside the instance may have modified this and we don't want to reset it on restart.
+	if !shared.PathExists(snapPath) || snapVol.volType != VolumeTypeCustom {
+		err := snapVol.EnsureMountPath()
+		if err != nil {
+			return false, err
+		}
 	}
 
-	snapPath := snapVol.MountPath()
 	return mountReadOnly(snapPath, snapPath)
 }
 


More information about the lxc-devel mailing list