From 0ccb8361562a155a97d510e2a9eed095047ef267 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Fri, 14 Mar 2025 09:37:54 -0400 Subject: [PATCH 1/4] Fix a potential deadlock during `podman cp` Have one function without a `defer lock.unlock()` as one of the commands in it calls a function that also takes the same lock, so the unlock has to happen prior to function completion. Unfortunately, this is prone to errors, like the one here: I missed a case, and we could return without unlocking, causing a deadlock later in the cleanup code as we tried to take the same lock again. Refactor the command to use `defer unlock()` to simplify and avoid any further errors of this type. Introduced by e66b788a514fb8df2c8b8d3c000e0d543bbd60df - this should be included in any backports of that commit. Fixes #25585 Signed-off-by: Matt Heon --- libpod/container_copy_common.go | 7 ++----- libpod/container_internal_common.go | 4 ++++ test/e2e/cp_test.go | 22 +++++++++------------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/libpod/container_copy_common.go b/libpod/container_copy_common.go index 0553cedf8a..8d21c0f2ad 100644 --- a/libpod/container_copy_common.go +++ b/libpod/container_copy_common.go @@ -160,10 +160,10 @@ func (c *Container) copyFromArchive(path string, chown, noOverwriteDirNonDir boo // populated the volume and that will block a future // copy-up. volume.lock.Lock() + defer volume.lock.Unlock() if err := volume.update(); err != nil { logrus.Errorf("Unable to update volume %s status: %v", volume.Name(), err) - volume.lock.Unlock() return } @@ -172,15 +172,12 @@ func (c *Container) copyFromArchive(path string, chown, noOverwriteDirNonDir boo volume.state.CopiedUp = true if err := volume.save(); err != nil { logrus.Errorf("Unable to save volume %s state: %v", volume.Name(), err) - volume.lock.Unlock() return } - volume.lock.Unlock() - for _, namedVol := range c.config.NamedVolumes { if namedVol.Name == volume.Name() { - if err := c.fixVolumePermissions(namedVol); err != nil { + if err := c.fixVolumePermissionsUnlocked(namedVol, volume); err != nil { logrus.Errorf("Unable to fix volume %s permissions: %v", volume.Name(), err) } return diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 77eb280f31..ac9e41abd0 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2916,6 +2916,10 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { vol.lock.Lock() defer vol.lock.Unlock() + return c.fixVolumePermissionsUnlocked(v, vol) +} + +func (c *Container) fixVolumePermissionsUnlocked(v *ContainerNamedVolume, vol *Volume) error { // The volume may need a copy-up. Check the state. if err := vol.update(); err != nil { return err diff --git a/test/e2e/cp_test.go b/test/e2e/cp_test.go index 5c543c3e3d..028620e27a 100644 --- a/test/e2e/cp_test.go +++ b/test/e2e/cp_test.go @@ -280,22 +280,18 @@ RUN chown 9999:9999 %s`, ALPINE, ctrVolPath, ctrVolPath) defer srcFile.Close() defer os.Remove(srcFile.Name()) - volCreate := podmanTest.Podman([]string{"volume", "create", volName}) - volCreate.WaitWithDefaultTimeout() - Expect(volCreate).Should(ExitCleanly()) + _ = podmanTest.PodmanExitCleanly("volume", "create", volName) + _ = podmanTest.PodmanExitCleanly("create", "--name", ctrName, "-v", fmt.Sprintf("%s:%s", volName, ctrVolPath), imgName, "sh") - ctrCreate := podmanTest.Podman([]string{"create", "--name", ctrName, "-v", fmt.Sprintf("%s:%s", volName, ctrVolPath), imgName, "sh"}) - ctrCreate.WaitWithDefaultTimeout() - Expect(ctrCreate).To(ExitCleanly()) + _ = podmanTest.PodmanExitCleanly("cp", srcFile.Name(), fmt.Sprintf("%s:%s", ctrName, ctrVolPath)) - cp := podmanTest.Podman([]string{"cp", srcFile.Name(), fmt.Sprintf("%s:%s", ctrName, ctrVolPath)}) - cp.WaitWithDefaultTimeout() - Expect(cp).To(ExitCleanly()) - - ls := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:%s", volName, ctrVolPath), ALPINE, "ls", "-al", ctrVolPath}) - ls.WaitWithDefaultTimeout() - Expect(ls).To(ExitCleanly()) + ls := podmanTest.PodmanExitCleanly("run", "-v", fmt.Sprintf("%s:%s", volName, ctrVolPath), ALPINE, "ls", "-al", ctrVolPath) Expect(ls.OutputToString()).To(ContainSubstring("9999 9999")) Expect(ls.OutputToString()).To(ContainSubstring(filepath.Base(srcFile.Name()))) + + // Test for #25585 + _ = podmanTest.PodmanExitCleanly("rm", ctrName) + _ = podmanTest.PodmanExitCleanly("create", "--name", ctrName, "-v", fmt.Sprintf("%s:%s", volName, ctrVolPath), imgName, "sh") + _ = podmanTest.PodmanExitCleanly("cp", srcFile.Name(), fmt.Sprintf("%s:%sfile2", ctrName, ctrVolPath)) }) }) From 96ea5c9ed28e94ae8243207435e1625f2a188725 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Wed, 2 Apr 2025 09:37:31 -0400 Subject: [PATCH 2/4] Add release notes for v5.4.2 Signed-off-by: Matt Heon --- RELEASE_NOTES.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a66a154e1d..61f3071934 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,19 @@ # Release Notes +## 5.4.2 +### Bugfixes +- Fixed a bug where the `podman import` command could not import images compressed with algorithms other than gzip ([#25593](https://github.com/containers/podman/issues/25593)). +- Fixed a bug where the `podman cp` command could deadlock when copying into a non-empty volume on a container that is not running ([#25585](https://github.com/containers/podman/issues/25585)). + +### API +- Fixed a bug where the default values for some fields in the Libpod Create endpoint for Containers did not have sensible defaults for some healthcheck fields, causing unrestricted log growth for containers which did not set these fields ([#25473](https://github.com/containers/podman/issues/25473)). + +### Misc +- Updated vendored Buildah to v1.39.4 +- Updated the containers/common library to v0.62.3 +- Updated the containers/image library to v5.34.3 +- Updated the containers/storage library to v1.57.2 + ## 5.4.1 ### Bugfixes - Fixed a bug where volume quotas were not being applied ([#25368](https://github.com/containers/podman/issues/25368)). From be85287fcf4590961614ee37be65eeb315e5d9ff Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Wed, 2 Apr 2025 09:43:09 -0400 Subject: [PATCH 3/4] Bump to v5.4.2 Signed-off-by: Matt Heon --- version/rawversion/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/rawversion/version.go b/version/rawversion/version.go index fd87c75e74..6eaec48dd8 100644 --- a/version/rawversion/version.go +++ b/version/rawversion/version.go @@ -7,4 +7,4 @@ package rawversion // // NOTE: remember to bump the version at the top of the top-level README.md // file when this is bumped. -const RawVersion = "5.4.2-dev" +const RawVersion = "5.4.2" From ddfa2dd62341edbaf3e7118f73b5a8d3ab708f3c Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Wed, 2 Apr 2025 09:43:35 -0400 Subject: [PATCH 4/4] Bump to v5.4.3-dev Signed-off-by: Matt Heon --- version/rawversion/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/rawversion/version.go b/version/rawversion/version.go index 6eaec48dd8..84450383ef 100644 --- a/version/rawversion/version.go +++ b/version/rawversion/version.go @@ -7,4 +7,4 @@ package rawversion // // NOTE: remember to bump the version at the top of the top-level README.md // file when this is bumped. -const RawVersion = "5.4.2" +const RawVersion = "5.4.3-dev"