volumes: switch order of checks

avoid any I/O operation on the volume if the source directory is empty.

This is useful on network file systems (since CAP_DAC_OVERRIDE is not
honored) where the root user might not have enough privileges to
perform an I/O operation on the NFS mount but the user running inside
the container has.

[NO NEW TESTS NEEDED] it needs a setup with a network file system

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

1 files changed, 13 insertions, 13 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index ae61298f3..fe2a19a4e 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1642,19 +1642,6 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string)
 	if vol.state.NeedsCopyUp {
 		logrus.Debugf("Copying up contents from container %s to volume %s", c.ID(), vol.Name())
 
-		// If the volume is not empty, we should not copy up.
-		volMount := vol.mountPoint()
-		contents, err := ioutil.ReadDir(volMount)
-		if err != nil {
-			return nil, errors.Wrapf(err, "error listing contents of volume %s mountpoint when copying up from container %s", vol.Name(), c.ID())
-		}
-		if len(contents) > 0 {
-			// The volume is not empty. It was likely modified
-			// outside of Podman. For safety, let's not copy up into
-			// it. Fixes CVE-2020-1726.
-			return vol, nil
-		}
-
 		srcDir, err := securejoin.SecureJoin(mountpoint, v.Dest)
 		if err != nil {
 			return nil, errors.Wrapf(err, "error calculating destination path to copy up container %s volume %s", c.ID(), vol.Name())
@@ -1688,6 +1675,19 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string)
 			return vol, nil
 		}
 
+		// If the volume is not empty, we should not copy up.
+		volMount := vol.mountPoint()
+		contents, err := ioutil.ReadDir(volMount)
+		if err != nil {
+			return nil, errors.Wrapf(err, "error listing contents of volume %s mountpoint when copying up from container %s", vol.Name(), c.ID())
+		}
+		if len(contents) > 0 {
+			// The volume is not empty. It was likely modified
+			// outside of Podman. For safety, let's not copy up into
+			// it. Fixes CVE-2020-1726.
+			return vol, nil
+		}
+
 		// Set NeedsCopyUp to false since we are about to do first copy
 		// Do not copy second time.
 		vol.state.NeedsCopyUp = false