From 0f1d6772c69f0be56bc943f1e67979c3045bde0f Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 26 Apr 2016 00:15:17 +1000 Subject: [PATCH 1/2] libcontainer: rootfs: use CleanPath when comparing paths Comparisons with paths aren't really a good idea unless you're guaranteed that the comparison will work will all paths that resolve to the same lexical path as the compared path. Signed-off-by: Aleksa Sarai --- libcontainer/rootfs_linux.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index fe1db75eb..568dbf06b 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -28,7 +28,7 @@ const defaultMountFlags = syscall.MS_NOEXEC | syscall.MS_NOSUID | syscall.MS_NOD // needsSetupDev returns true if /dev needs to be set up. func needsSetupDev(config *configs.Config) bool { for _, m := range config.Mounts { - if m.Device == "bind" && (m.Destination == "/dev" || m.Destination == "/dev/") { + if m.Device == "bind" && libcontainerUtils.CleanPath(m.Destination) == "/dev" { return false } } @@ -95,7 +95,7 @@ func setupRootfs(config *configs.Config, console *linuxConsole, pipe io.ReadWrit } // remount dev as ro if specifed for _, m := range config.Mounts { - if m.Destination == "/dev" { + if libcontainerUtils.CleanPath(m.Destination) == "/dev" { if m.Flags&syscall.MS_RDONLY != 0 { if err := remountReadonly(m.Destination); err != nil { return newSystemErrorWithCausef(err, "remounting %q as readonly", m.Destination) @@ -713,7 +713,7 @@ func mountPropagate(m *configs.Mount, rootfs string, mountLabel string) error { data = label.FormatMountLabel(m.Data, mountLabel) flags = m.Flags ) - if dest == "/dev" { + if libcontainerUtils.CleanPath(dest) == "/dev" { flags &= ^syscall.MS_RDONLY } if !strings.HasPrefix(dest, rootfs) { From c29695ad0a1473c5c8474ffbd14a4ffab45a511c Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 22 Jun 2016 01:43:01 +1000 Subject: [PATCH 2/2] rootfs: don't change directory There's no point in changing directory here. Syscalls are resolved local to the linkpath, not to the current directory that the process was in when creating the symlink. Changing directories just confuses people who are trying to debug things. Signed-off-by: Aleksa Sarai --- libcontainer/rootfs_linux.go | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 568dbf06b..943b2fc09 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -238,29 +238,16 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error { return err } } - // create symlinks for merged cgroups - cwd, err := os.Getwd() - if err != nil { - return err - } - if err := os.Chdir(filepath.Join(rootfs, m.Destination)); err != nil { - return err - } for _, mc := range merged { for _, ss := range strings.Split(mc, ",") { - if err := os.Symlink(mc, ss); err != nil { - // if cgroup already exists, then okay(it could have been created before) - if os.IsExist(err) { - continue - } - os.Chdir(cwd) + // symlink(2) is very dumb, it will just shove the path into + // the link and doesn't do any checks or relative path + // conversion. Also, don't error out if the cgroup already exists. + if err := os.Symlink(mc, filepath.Join(rootfs, m.Destination, ss)); err != nil && !os.IsExist(err) { return err } } } - if err := os.Chdir(cwd); err != nil { - return err - } if m.Flags&syscall.MS_RDONLY != 0 { // remount cgroup root as readonly mcgrouproot := &configs.Mount{