Do not make graphdriver homes private mounts.
The idea behind making the graphdrivers private is to prevent leaking
mounts into other namespaces.
Unfortunately this is not really what happens.
There is one case where this does work, and that is when the namespace
was created before the daemon's namespace.
However with systemd each system servie winds up with it's own mount
namespace. This causes a race betwen daemon startup and other system
services as to if the mount is actually private.
This also means there is a negative impact when other system services
are started while the daemon is running.
Basically there are too many things that the daemon does not have
control over (nor should it) to be able to protect against these kinds
of leakages. One thing is certain, setting the graphdriver roots to
private disconnects the mount ns heirarchy preventing propagation of
unmounts... new mounts are of course not propagated either, but the
behavior is racey (or just bad in the case of restarting services)... so
it's better to just be able to keep mount propagation in tact.
It also does not protect situations like `-v
/var/lib/docker:/var/lib/docker` where all mounts are recursively bound
into the container anyway.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Brian Goff authored on 2018/01/18 11:17:26Showing 6 changed files
- daemon/graphdriver/aufs/aufs.go
- daemon/graphdriver/btrfs/btrfs.go
- daemon/graphdriver/devmapper/driver.go
- daemon/graphdriver/overlay/overlay.go
- daemon/graphdriver/overlay2/overlay.go
- daemon/graphdriver/zfs/zfs.go
| ... | ... |
@@ -136,10 +136,6 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap |
| 136 | 136 |
return nil, err |
| 137 | 137 |
} |
| 138 | 138 |
|
| 139 |
- if err := mountpk.MakePrivate(root); err != nil {
|
|
| 140 |
- return nil, err |
|
| 141 |
- } |
|
| 142 |
- |
|
| 143 | 139 |
// Populate the dir structure |
| 144 | 140 |
for _, p := range paths {
|
| 145 | 141 |
if err := idtools.MkdirAllAndChown(path.Join(root, p), 0700, idtools.IDPair{UID: rootUID, GID: rootGID}); err != nil {
|
| ... | ... |
@@ -607,7 +603,7 @@ func (a *Driver) Cleanup() error {
|
| 607 | 607 |
logrus.Debugf("aufs error unmounting %s: %s", m, err)
|
| 608 | 608 |
} |
| 609 | 609 |
} |
| 610 |
- return mountpk.Unmount(a.root) |
|
| 610 |
+ return mountpk.RecursiveUnmount(a.root) |
|
| 611 | 611 |
} |
| 612 | 612 |
|
| 613 | 613 |
func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err error) {
|
| ... | ... |
@@ -77,10 +77,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap |
| 77 | 77 |
return nil, err |
| 78 | 78 |
} |
| 79 | 79 |
|
| 80 |
- if err := mount.MakePrivate(home); err != nil {
|
|
| 81 |
- return nil, err |
|
| 82 |
- } |
|
| 83 |
- |
|
| 84 | 80 |
opt, userDiskQuota, err := parseOptions(options) |
| 85 | 81 |
if err != nil {
|
| 86 | 82 |
return nil, err |
| ... | ... |
@@ -167,7 +163,7 @@ func (d *Driver) Cleanup() error {
|
| 167 | 167 |
return err |
| 168 | 168 |
} |
| 169 | 169 |
|
| 170 |
- return mount.Unmount(d.home) |
|
| 170 |
+ return mount.RecursiveUnmount(d.home) |
|
| 171 | 171 |
} |
| 172 | 172 |
|
| 173 | 173 |
func free(p *C.char) {
|
| ... | ... |
@@ -42,10 +42,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap |
| 42 | 42 |
return nil, err |
| 43 | 43 |
} |
| 44 | 44 |
|
| 45 |
- if err := mount.MakePrivate(home); err != nil {
|
|
| 46 |
- return nil, err |
|
| 47 |
- } |
|
| 48 |
- |
|
| 49 | 45 |
d := &Driver{
|
| 50 | 46 |
DeviceSet: deviceSet, |
| 51 | 47 |
home: home, |
| ... | ... |
@@ -127,7 +123,7 @@ func (d *Driver) GetMetadata(id string) (map[string]string, error) {
|
| 127 | 127 |
func (d *Driver) Cleanup() error {
|
| 128 | 128 |
err := d.DeviceSet.Shutdown(d.home) |
| 129 | 129 |
|
| 130 |
- if err2 := mount.Unmount(d.home); err == nil {
|
|
| 130 |
+ if err2 := mount.RecursiveUnmount(d.home); err == nil {
|
|
| 131 | 131 |
err = err2 |
| 132 | 132 |
} |
| 133 | 133 |
|
| ... | ... |
@@ -164,10 +164,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap |
| 164 | 164 |
return nil, err |
| 165 | 165 |
} |
| 166 | 166 |
|
| 167 |
- if err := mount.MakePrivate(home); err != nil {
|
|
| 168 |
- return nil, err |
|
| 169 |
- } |
|
| 170 |
- |
|
| 171 | 167 |
d := &Driver{
|
| 172 | 168 |
home: home, |
| 173 | 169 |
uidMaps: uidMaps, |
| ... | ... |
@@ -248,7 +244,7 @@ func (d *Driver) GetMetadata(id string) (map[string]string, error) {
|
| 248 | 248 |
// is being shutdown. For now, we just have to unmount the bind mounted |
| 249 | 249 |
// we had created. |
| 250 | 250 |
func (d *Driver) Cleanup() error {
|
| 251 |
- return mount.Unmount(d.home) |
|
| 251 |
+ return mount.RecursiveUnmount(d.home) |
|
| 252 | 252 |
} |
| 253 | 253 |
|
| 254 | 254 |
// CreateReadWrite creates a layer that is writable for use as a container |
| ... | ... |
@@ -201,10 +201,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap |
| 201 | 201 |
return nil, err |
| 202 | 202 |
} |
| 203 | 203 |
|
| 204 |
- if err := mount.MakePrivate(home); err != nil {
|
|
| 205 |
- return nil, err |
|
| 206 |
- } |
|
| 207 |
- |
|
| 208 | 204 |
d := &Driver{
|
| 209 | 205 |
home: home, |
| 210 | 206 |
uidMaps: uidMaps, |
| ... | ... |
@@ -335,7 +331,7 @@ func (d *Driver) GetMetadata(id string) (map[string]string, error) {
|
| 335 | 335 |
// is being shutdown. For now, we just have to unmount the bind mounted |
| 336 | 336 |
// we had created. |
| 337 | 337 |
func (d *Driver) Cleanup() error {
|
| 338 |
- return mount.Unmount(d.home) |
|
| 338 |
+ return mount.RecursiveUnmount(d.home) |
|
| 339 | 339 |
} |
| 340 | 340 |
|
| 341 | 341 |
// CreateReadWrite creates a layer that is writable for use as a container |
| ... | ... |
@@ -108,9 +108,6 @@ func Init(base string, opt []string, uidMaps, gidMaps []idtools.IDMap) (graphdri |
| 108 | 108 |
return nil, fmt.Errorf("Failed to create '%s': %v", base, err)
|
| 109 | 109 |
} |
| 110 | 110 |
|
| 111 |
- if err := mount.MakePrivate(base); err != nil {
|
|
| 112 |
- return nil, err |
|
| 113 |
- } |
|
| 114 | 111 |
d := &Driver{
|
| 115 | 112 |
dataset: rootDataset, |
| 116 | 113 |
options: options, |
| ... | ... |
@@ -181,10 +178,9 @@ func (d *Driver) String() string {
|
| 181 | 181 |
return "zfs" |
| 182 | 182 |
} |
| 183 | 183 |
|
| 184 |
-// Cleanup is called on daemon shutdown. It unmounts the bind mount |
|
| 185 |
-// created by mount.MakePrivate() in Init(). |
|
| 184 |
+// Cleanup is called on daemon shutdown, it is used to clean up any remaining mounts |
|
| 186 | 185 |
func (d *Driver) Cleanup() error {
|
| 187 |
- return mount.Unmount(d.options.mountPath) |
|
| 186 |
+ return mount.RecursiveUnmount(d.options.mountPath) |
|
| 188 | 187 |
} |
| 189 | 188 |
|
| 190 | 189 |
// Status returns information about the ZFS filesystem. It returns a two dimensional array of information |