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 <cpuguy83@gmail.com>

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
 			}
 		}

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
-						}
 					}
 				}
 			}

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)
 			}

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

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
 }