From 93e510e26fca90a90e10a550ce6ca8d7dfa0b55c Mon Sep 17 00:00:00 2001 From: moricho Date: Mon, 20 Apr 2020 16:46:22 +0900 Subject: [PATCH 1/3] fix behavior of `getMountNameAndOptions` when options include either bind or rbind Signed-off-by: moricho --- runsc/boot/fs.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 98cce60af..e1519673c 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -756,6 +756,15 @@ func (c *containerMounter) createRootMount(ctx context.Context, conf *Config) (* return rootInode, nil } +func (c *containerMounter) getBindMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool) { + fd := c.fds.remove() + fsName = "9p" + opts = p9MountOptions(fd, c.getMountAccessType(m)) + // If configured, add overlay to all writable mounts. + useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly + return fsName, opts, useOverlay +} + // getMountNameAndOptions retrieves the fsName, opts, and useOverlay values // used for mounts. func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool, error) { @@ -769,6 +778,13 @@ func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) ( case devpts, devtmpfs, proc, sysfs: fsName = m.Type case nonefs: + for _, opt := range m.Options { + if opt == "bind" || opt == "rbind" { + fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m) + return fsName, opts, useOverlay, nil + } + } + fsName = sysfs case tmpfs: fsName = m.Type @@ -778,15 +794,16 @@ func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) ( if err != nil { return "", nil, false, err } - case bind: - fd := c.fds.remove() - fsName = "9p" - opts = p9MountOptions(fd, c.getMountAccessType(m)) - // If configured, add overlay to all writable mounts. - useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly - + fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m) default: + for _, opt := range m.Options { + if opt == "bind" || opt == "rbind" { + fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m) + return fsName, opts, useOverlay, nil + } + } + log.Warningf("ignoring unknown filesystem type %q", m.Type) } return fsName, opts, useOverlay, nil From 0b3166f6243472fbb72cc749c57d3a59aa481979 Mon Sep 17 00:00:00 2001 From: moricho Date: Mon, 20 Apr 2020 17:03:00 +0900 Subject: [PATCH 2/3] add bind/rbind options for mount Signed-off-by: moricho --- pkg/sentry/fs/filesystems.go | 4 ++++ runsc/boot/fs.go | 23 ++++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/sentry/fs/filesystems.go b/pkg/sentry/fs/filesystems.go index 084da2a8d..d6abddfd8 100644 --- a/pkg/sentry/fs/filesystems.go +++ b/pkg/sentry/fs/filesystems.go @@ -144,6 +144,10 @@ type MountSourceFlags struct { // NoExec corresponds to mount(2)'s "MS_NOEXEC" and indicates that // binaries from this file system can't be executed. NoExec bool + + // Bind corresponds to mount(2)'s MS_BIND and indicates that + // the filesystem should be bind mounted. + Bind bool } // GenericMountSourceOptions splits a string containing comma separated tokens of the diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index e1519673c..78d6a0c14 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -63,8 +63,13 @@ const ( nonefs = "none" ) -// tmpfs has some extra supported options that we must pass through. -var tmpfsAllowedOptions = []string{"mode", "uid", "gid"} +var ( + // tmpfs has some extra supported options that we must pass through. + tmpfsAllowedOptions = []string{"mode", "uid", "gid"} + + // filesystems supported on gVisor. + supportedFilesystems = []string{bind, devpts, devtmpfs, proc, sysfs, tmpfs} +) func addOverlay(ctx context.Context, conf *Config, lower *fs.Inode, name string, lowerFlags fs.MountSourceFlags) (*fs.Inode, error) { // Upper layer uses the same flags as lower, but it must be read-write. @@ -219,6 +224,8 @@ func mountFlags(opts []string) fs.MountSourceFlags { mf.NoAtime = true case "noexec": mf.NoExec = true + case "bind", "rbind": + mf.Bind = true default: log.Warningf("ignoring unknown mount option %q", o) } @@ -230,6 +237,10 @@ func isSupportedMountFlag(fstype, opt string) bool { switch opt { case "rw", "ro", "noatime", "noexec": return true + case "bind", "rbind": + if fstype == nonefs || !specutils.ContainsStr(supportedFilesystems, fstype) { + return true + } } if fstype == tmpfs { ok, err := parseMountOption(opt, tmpfsAllowedOptions...) @@ -756,12 +767,14 @@ func (c *containerMounter) createRootMount(ctx context.Context, conf *Config) (* return rootInode, nil } +// getBindMountNameAndOptions retrieves the fsName, opts, and useOverlay values +// used for bind mounts. func (c *containerMounter) getBindMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool) { fd := c.fds.remove() - fsName = "9p" - opts = p9MountOptions(fd, c.getMountAccessType(m)) + fsName := "9p" + opts := p9MountOptions(fd, c.getMountAccessType(m)) // If configured, add overlay to all writable mounts. - useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly + useOverlay := conf.Overlay && !mountFlags(m.Options).ReadOnly return fsName, opts, useOverlay } From fc53d6436776d5de052075e98f44417f04ced7e7 Mon Sep 17 00:00:00 2001 From: moricho Date: Wed, 22 Apr 2020 15:04:18 +0900 Subject: [PATCH 3/3] refactor and add test for bindmount Signed-off-by: moricho --- pkg/sentry/fs/filesystems.go | 4 -- runsc/boot/fs.go | 61 +++++++++---------------- runsc/container/container_test.go | 22 +++++++++ runsc/container/multi_container_test.go | 2 +- runsc/specutils/specutils.go | 14 +++++- 5 files changed, 58 insertions(+), 45 deletions(-) diff --git a/pkg/sentry/fs/filesystems.go b/pkg/sentry/fs/filesystems.go index d6abddfd8..084da2a8d 100644 --- a/pkg/sentry/fs/filesystems.go +++ b/pkg/sentry/fs/filesystems.go @@ -144,10 +144,6 @@ type MountSourceFlags struct { // NoExec corresponds to mount(2)'s "MS_NOEXEC" and indicates that // binaries from this file system can't be executed. NoExec bool - - // Bind corresponds to mount(2)'s MS_BIND and indicates that - // the filesystem should be bind mounted. - Bind bool } // GenericMountSourceOptions splits a string containing comma separated tokens of the diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 78d6a0c14..4875452e2 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -63,13 +63,8 @@ const ( nonefs = "none" ) -var ( - // tmpfs has some extra supported options that we must pass through. - tmpfsAllowedOptions = []string{"mode", "uid", "gid"} - - // filesystems supported on gVisor. - supportedFilesystems = []string{bind, devpts, devtmpfs, proc, sysfs, tmpfs} -) +// tmpfs has some extra supported options that we must pass through. +var tmpfsAllowedOptions = []string{"mode", "uid", "gid"} func addOverlay(ctx context.Context, conf *Config, lower *fs.Inode, name string, lowerFlags fs.MountSourceFlags) (*fs.Inode, error) { // Upper layer uses the same flags as lower, but it must be read-write. @@ -225,7 +220,8 @@ func mountFlags(opts []string) fs.MountSourceFlags { case "noexec": mf.NoExec = true case "bind", "rbind": - mf.Bind = true + // When options include either "bind" or "rbind", + // it's converted to a 9P mount. default: log.Warningf("ignoring unknown mount option %q", o) } @@ -237,10 +233,6 @@ func isSupportedMountFlag(fstype, opt string) bool { switch opt { case "rw", "ro", "noatime", "noexec": return true - case "bind", "rbind": - if fstype == nonefs || !specutils.ContainsStr(supportedFilesystems, fstype) { - return true - } } if fstype == tmpfs { ok, err := parseMountOption(opt, tmpfsAllowedOptions...) @@ -767,17 +759,6 @@ func (c *containerMounter) createRootMount(ctx context.Context, conf *Config) (* return rootInode, nil } -// getBindMountNameAndOptions retrieves the fsName, opts, and useOverlay values -// used for bind mounts. -func (c *containerMounter) getBindMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool) { - fd := c.fds.remove() - fsName := "9p" - opts := p9MountOptions(fd, c.getMountAccessType(m)) - // If configured, add overlay to all writable mounts. - useOverlay := conf.Overlay && !mountFlags(m.Options).ReadOnly - return fsName, opts, useOverlay -} - // getMountNameAndOptions retrieves the fsName, opts, and useOverlay values // used for mounts. func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) (string, []string, bool, error) { @@ -787,17 +768,20 @@ func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) ( useOverlay bool ) + for _, opt := range m.Options { + // When options include either "bind" or "rbind", this behaves as + // bind mount even if the mount type is equal to a filesystem supported + // on runsc. + if opt == "bind" || opt == "rbind" { + m.Type = bind + break + } + } + switch m.Type { case devpts, devtmpfs, proc, sysfs: fsName = m.Type case nonefs: - for _, opt := range m.Options { - if opt == "bind" || opt == "rbind" { - fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m) - return fsName, opts, useOverlay, nil - } - } - fsName = sysfs case tmpfs: fsName = m.Type @@ -807,16 +791,15 @@ func (c *containerMounter) getMountNameAndOptions(conf *Config, m specs.Mount) ( if err != nil { return "", nil, false, err } - case bind: - fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m) - default: - for _, opt := range m.Options { - if opt == "bind" || opt == "rbind" { - fsName, opts, useOverlay = c.getBindMountNameAndOptions(conf, m) - return fsName, opts, useOverlay, nil - } - } + case bind: + fd := c.fds.remove() + fsName = "9p" + opts = p9MountOptions(fd, c.getMountAccessType(m)) + // If configured, add overlay to all writable mounts. + useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly + + default: log.Warningf("ignoring unknown filesystem type %q", m.Type) } return fsName, opts, useOverlay, nil diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index 3ff89f38c..c963c2153 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -1523,6 +1523,28 @@ func TestReadonlyMount(t *testing.T) { } } +func TestBindMountByOption(t *testing.T) { + for _, conf := range configs(t, overlay) { + t.Logf("Running test with conf: %+v", conf) + + dir, err := ioutil.TempDir(testutil.TmpDir(), "bind-mount") + spec := testutil.NewSpecWithArgs("/bin/touch", path.Join(dir, "file")) + if err != nil { + t.Fatalf("ioutil.TempDir() failed: %v", err) + } + spec.Mounts = append(spec.Mounts, specs.Mount{ + Destination: dir, + Source: dir, + Type: "none", + Options: []string{"rw", "bind"}, + }) + + if err := run(spec, conf); err != nil { + t.Fatalf("error running sandbox: %v", err) + } + } +} + // TestAbbreviatedIDs checks that runsc supports using abbreviated container // IDs in place of full IDs. func TestAbbreviatedIDs(t *testing.T) { diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index e3704b453..f6861b1dd 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -1394,7 +1394,7 @@ func TestMultiContainerSharedMountUnsupportedOptions(t *testing.T) { Destination: "/mydir/test", Source: "/some/dir", Type: "tmpfs", - Options: []string{"rw", "rbind", "relatime"}, + Options: []string{"rw", "relatime"}, } podSpec[0].Mounts = append(podSpec[0].Mounts, mnt0) diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 837d5e238..202518b58 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -311,7 +311,19 @@ func capsFromNames(names []string, skipSet map[linux.Capability]struct{}) (auth. // Is9PMount returns true if the given mount can be mounted as an external gofer. func Is9PMount(m specs.Mount) bool { - return m.Type == "bind" && m.Source != "" && IsSupportedDevMount(m) + var isBind bool + switch m.Type { + case "bind": + isBind = true + default: + for _, opt := range m.Options { + if opt == "bind" || opt == "rbind" { + isBind = true + break + } + } + } + return isBind && m.Source != "" && IsSupportedDevMount(m) } // IsSupportedDevMount returns true if the mount is a supported /dev mount.