Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Podman run blocks in isPathOnVolume function forever #10216

Closed
bacher09 opened this issue May 5, 2021 · 3 comments · Fixed by #10238
Closed

Podman run blocks in isPathOnVolume function forever #10216

bacher09 opened this issue May 5, 2021 · 3 comments · Fixed by #10238
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@bacher09
Copy link
Contributor

bacher09 commented May 5, 2021

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

There is a bug in current isPathOnVolume function which in some cases leads to infinite loop which causes some commands to stuck. This happens cause filepath.Dir(dest) could return . in some cases and calling same method again would return same result (.).

Steps to reproduce the issue:

  1. Create a Docker image using this Dockerfile:
cat Dockerfile 
FROM alpine:3.12
VOLUME ['/etc/foo']
WORKDIR /etc/foo
CMD ["echo", "test"]

it can be compiled using any tool (I used original docker build).
NOTE: that VOLUME uses single quotes ' and not ".

  1. Pull that image into your system and make it available for podman

  2. Try to start that container using that image:

podman run testimg

and this would stuck forever in the loop.

Describe the results you received:

podman run testimg got stuck consuming cpu resources.

Describe the results you expected:

It should either fail or succeed. Interestingly spec like this works fine in a docker, it also used to work in a podman version 2.

Extra information

Basically using single quotes causes vol.Dest to become [/etc/foo] string and calling filepath.Dir on it will eventually return [ and then . so will never match /.

I skipped some fields cause these are not really necessary cause issue is really issue to reproduce.

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label May 5, 2021
@bacher09
Copy link
Contributor Author

bacher09 commented May 5, 2021

The simplest fix would be something like this:

diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go
index d798963b1..ec7306ca1 100644
--- a/libpod/container_path_resolution.go
+++ b/libpod/container_path_resolution.go
@@ -128,7 +128,7 @@ func isPathOnVolume(c *Container, containerPath string) bool {
                if cleanedContainerPath == filepath.Clean(vol.Dest) {
                        return true
                }
-               for dest := vol.Dest; dest != "/"; dest = filepath.Dir(dest) {
+               for dest := vol.Dest; dest != "/" && dest != "."; dest = filepath.Dir(dest) {
                        if cleanedContainerPath == dest {
                                return true
                        }

which should ensure that it would never loop forever in that code. But also IMHO it would be nice to have some validation for volume as well.

@mheon
Copy link
Member

mheon commented May 5, 2021

@vrothberg PTAL

@rhatdan
Copy link
Member

rhatdan commented May 5, 2021

@bacher09 Please open a PR to fix this?

bacher09 added a commit to bacher09/podman that referenced this issue May 5, 2021
filepath.Dir in some cases returns `.` symbol and calling this function
again returns same result. In such cases this function
never returns and causes some operations to stuck forever.

Closes containers#10216
bacher09 added a commit to bacher09/podman that referenced this issue May 5, 2021
filepath.Dir in some cases returns `.` symbol and calling this function
again returns same result. In such cases this function
never returns and causes some operations to stuck forever.

Closes containers#10216

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
bacher09 added a commit to bacher09/podman that referenced this issue May 6, 2021
filepath.Dir in some cases returns `.` symbol and calling this function
again returns same result. In such cases this function
never returns and causes some operations to stuck forever.

Closes containers#10216

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
bacher09 added a commit to bacher09/podman that referenced this issue May 6, 2021
filepath.Dir in some cases returns `.` symbol and calling this function
again returns same result. In such cases this function
never returns and causes some operations to stuck forever.

Closes containers#10216

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
bacher09 added a commit to bacher09/podman that referenced this issue May 6, 2021
filepath.Dir in some cases returns `.` symbol and calling this function
again returns same result. In such cases this function
never returns and causes some operations to stuck forever.

Closes containers#10216

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants