Skip to content

Commit

Permalink
Do not use rbind mount for archival and other use cases
Browse files Browse the repository at this point in the history
Fix moby#20670, which reports that /dev/pts is unmounted on the host when `docker cp` is attempted to a container running with `-v /dev:/dev` without the systemd option MountFlags=slave.

Note that moby#22009 which uses rprivate mount was not enough in a corner case described below.

How to test:

 - Start the daemon without systemd option MountFlags=slave.
 - On terminal 1, run `docker run -it --name test_container --rm busybox`, and keep the container running.
 - On terminal 2, run `docker cp test_container:/bin/sh /tmp`, and make sure /dev/pts is kept by running `mount | grep pts`.
   The original issue moby#20670 reports that /dev/pts is unmounted at this point.
 - On terminal 1, press ^D to shut down the container, and make sure /dev/pts is still kept.
   The new issue appeared in moby#22009 reports that it is unmounted at this point.

Signed-off-by: Akihiro Suda <[email protected]>
  • Loading branch information
AkihiroSuda committed Apr 14, 2016
1 parent 7268eb9 commit c85e9e5
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 7 deletions.
9 changes: 2 additions & 7 deletions daemon/container_operations_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,7 @@ func (daemon *Daemon) mountVolumes(container *container.Container) error {
return err
}

opts := "rbind,ro"
if m.Writable {
opts = "rbind,rw"
}

if err := mount.Mount(m.Source, dest, "bind", opts); err != nil {
if err := mount.BindTree(m.Source, dest, m.Writable); err != nil {
return err
}
}
Expand Down Expand Up @@ -342,7 +337,7 @@ func getDevicesFromPath(deviceMapping containertypes.DeviceMapping) (devs []spec
}

func detachMounted(path string) error {
return syscall.Unmount(path, syscall.MNT_DETACH)
return mount.UnbindTree(path)
}

func isLinkable(child *container.Container) bool {
Expand Down
116 changes: 116 additions & 0 deletions pkg/mount/bindtree_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// +build linux

package mount

import (
"path/filepath"
"sort"
"strings"
"syscall"
)

// BindTree bind-mounts the tree recursively.
// See also docker/docker#20670.
func BindTree(source, dest string, rw bool) error {
mounts, err := GetMounts()
if err != nil {
return err
}
opts := "bind,ro"
if rw {
opts = "bind,rw"
}
f := func(source, dest string) error {
return Mount(source, dest, "bind", opts)
}
// the function is split for ease of mocking.
return bindTree(mounts, source, dest, f)
}

func bindTree(mounts []*Info, source, dest string,
f func(source, dest string) error) error {
// sort the mounts in top-down order, using the number of '/' in m.Mountpoint
sort.Sort(mountsByNumSep(mounts))
hitSource := false
for _, m := range mounts {
if m.Mountpoint == source {
hitSource = true
}
// typically m.Mountpoint="/dev/pts" (or just match the source),
// source="/dev", dest="/var/lib/docker/overlay/foobar/merged/dev"
if !strings.HasPrefix(m.Mountpoint, source) {
continue
}
childSource := m.Mountpoint
childDest := filepath.Join(filepath.Dir(dest), m.Mountpoint)
if err := f(childSource, childDest); err != nil {
return err
}

}
if !hitSource {
if err := f(source, dest); err != nil {
return err
}
}
return nil
}

// UnbindTree unmounts the bind-mounted tree recursively.
func UnbindTree(dest string) error {
mounts, err := GetMounts()
if err != nil {
return err
}
f := func(dest string) error {
// should we move MNT_DETACH to elsewhere?
return syscall.Unmount(dest, syscall.MNT_DETACH)
}
// the function is split for ease of mocking,
return unbindTree(mounts, dest, f)
}

func unbindTree(mounts []*Info, dest string,
f func(dest string) error) error {
// sort the mounts in bottom-up order, using the number of '/' in m.Mountpoint
sort.Sort(sort.Reverse(mountsByNumSep(mounts)))
hitDest := false
for _, m := range mounts {
if m.Mountpoint == dest {
hitDest = true
}
// typically dest="/var/lib/docker/overlay/foobar/merged/dev"
// m.Mountpoint="/var/lib/docker/overlay/foobar/merged/dev/pts"
// (or just match the dest)
if !strings.HasPrefix(m.Mountpoint, dest) {
continue
}

if err := f(m.Mountpoint); err != nil {
return err
}
}
if !hitDest {
if err := f(dest); err != nil {
return err
}
}
return nil
}

type mountsByNumSep []*Info

func (mounts mountsByNumSep) Len() int {
return len(mounts)
}

func (mounts mountsByNumSep) Swap(i, j int) {
mounts[i], mounts[j] = mounts[j], mounts[i]
}

func (mounts mountsByNumSep) Less(i, j int) bool {
sep := "/"
c := strings.Count(mounts[i].Mountpoint, sep)
d := strings.Count(mounts[j].Mountpoint, sep)
return c < d
}

0 comments on commit c85e9e5

Please sign in to comment.