From a53930a04fa81b082aa78e66b342ff19cc63cc5f Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 14 Dec 2017 09:29:11 -0500 Subject: [PATCH] Plugins perform propagated mount in runtime spec Setting up the mounts on the host increases chances of mount leakage and makes for more cleanup after the plugin has stopped. With this change all mounts for the plugin are performed by the container runtime and automatically cleaned up when the container exits. Signed-off-by: Brian Goff --- daemon/graphdriver/plugin.go | 2 +- plugin/manager.go | 28 +++------------------------- plugin/manager_linux.go | 17 +++++------------ plugin/v2/plugin.go | 11 +++++------ plugin/v2/plugin_linux.go | 21 ++++++++++++++++----- 5 files changed, 30 insertions(+), 49 deletions(-) diff --git a/daemon/graphdriver/plugin.go b/daemon/graphdriver/plugin.go index 5d433e5196..9f560f0381 100644 --- a/daemon/graphdriver/plugin.go +++ b/daemon/graphdriver/plugin.go @@ -23,7 +23,7 @@ func newPluginDriver(name string, pl plugingetter.CompatPlugin, config Options) home := config.Root if !pl.IsV1() { if p, ok := pl.(*v2.Plugin); ok { - if p.PropagatedMount != "" { + if p.PluginObj.Config.PropagatedMount != "" { home = p.PluginObj.Config.PropagatedMount } } diff --git a/plugin/manager.go b/plugin/manager.go index d686443c6f..a163a2aa7e 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/layer" "github.com/docker/docker/pkg/authorization" "github.com/docker/docker/pkg/ioutils" - "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/pubsub" "github.com/docker/docker/pkg/system" "github.com/docker/docker/plugin/v2" @@ -151,16 +150,6 @@ func (pm *Manager) HandleExitEvent(id string) error { os.RemoveAll(filepath.Join(pm.config.ExecRoot, id)) - if p.PropagatedMount != "" { - if err := mount.Unmount(p.PropagatedMount); err != nil { - logrus.Warnf("Could not unmount %s: %v", p.PropagatedMount, err) - } - propRoot := filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") - if err := mount.Unmount(propRoot); err != nil { - logrus.Warn("Could not unmount %s: %v", propRoot, err) - } - } - pm.mu.RLock() c := pm.cMap[p] if c.exitChan != nil { @@ -239,28 +228,17 @@ func (pm *Manager) reload() error { // todo: restore // check if we need to migrate an older propagated mount from before // these mounts were stored outside the plugin rootfs if _, err := os.Stat(propRoot); os.IsNotExist(err) { - if _, err := os.Stat(p.PropagatedMount); err == nil { - // make sure nothing is mounted here - // don't care about errors - mount.Unmount(p.PropagatedMount) - if err := os.Rename(p.PropagatedMount, propRoot); err != nil { + rootfsProp := filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) + if _, err := os.Stat(rootfsProp); err == nil { + if err := os.Rename(rootfsProp, propRoot); err != nil { logrus.WithError(err).WithField("dir", propRoot).Error("error migrating propagated mount storage") } - if err := os.MkdirAll(p.PropagatedMount, 0755); err != nil { - logrus.WithError(err).WithField("dir", p.PropagatedMount).Error("error migrating propagated mount storage") - } } } if err := os.MkdirAll(propRoot, 0755); err != nil { logrus.Errorf("failed to create PropagatedMount directory at %s: %v", propRoot, err) } - // TODO: sanitize PropagatedMount and prevent breakout - p.PropagatedMount = filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) - if err := os.MkdirAll(p.PropagatedMount, 0755); err != nil { - logrus.Errorf("failed to create PropagatedMount directory at %s: %v", p.PropagatedMount, err) - return - } } } } diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index 65380af06f..00eaf869cc 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -22,7 +22,7 @@ import ( "golang.org/x/sys/unix" ) -func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) (err error) { +func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { p.Rootfs = filepath.Join(pm.config.Root, p.PluginObj.ID, "rootfs") if p.IsEnabled() && !force { return errors.Wrap(enabledError(p.Name()), "plugin already enabled") @@ -40,20 +40,16 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) (err error) { pm.mu.Unlock() var propRoot string - if p.PropagatedMount != "" { + if p.PluginObj.Config.PropagatedMount != "" { propRoot = filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") - if err = os.MkdirAll(propRoot, 0755); err != nil { + if err := os.MkdirAll(propRoot, 0755); err != nil { logrus.Errorf("failed to create PropagatedMount directory at %s: %v", propRoot, err) } - if err = mount.MakeRShared(propRoot); err != nil { + if err := mount.MakeRShared(propRoot); err != nil { return errors.Wrap(err, "error setting up propagated mount dir") } - - if err = mount.Mount(propRoot, p.PropagatedMount, "none", "rbind"); err != nil { - return errors.Wrap(err, "error creating mount for propagated mount") - } } rootFS := containerfs.NewLocalContainerFS(filepath.Join(pm.config.Root, p.PluginObj.ID, rootFSFileName)) @@ -63,10 +59,7 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) (err error) { stdout, stderr := makeLoggerStreams(p.GetID()) if err := pm.executor.Create(p.GetID(), *spec, stdout, stderr); err != nil { - if p.PropagatedMount != "" { - if err := mount.Unmount(p.PropagatedMount); err != nil { - logrus.Warnf("Could not unmount %s: %v", p.PropagatedMount, err) - } + if p.PluginObj.Config.PropagatedMount != "" { if err := mount.Unmount(propRoot); err != nil { logrus.Warnf("Could not unmount %s: %v", propRoot, err) } diff --git a/plugin/v2/plugin.go b/plugin/v2/plugin.go index a3e95f6cad..a4c7c535f6 100644 --- a/plugin/v2/plugin.go +++ b/plugin/v2/plugin.go @@ -14,12 +14,11 @@ import ( // Plugin represents an individual plugin. type Plugin struct { - mu sync.RWMutex - PluginObj types.Plugin `json:"plugin"` // todo: embed struct - pClient *plugins.Client - refCount int - PropagatedMount string // TODO: make private - Rootfs string // TODO: make private + mu sync.RWMutex + PluginObj types.Plugin `json:"plugin"` // todo: embed struct + pClient *plugins.Client + refCount int + Rootfs string // TODO: make private Config digest.Digest Blobsums []digest.Digest diff --git a/plugin/v2/plugin_linux.go b/plugin/v2/plugin_linux.go index b31a4b4c23..fa4be7a32c 100644 --- a/plugin/v2/plugin_linux.go +++ b/plugin/v2/plugin_linux.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "runtime" + "sort" "strings" "github.com/docker/docker/api/types" @@ -32,6 +33,17 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { return nil, errors.WithStack(err) } + if p.PluginObj.Config.PropagatedMount != "" { + pRoot := filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") + s.Mounts = append(s.Mounts, specs.Mount{ + Source: pRoot, + Destination: p.PluginObj.Config.PropagatedMount, + Type: "bind", + Options: []string{"rbind", "rw", "rshared"}, + }) + s.Linux.RootfsPropagation = "rshared" + } + mounts := append(p.PluginObj.Config.Mounts, types.PluginMount{ Source: &execRoot, Destination: defaultPluginRuntimeDestination, @@ -89,11 +101,6 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { } } - if p.PluginObj.Config.PropagatedMount != "" { - p.PropagatedMount = filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) - s.Linux.RootfsPropagation = "rshared" - } - if p.PluginObj.Config.Linux.AllowAllDevices { s.Linux.Resources.Devices = []specs.LinuxDeviceCgroup{{Allow: true, Access: "rwm"}} } @@ -131,5 +138,9 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { p.modifyRuntimeSpec(&s) } + sort.Slice(s.Mounts, func(i, j int) bool { + return s.Mounts[i].Destination < s.Mounts[j].Destination + }) + return &s, nil }