Merge pull request #34573 from cyphar/dm-dos-prevention-remove-mountpoint
devicemapper: remove container rootfs mountPath after umount
diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index deb8c87..db42aad 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -2428,6 +2428,18 @@
}
logrus.Debug("devmapper: Unmount done")
+ // Remove the mountpoint here. Removing the mountpoint (in newer kernels)
+ // will cause all other instances of this mount in other mount namespaces
+ // to be killed (this is an anti-DoS measure that is necessary for things
+ // like devicemapper). This is necessary to avoid cases where a libdm mount
+ // that is present in another namespace will cause subsequent RemoveDevice
+ // operations to fail. We ignore any errors here because this may fail on
+ // older kernels which don't have
+ // torvalds/linux@8ed936b5671bfb33d89bc60bdcc7cf0470ba52fe applied.
+ if err := os.Remove(mountPath); err != nil {
+ logrus.Debugf("devmapper: error doing a remove on unmounted device %s: %v", mountPath, err)
+ }
+
return devices.deactivateDevice(info)
}
diff --git a/daemon/graphdriver/devmapper/devmapper_test.go b/daemon/graphdriver/devmapper/devmapper_test.go
index 7501397..24e5358 100644
--- a/daemon/graphdriver/devmapper/devmapper_test.go
+++ b/daemon/graphdriver/devmapper/devmapper_test.go
@@ -5,12 +5,15 @@
import (
"fmt"
"os"
+ "os/exec"
"syscall"
"testing"
"time"
"github.com/docker/docker/daemon/graphdriver"
"github.com/docker/docker/daemon/graphdriver/graphtest"
+ "github.com/docker/docker/pkg/parsers/kernel"
+ "golang.org/x/sys/unix"
)
func init() {
@@ -150,3 +153,53 @@
case <-doneChan:
}
}
+
+// Ensure that mounts aren't leakedriver. It's non-trivial for us to test the full
+// reproducer of #34573 in a unit test, but we can at least make sure that a
+// simple command run in a new namespace doesn't break things horribly.
+func TestDevmapperMountLeaks(t *testing.T) {
+ if !kernel.CheckKernelVersion(3, 18, 0) {
+ t.Skipf("kernel version <3.18.0 and so is missing torvalds/linux@8ed936b5671bfb33d89bc60bdcc7cf0470ba52fe.")
+ }
+
+ driver := graphtest.GetDriver(t, "devicemapper", "dm.use_deferred_removal=false", "dm.use_deferred_deletion=false").(*graphtest.Driver).Driver.(*graphdriver.NaiveDiffDriver).ProtoDriver.(*Driver)
+ defer graphtest.PutDriver(t)
+
+ // We need to create a new (dummy) device.
+ if err := driver.Create("some-layer", "", nil); err != nil {
+ t.Fatalf("setting up some-layer: %v", err)
+ }
+
+ // Mount the device.
+ _, err := driver.Get("some-layer", "")
+ if err != nil {
+ t.Fatalf("mounting some-layer: %v", err)
+ }
+
+ // Create a new subprocess which will inherit our mountpoint, then
+ // intentionally leak it and stick around. We can't do this entirely within
+ // Go because forking and namespaces in Go are really not handled well at
+ // all.
+ cmd := exec.Cmd{
+ Path: "/bin/sh",
+ Args: []string{
+ "/bin/sh", "-c",
+ "mount --make-rprivate / && sleep 1000s",
+ },
+ SysProcAttr: &syscall.SysProcAttr{
+ Unshareflags: syscall.CLONE_NEWNS,
+ },
+ }
+ if err := cmd.Start(); err != nil {
+ t.Fatalf("starting sub-command: %v", err)
+ }
+ defer func() {
+ unix.Kill(cmd.Process.Pid, unix.SIGKILL)
+ cmd.Wait()
+ }()
+
+ // Now try to "drop" the device.
+ if err := driver.Put("some-layer"); err != nil {
+ t.Fatalf("unmounting some-layer: %v", err)
+ }
+}
diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go
index b485096..bdb2336 100644
--- a/daemon/graphdriver/devmapper/driver.go
+++ b/daemon/graphdriver/devmapper/driver.go
@@ -232,10 +232,12 @@
if count := d.ctr.Decrement(mp); count > 0 {
return nil
}
+
err := d.DeviceSet.UnmountDevice(id, mp)
if err != nil {
- logrus.Errorf("devmapper: Error unmounting device %s: %s", id, err)
+ logrus.Errorf("devmapper: Error unmounting device %s: %v", id, err)
}
+
return err
}
