giuseppe
commented
2 years ago does it work for you if we exchange the order of the checks?
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index ce48987f6..c11164ce3 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1666,19 +1666,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())
@@ -1712,6 +1699,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
Ramblurr
mheon
rhatdan
Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)
/kind bug
Description
Using nfs named volumes has broken recently in some cases. I think this is related to
The error is:
My current theory is that the copy up routine is being performed as a user that doesn't have permission to write to the nfs share. In my case the only allowed user is
2000so even uid0on the podman host won't have permission.The funny thing is that I'm not even convinced the pre-condition for NeedsCopyUp is met, because the
/var/www/html/datadirectory in the container image is empty to start with.I have manually mounted the share using the same mount options, then
sued into user w/ uid2000and verified the mount works fine.Using
docker-ce(on another fresh host), I've tested it with docker and it works as expected.Steps to reproduce the issue:
Create a named podman volume using nfs. The exposed share should be owned by a specific uid. In my case it is
2000Create a container using the mount:
Describe the results you received:
Describe the results you expected:
I expect the container to start and work with the mount.
This works fine with docker-ce (on centos 9 stream)
Additional information you deem important (e.g. issue happens only occasionally):
podman volume inspect my-nfs-volOutput of
podman version:Output of
podman info --debug:Package info (e.g. output of
rpm -q podmanorapt list podman):Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/main/troubleshooting.md)
Yes
Additional environment details (AWS, VirtualBox, physical, etc.): kvm/qemu virtual machine running centos 9 stream