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

rootfs with symlinks path components #586

Closed
giuseppe opened this issue Feb 24, 2016 · 9 comments
Closed

rootfs with symlinks path components #586

giuseppe opened this issue Feb 24, 2016 · 9 comments

Comments

@giuseppe
Copy link
Member

is there a reason why it is not possible to run a container from a rootfs which has a symlink in its path?

I see it is checked explicitly by rootfs(config *configs.Config) in libcontainer/configs/validate/config.go, but can the evaluated path be used instead of returning an error?

For a quick test I tried this patch:

diff --git a/libcontainer/configs/validate/config.go b/libcontainer/configs/validate/config.go
index 848a67c..519a779 100644
--- a/libcontainer/configs/validate/config.go
+++ b/libcontainer/configs/validate/config.go
@@ -48,9 +48,7 @@ func (v *ConfigValidator) rootfs(config *configs.Config) error {
        if cleaned, err = filepath.EvalSymlinks(cleaned); err != nil {
                return err
        }
-       if config.Rootfs != cleaned {
-               return fmt.Errorf("%s is not an absolute path or is a symlink", config.Rootfs)
-       }
+       config.Rootfs = cleaned
        return nil
 }

but I suppose ConfigValidator is not meant to modify the configuration. What would be the best place to store this information?

@mikebrow
Copy link
Member

Maybe @crosbymichael will explain why that test was put in. The spec does not specify one way or the other whether such limitations MUST or MAY be placed on the rootfs. It went in a long time ago maybe it's time to pull it out?

@crosbymichael
Copy link
Member

There are various security issues and bugs that happen when rootfs is a symlink. If you want it to live somewhere else, you should use a bind mount.

There is alot of bugs and weird things that happen when you have this as a symlink so that is why we made this a requirement in libcontainer.

@giuseppe
Copy link
Member Author

what kinds of issues? Would not these be fixed if any symlink in the rootfs is resolved before using it (as my hacky patch does)?

@c4milo
Copy link

c4milo commented Aug 2, 2017

@crosbymichael is there anyway to force runc to follow the symlink?

@c4milo
Copy link

c4milo commented Aug 2, 2017

nvm, I'm re-rendering config.json instead. It works just like a symlink.

@cyphar
Copy link
Member

cyphar commented Aug 3, 2017

@giuseppe Well, for one you can change the symlink after runc has started and now a higher level runner will think that rootfs is different from the actual rootfs. Granted, you could probably do the same with multiple bind-mounts, but I think the concern is that we use Lstat and Stat in ways that means we'd have to carefully audit things to make sure that symlink rootfs wouldn't break anything.

stefanberger pushed a commit to stefanberger/runc that referenced this issue Sep 8, 2017
…-required-type

specs-go/config: fix required items type
@eMPee584
Copy link

@crosbymichael a warning message more grokable than

docker: Error response from daemon: OCI runtime create failed: /var/lib/docker/overlay2/959e47dfae61bae2601529408eadb1419b631fa12af46b5f24c10e752d3
d8134/merged is not an absolute path or is a symlink: unknown.

would be nice. An addition such as

(!) /var/lib/docker is a symlink, this causes problems and is not supported.
(i) You should bind-mount the directory instead (c.f. http://…/docs#rootfs).

@eMPee584
Copy link

eMPee584 commented Aug 17, 2018

Oh, I only now realized this is most probably not the right project to point my suggestion at (as I'm not even using runc)... Or is it 😁

@crosbymichael
Copy link
Member

@eMPee584 no problem. How are you getting a symlinked rootfs when using docker?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants