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

Move libcontainerd root inside /var/lib/docker #22164

Closed
wants to merge 1 commit into from

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Apr 19, 2016

Because libcontainerd mounts the container rootfs to
/var/run/libcontainerd, and some people mount /var/run into the
container. This causes mounts used by other containers to leak into
containers that do mount /var/run.

This is really just moving the problem, but since
mounting /var/lib/docker into a container is already problematic
(for the same reasons) at least we can fix issues of people being able
to mount /var/run

I chose not to change the default exec root path since this also contains other items.
We may want to change this all anyway... and do so before container re-attach is stable, which would complicate this since we couldn't really clean up the old files at that point.

This is a work-around for #21969

Because libcontainerd mounts the container rootfs to
`/var/run/libcontainerd`, and some people mount `/var/run` into the
container. This causes mounts used by other containers to leak into
containers that do mount `/var/run`.

This is really just moving the problem since mounting, but since
mounting `/var/lib/docker` into a container is already problematic
(for the same reasons) at least we can fix issues of people being able
to mount `/var/run`

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83
Copy link
Member Author

ping @tonistiigi @anusha-ragunathan

@mlaventure
Copy link
Contributor

I think we rely on /var/run being a tmpfs mount for things to be cleaned up nicely on reboot if something went wrong.

@cpuguy83
Copy link
Member Author

@mlaventure We can set it up as a tmpfs.
I wasn't sure if we relied on that or not since with docker-in-docker it's not a tmpfs by default.

@crosbymichael
Copy link
Contributor

crosbymichael commented Apr 19, 2016

We could also remove the bind mount and just provide the graph driver mount path in the config

@cpuguy83
Copy link
Member Author

I like the way you think @crosbymichael. I'm going to close this one since it's kinda yuck anyway.

@cpuguy83 cpuguy83 closed this Apr 19, 2016
@cpuguy83 cpuguy83 deleted the 21969_mv_containerd_statedir branch April 19, 2016 23:59
@mlaventure mlaventure removed this from the 1.11.1 milestone Apr 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants