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>
This commit is contained in:
Giuseppe Scrivano 2022-06-25 14:19:04 +02:00
parent 28e2a604b4
commit 443a2afdb5
No known key found for this signature in database
GPG Key ID: 67E38F7A8BA21772
1 changed files with 13 additions and 13 deletions

View File

@ -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