Browse code

Merge pull request #35830 from cpuguy83/unbindable_shm

Make container shm parent unbindable

Anusha Ragunathan authored on 2018/01/20 10:43:30
Showing 9 changed files
... ...
@@ -1022,14 +1022,23 @@ func (container *Container) InitializeStdio(iop *cio.DirectIO) (cio.IO, error) {
1022 1022
 	return &rio{IO: iop, sc: container.StreamConfig}, nil
1023 1023
 }
1024 1024
 
1025
+// MountsResourcePath returns the path where mounts are stored for the given mount
1026
+func (container *Container) MountsResourcePath(mount string) (string, error) {
1027
+	return container.GetRootResourcePath(filepath.Join("mounts", mount))
1028
+}
1029
+
1025 1030
 // SecretMountPath returns the path of the secret mount for the container
1026
-func (container *Container) SecretMountPath() string {
1027
-	return filepath.Join(container.Root, "secrets")
1031
+func (container *Container) SecretMountPath() (string, error) {
1032
+	return container.MountsResourcePath("secrets")
1028 1033
 }
1029 1034
 
1030 1035
 // SecretFilePath returns the path to the location of a secret on the host.
1031
-func (container *Container) SecretFilePath(secretRef swarmtypes.SecretReference) string {
1032
-	return filepath.Join(container.SecretMountPath(), secretRef.SecretID)
1036
+func (container *Container) SecretFilePath(secretRef swarmtypes.SecretReference) (string, error) {
1037
+	secrets, err := container.SecretMountPath()
1038
+	if err != nil {
1039
+		return "", err
1040
+	}
1041
+	return filepath.Join(secrets, secretRef.SecretID), nil
1033 1042
 }
1034 1043
 
1035 1044
 func getSecretTargetPath(r *swarmtypes.SecretReference) string {
... ...
@@ -1042,13 +1051,17 @@ func getSecretTargetPath(r *swarmtypes.SecretReference) string {
1042 1042
 
1043 1043
 // ConfigsDirPath returns the path to the directory where configs are stored on
1044 1044
 // disk.
1045
-func (container *Container) ConfigsDirPath() string {
1046
-	return filepath.Join(container.Root, "configs")
1045
+func (container *Container) ConfigsDirPath() (string, error) {
1046
+	return container.GetRootResourcePath("configs")
1047 1047
 }
1048 1048
 
1049 1049
 // ConfigFilePath returns the path to the on-disk location of a config.
1050
-func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) string {
1051
-	return filepath.Join(container.ConfigsDirPath(), configRef.ConfigID)
1050
+func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) {
1051
+	configs, err := container.ConfigsDirPath()
1052
+	if err != nil {
1053
+		return "", err
1054
+	}
1055
+	return filepath.Join(configs, configRef.ConfigID), nil
1052 1056
 }
1053 1057
 
1054 1058
 // CreateDaemonEnvironment creates a new environment variable slice for this container.
... ...
@@ -151,7 +151,7 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st
151 151
 
152 152
 // ShmResourcePath returns path to shm
153 153
 func (container *Container) ShmResourcePath() (string, error) {
154
-	return container.GetRootResourcePath("shm")
154
+	return container.MountsResourcePath("shm")
155 155
 }
156 156
 
157 157
 // HasMountFor checks if path is a mountpoint
... ...
@@ -218,49 +218,61 @@ func (container *Container) IpcMounts() []Mount {
218 218
 }
219 219
 
220 220
 // SecretMounts returns the mounts for the secret path.
221
-func (container *Container) SecretMounts() []Mount {
221
+func (container *Container) SecretMounts() ([]Mount, error) {
222 222
 	var mounts []Mount
223 223
 	for _, r := range container.SecretReferences {
224 224
 		if r.File == nil {
225 225
 			continue
226 226
 		}
227
+		src, err := container.SecretFilePath(*r)
228
+		if err != nil {
229
+			return nil, err
230
+		}
227 231
 		mounts = append(mounts, Mount{
228
-			Source:      container.SecretFilePath(*r),
232
+			Source:      src,
229 233
 			Destination: getSecretTargetPath(r),
230 234
 			Writable:    false,
231 235
 		})
232 236
 	}
233 237
 
234
-	return mounts
238
+	return mounts, nil
235 239
 }
236 240
 
237 241
 // UnmountSecrets unmounts the local tmpfs for secrets
238 242
 func (container *Container) UnmountSecrets() error {
239
-	if _, err := os.Stat(container.SecretMountPath()); err != nil {
243
+	p, err := container.SecretMountPath()
244
+	if err != nil {
245
+		return err
246
+	}
247
+	if _, err := os.Stat(p); err != nil {
240 248
 		if os.IsNotExist(err) {
241 249
 			return nil
242 250
 		}
243 251
 		return err
244 252
 	}
245 253
 
246
-	return detachMounted(container.SecretMountPath())
254
+	return mount.RecursiveUnmount(p)
247 255
 }
248 256
 
249 257
 // ConfigMounts returns the mounts for configs.
250
-func (container *Container) ConfigMounts() []Mount {
258
+func (container *Container) ConfigMounts() ([]Mount, error) {
251 259
 	var mounts []Mount
252 260
 	for _, configRef := range container.ConfigReferences {
253 261
 		if configRef.File == nil {
254 262
 			continue
255 263
 		}
264
+		src, err := container.ConfigFilePath(*configRef)
265
+		if err != nil {
266
+			return nil, err
267
+		}
256 268
 		mounts = append(mounts, Mount{
257
-			Source:      container.ConfigFilePath(*configRef),
269
+			Source:      src,
258 270
 			Destination: configRef.File.Name,
259 271
 			Writable:    false,
260 272
 		})
261 273
 	}
262 274
 
263
-	return mounts
275
+	return mounts, nil
264 276
 }
265 277
 
266 278
 type conflictingUpdateOptions string
... ...
@@ -54,22 +54,30 @@ func (container *Container) CreateSecretSymlinks() error {
54 54
 // SecretMounts returns the mount for the secret path.
55 55
 // All secrets are stored in a single mount on Windows. Target symlinks are
56 56
 // created for each secret, pointing to the files in this mount.
57
-func (container *Container) SecretMounts() []Mount {
57
+func (container *Container) SecretMounts() ([]Mount, error) {
58 58
 	var mounts []Mount
59 59
 	if len(container.SecretReferences) > 0 {
60
+		src, err := container.SecretMountPath()
61
+		if err != nil {
62
+			return nil, err
63
+		}
60 64
 		mounts = append(mounts, Mount{
61
-			Source:      container.SecretMountPath(),
65
+			Source:      src,
62 66
 			Destination: containerInternalSecretMountPath,
63 67
 			Writable:    false,
64 68
 		})
65 69
 	}
66 70
 
67
-	return mounts
71
+	return mounts, nil
68 72
 }
69 73
 
70 74
 // UnmountSecrets unmounts the fs for secrets
71 75
 func (container *Container) UnmountSecrets() error {
72
-	return os.RemoveAll(container.SecretMountPath())
76
+	p, err := container.SecretMountPath()
77
+	if err != nil {
78
+		return err
79
+	}
80
+	return os.RemoveAll(p)
73 81
 }
74 82
 
75 83
 // CreateConfigSymlinks creates symlinks to files in the config mount.
... ...
@@ -96,17 +104,21 @@ func (container *Container) CreateConfigSymlinks() error {
96 96
 // ConfigMounts returns the mount for configs.
97 97
 // All configs are stored in a single mount on Windows. Target symlinks are
98 98
 // created for each config, pointing to the files in this mount.
99
-func (container *Container) ConfigMounts() []Mount {
99
+func (container *Container) ConfigMounts() ([]Mount, error) {
100 100
 	var mounts []Mount
101 101
 	if len(container.ConfigReferences) > 0 {
102
+		src, err := container.ConfigsDirPath()
103
+		if err != nil {
104
+			return nil, err
105
+		}
102 106
 		mounts = append(mounts, Mount{
103
-			Source:      container.ConfigsDirPath(),
107
+			Source:      src,
104 108
 			Destination: containerInternalConfigsDirPath,
105 109
 			Writable:    false,
106 110
 		})
107 111
 	}
108 112
 
109
-	return mounts
113
+	return mounts, nil
110 114
 }
111 115
 
112 116
 // DetachAndUnmount unmounts all volumes.
... ...
@@ -165,7 +165,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
165 165
 		return nil
166 166
 	}
167 167
 
168
-	localMountPath := c.SecretMountPath()
168
+	localMountPath, err := c.SecretMountPath()
169
+	if err != nil {
170
+		return errors.Wrap(err, "error getting secrets mount dir")
171
+	}
169 172
 	logrus.Debugf("secrets: setting up secret dir: %s", localMountPath)
170 173
 
171 174
 	// retrieve possible remapped range start for root UID, GID
... ...
@@ -204,7 +207,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
204 204
 
205 205
 		// secrets are created in the SecretMountPath on the host, at a
206 206
 		// single level
207
-		fPath := c.SecretFilePath(*s)
207
+		fPath, err := c.SecretFilePath(*s)
208
+		if err != nil {
209
+			return errors.Wrap(err, "error getting secret file path")
210
+		}
208 211
 		if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil {
209 212
 			return errors.Wrap(err, "error creating secret mount path")
210 213
 		}
... ...
@@ -250,7 +256,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
250 250
 		return nil
251 251
 	}
252 252
 
253
-	localPath := c.ConfigsDirPath()
253
+	localPath, err := c.ConfigsDirPath()
254
+	if err != nil {
255
+		return err
256
+	}
254 257
 	logrus.Debugf("configs: setting up config dir: %s", localPath)
255 258
 
256 259
 	// retrieve possible remapped range start for root UID, GID
... ...
@@ -279,7 +288,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
279 279
 			continue
280 280
 		}
281 281
 
282
-		fPath := c.ConfigFilePath(*configRef)
282
+		fPath, err := c.ConfigFilePath(*configRef)
283
+		if err != nil {
284
+			return err
285
+		}
283 286
 
284 287
 		log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath})
285 288
 
... ...
@@ -379,3 +391,22 @@ func (daemon *Daemon) initializeNetworkingPaths(container *container.Container,
379 379
 	container.ResolvConfPath = nc.ResolvConfPath
380 380
 	return nil
381 381
 }
382
+
383
+func (daemon *Daemon) setupContainerMountsRoot(c *container.Container) error {
384
+	// get the root mount path so we can make it unbindable
385
+	p, err := c.MountsResourcePath("")
386
+	if err != nil {
387
+		return err
388
+	}
389
+
390
+	if err := idtools.MkdirAllAndChown(p, 0700, daemon.idMappings.RootPair()); err != nil {
391
+		return err
392
+	}
393
+
394
+	if err := mount.MakeUnbindable(p); err != nil {
395
+		// Setting unbindable is a precaution and is not neccessary for correct operation.
396
+		// Do not error out if this fails.
397
+		logrus.WithError(err).WithField("resource", p).WithField("container", c.ID).Warn("Error setting container resource mounts to unbindable, this may cause mount leakages, preventing removal of this container.")
398
+	}
399
+	return nil
400
+}
... ...
@@ -21,7 +21,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
21 21
 		return nil
22 22
 	}
23 23
 
24
-	localPath := c.ConfigsDirPath()
24
+	localPath, err := c.ConfigsDirPath()
25
+	if err != nil {
26
+		return err
27
+	}
25 28
 	logrus.Debugf("configs: setting up config dir: %s", localPath)
26 29
 
27 30
 	// create local config root
... ...
@@ -48,7 +51,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
48 48
 			continue
49 49
 		}
50 50
 
51
-		fPath := c.ConfigFilePath(*configRef)
51
+		fPath, err := c.ConfigFilePath(*configRef)
52
+		if err != nil {
53
+			return err
54
+		}
52 55
 
53 56
 		log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath})
54 57
 
... ...
@@ -94,7 +100,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
94 94
 		return nil
95 95
 	}
96 96
 
97
-	localMountPath := c.SecretMountPath()
97
+	localMountPath, err := c.SecretMountPath()
98
+	if err != nil {
99
+		return err
100
+	}
98 101
 	logrus.Debugf("secrets: setting up secret dir: %s", localMountPath)
99 102
 
100 103
 	// create local secret root
... ...
@@ -123,7 +132,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
123 123
 
124 124
 		// secrets are created in the SecretMountPath on the host, at a
125 125
 		// single level
126
-		fPath := c.SecretFilePath(*s)
126
+		fPath, err := c.SecretFilePath(*s)
127
+		if err != nil {
128
+			return err
129
+		}
127 130
 		logrus.WithFields(logrus.Fields{
128 131
 			"name": s.File.Name,
129 132
 			"path": fPath,
... ...
@@ -812,6 +812,10 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
812 812
 		return nil, fmt.Errorf("linux seccomp: %v", err)
813 813
 	}
814 814
 
815
+	if err := daemon.setupContainerMountsRoot(c); err != nil {
816
+		return nil, err
817
+	}
818
+
815 819
 	if err := daemon.setupIpcDirs(c); err != nil {
816 820
 		return nil, err
817 821
 	}
... ...
@@ -839,11 +843,17 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
839 839
 	}
840 840
 	ms = append(ms, tmpfsMounts...)
841 841
 
842
-	if m := c.SecretMounts(); m != nil {
843
-		ms = append(ms, m...)
842
+	secretMounts, err := c.SecretMounts()
843
+	if err != nil {
844
+		return nil, err
844 845
 	}
846
+	ms = append(ms, secretMounts...)
845 847
 
846
-	ms = append(ms, c.ConfigMounts()...)
848
+	configMounts, err := c.ConfigMounts()
849
+	if err != nil {
850
+		return nil, err
851
+	}
852
+	ms = append(ms, configMounts...)
847 853
 
848 854
 	sort.Sort(mounts(ms))
849 855
 	if err := setMounts(daemon, &s, c, ms); err != nil {
... ...
@@ -93,12 +93,20 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
93 93
 		}
94 94
 	}
95 95
 
96
-	if m := c.SecretMounts(); m != nil {
97
-		mounts = append(mounts, m...)
96
+	secretMounts, err := c.SecretMounts()
97
+	if err != nil {
98
+		return nil, err
99
+	}
100
+	if secretMounts != nil {
101
+		mounts = append(mounts, secretMounts...)
98 102
 	}
99 103
 
100
-	if m := c.ConfigMounts(); m != nil {
101
-		mounts = append(mounts, m...)
104
+	configMounts, err := c.ConfigMounts()
105
+	if err != nil {
106
+		return nil, err
107
+	}
108
+	if configMounts != nil {
109
+		mounts = append(mounts, configMounts...)
102 110
 	}
103 111
 
104 112
 	for _, mount := range mounts {
... ...
@@ -9,6 +9,7 @@ import (
9 9
 	containertypes "github.com/docker/docker/api/types/container"
10 10
 	"github.com/docker/docker/container"
11 11
 	"github.com/docker/docker/errdefs"
12
+	"github.com/docker/docker/pkg/mount"
12 13
 	"github.com/pkg/errors"
13 14
 	"github.com/sirupsen/logrus"
14 15
 )
... ...
@@ -231,6 +232,10 @@ func (daemon *Daemon) Cleanup(container *container.Container) {
231 231
 		logrus.Warnf("%s cleanup: failed to unmount secrets: %s", container.ID, err)
232 232
 	}
233 233
 
234
+	if err := mount.RecursiveUnmount(container.Root); err != nil {
235
+		logrus.WithError(err).WithField("container", container.ID).Warn("Error while cleaning up container resource mounts.")
236
+	}
237
+
234 238
 	for _, eConfig := range container.ExecCommands.Commands() {
235 239
 		daemon.unregisterExecCommand(container, eConfig)
236 240
 	}
237 241
new file mode 100644
... ...
@@ -0,0 +1,84 @@
0
+package container
1
+
2
+import (
3
+	"bytes"
4
+	"context"
5
+	"fmt"
6
+	"testing"
7
+
8
+	"github.com/docker/docker/api/types"
9
+	"github.com/docker/docker/api/types/container"
10
+	"github.com/docker/docker/api/types/mount"
11
+	"github.com/docker/docker/integration-cli/daemon"
12
+	"github.com/docker/docker/pkg/stdcopy"
13
+)
14
+
15
+func TestContainerShmNoLeak(t *testing.T) {
16
+	t.Parallel()
17
+	d := daemon.New(t, "docker", "dockerd", daemon.Config{})
18
+	client, err := d.NewClient()
19
+	if err != nil {
20
+		t.Fatal(err)
21
+	}
22
+	d.StartWithBusybox(t)
23
+	defer d.Stop(t)
24
+
25
+	ctx := context.Background()
26
+	cfg := container.Config{
27
+		Image: "busybox",
28
+		Cmd:   []string{"top"},
29
+	}
30
+
31
+	ctr, err := client.ContainerCreate(ctx, &cfg, nil, nil, "")
32
+	if err != nil {
33
+		t.Fatal(err)
34
+	}
35
+	defer client.ContainerRemove(ctx, ctr.ID, types.ContainerRemoveOptions{Force: true})
36
+
37
+	if err := client.ContainerStart(ctx, ctr.ID, types.ContainerStartOptions{}); err != nil {
38
+		t.Fatal(err)
39
+	}
40
+
41
+	// this should recursively bind mount everything in the test daemons root
42
+	// except of course we are hoping that the previous containers /dev/shm mount did not leak into this new container
43
+	hc := container.HostConfig{
44
+		Mounts: []mount.Mount{
45
+			{
46
+				Type:        mount.TypeBind,
47
+				Source:      d.Root,
48
+				Target:      "/testdaemonroot",
49
+				BindOptions: &mount.BindOptions{Propagation: mount.PropagationRPrivate}},
50
+		},
51
+	}
52
+	cfg.Cmd = []string{"/bin/sh", "-c", fmt.Sprintf("mount | grep testdaemonroot | grep containers | grep %s", ctr.ID)}
53
+	cfg.AttachStdout = true
54
+	cfg.AttachStderr = true
55
+	ctrLeak, err := client.ContainerCreate(ctx, &cfg, &hc, nil, "")
56
+	if err != nil {
57
+		t.Fatal(err)
58
+	}
59
+
60
+	attach, err := client.ContainerAttach(ctx, ctrLeak.ID, types.ContainerAttachOptions{
61
+		Stream: true,
62
+		Stdout: true,
63
+		Stderr: true,
64
+	})
65
+	if err != nil {
66
+		t.Fatal(err)
67
+	}
68
+
69
+	if err := client.ContainerStart(ctx, ctrLeak.ID, types.ContainerStartOptions{}); err != nil {
70
+		t.Fatal(err)
71
+	}
72
+
73
+	buf := bytes.NewBuffer(nil)
74
+
75
+	if _, err := stdcopy.StdCopy(buf, buf, attach.Reader); err != nil {
76
+		t.Fatal(err)
77
+	}
78
+
79
+	out := bytes.TrimSpace(buf.Bytes())
80
+	if !bytes.Equal(out, []byte{}) {
81
+		t.Fatalf("mount leaked: %s", string(out))
82
+	}
83
+}