Skip to content
This repository was archived by the owner on Feb 8, 2021. It is now read-only.

Conversation

bergwolf
Copy link
Member

@bergwolf bergwolf commented Mar 6, 2017

If there is a symlink in the middle, resolve it as if we were in a chroot jail.

e.g., following pod file will fail to start without this PR, because nginx image has /var/run symlink pointing to /run. Fixes hyperhq/hyperd#521

{
        "containers" : [{
            "image": "nginx",
            "command": ["/bin/sh"],
            "user": {
                "name":"nobody"
            },
            "volumes": [{
                "volume": "tmp",
                "path": "/var/run/secrets/kubernetes.io/serviceaccount",
                "readOnly": false
             }]
        }],
        "resource": {
            "vcpu": 1,
            "memory": 256
        },
        "files": [],
        "volumes": [{
            "name": "tmp",
            "source": "/tmp",
            "format": "vfs"
        }],
        "tty": true
}

@bergwolf bergwolf force-pushed the symlink_mkdir branch 2 times, most recently from 7ea3d57 to 1c8393c Compare March 6, 2017 17:07
@bergwolf
Copy link
Member Author

bergwolf commented Mar 6, 2017

hykins failed with �Failed to mkfs the block:exec: "mkfs.xfs": executable file not found in $PATH:.

@gnawux
Copy link
Member

gnawux commented Mar 7, 2017

@bergwolf will check the hykins config. jimmy has fixed this for hyperd

@gnawux
Copy link
Member

gnawux commented Mar 7, 2017

I updated the jenkins configuration, could ask retest this please.

@bergwolf
Copy link
Member Author

bergwolf commented Mar 7, 2017

retest this please, @hykins

@feiskyer
Copy link
Contributor

feiskyer commented Mar 7, 2017

@bergwolf Confirmed this PR works properly. LGTM.

@feiskyer
Copy link
Contributor

feiskyer commented Mar 8, 2017

@bergwolf Any updates on this?

@bergwolf
Copy link
Member Author

bergwolf commented Mar 8, 2017

@feiskyer working on an updated version that handles symlinks in symlink targets.

@bergwolf
Copy link
Member Author

bergwolf commented Mar 8, 2017

updated to handle following additional cases:

  1. symlinks in symlink file contents.
  2. .. in path components in which case chroot jail is always valid.

For a container created with below Dockerfile:

FROM busybox
RUN mkdir /data; ln -s /etc/ /data/foo; ln -s ../../data/foo /etc/bar

Following pod file can still work:

{
        "containers" : [{
            "image": "bergwolf/test",
            "command": ["/bin/sh"],
            "user": {
                "name":"nobody"
            },
            "volumes": [{
                "volume": "tmp",
                "path": "/etc/bar/haha",
                "readOnly": false
             }]
        }],
        "resource": {
            "vcpu": 1,
            "memory": 256
        },
        "files": [],
        "volumes": [{
            "name": "tmp",
            "source": "/tmp",
            "format": "vfs"
        }],
        "tty": true
}

src/util.c Outdated
return NULL;
}

buf[len] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check the case len==sizeof(buf)

@bergwolf bergwolf force-pushed the symlink_mkdir branch 3 times, most recently from 59e5ef7 to e4dcdc1 Compare March 9, 2017 06:38
@bergwolf bergwolf closed this Mar 9, 2017
@bergwolf bergwolf reopened this Mar 9, 2017
If there is a symlink in the middle, resolve it as if we were in
a chroot jail. At most 40 symlinks are allowed to follow before
failing with ELOOP.

Signed-off-by: Peng Tao <bergwolf@gmail.com>
@laijs laijs merged commit 2175595 into hyperhq:master Mar 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Volume not mounted into container
4 participants