Merge pull request #35427 from sjeeva/master
fixed special character
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
}
diff --git a/integration/image/import_test.go b/integration/image/import_test.go
new file mode 100644
index 0000000..955891f
--- /dev/null
+++ b/integration/image/import_test.go
@@ -0,0 +1,36 @@
+package image
+
+import (
+ "archive/tar"
+ "bytes"
+ "context"
+ "io"
+ "testing"
+
+ "github.com/docker/docker/api/types"
+ "github.com/docker/docker/integration/util/request"
+ "github.com/docker/docker/internal/testutil"
+)
+
+// Ensure we don't regress on CVE-2017-14992.
+func TestImportExtremelyLargeImageWorks(t *testing.T) {
+ client := request.NewAPIClient(t)
+
+ // Construct an empty tar archive with about 8GB of junk padding at the
+ // end. This should not cause any crashes (the padding should be mostly
+ // ignored).
+ var tarBuffer bytes.Buffer
+ tw := tar.NewWriter(&tarBuffer)
+ if err := tw.Close(); err != nil {
+ t.Fatal(err)
+ }
+ imageRdr := io.MultiReader(&tarBuffer, io.LimitReader(testutil.DevZero, 8*1024*1024*1024))
+
+ _, err := client.ImageImport(context.Background(),
+ types.ImageImportSource{Source: imageRdr, SourceName: "-"},
+ "test1234:v42",
+ types.ImageImportOptions{})
+ if err != nil {
+ t.Fatal(err)
+ }
+}
diff --git a/internal/testutil/helpers.go b/internal/testutil/helpers.go
index a760569..287b3cb 100644
--- a/internal/testutil/helpers.go
+++ b/internal/testutil/helpers.go
@@ -1,6 +1,8 @@
package testutil
import (
+ "io"
+
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -11,3 +13,15 @@
require.Error(t, err, msgAndArgs...)
assert.Contains(t, err.Error(), expectedError, msgAndArgs...)
}
+
+// DevZero acts like /dev/zero but in an OS-independent fashion.
+var DevZero io.Reader = devZero{}
+
+type devZero struct{}
+
+func (d devZero) Read(p []byte) (n int, err error) {
+ for i := 0; i < len(p); i++ {
+ p[i] = '\x00'
+ }
+ return len(p), nil
+}
diff --git a/vendor.conf b/vendor.conf
index bec25b7..dc94fb4 100644
--- a/vendor.conf
+++ b/vendor.conf
@@ -55,7 +55,7 @@
# get graph and distribution packages
github.com/docker/distribution edc3ab29cdff8694dd6feb85cfeb4b5f1b38ed9c
-github.com/vbatts/tar-split v0.10.1
+github.com/vbatts/tar-split v0.10.2
github.com/opencontainers/go-digest a6d0ee40d4207ea02364bd3b9e8e77b9159ba1eb
# get go-zfs packages
@@ -67,7 +67,7 @@
# When updating, also update RUNC_COMMIT in hack/dockerfile/binaries-commits accordingly
github.com/opencontainers/runc 0351df1c5a66838d0c392b4ac4cf9450de844e2d
github.com/opencontainers/runtime-spec v1.0.0
-github.com/opencontainers/image-spec 372ad780f63454fbbbbcc7cf80e5b90245c13e13
+github.com/opencontainers/image-spec v1.0.0
github.com/seccomp/libseccomp-golang 32f571b70023028bd57d9288c20efbcb237f3ce0
# libcontainer deps (see src/github.com/opencontainers/runc/Godeps/Godeps.json)
diff --git a/vendor/github.com/opencontainers/image-spec/specs-go/v1/config.go b/vendor/github.com/opencontainers/image-spec/specs-go/v1/config.go
index 8475ff7..fe799bd 100644
--- a/vendor/github.com/opencontainers/image-spec/specs-go/v1/config.go
+++ b/vendor/github.com/opencontainers/image-spec/specs-go/v1/config.go
@@ -37,7 +37,7 @@
// Cmd defines the default arguments to the entrypoint of the container.
Cmd []string `json:"Cmd,omitempty"`
- // Volumes is a set of directories which should be created as data volumes in a container running this image.
+ // Volumes is a set of directories describing where the process is likely write data specific to a container instance.
Volumes map[string]struct{} `json:"Volumes,omitempty"`
// WorkingDir sets the current working directory of the entrypoint process in the container.
diff --git a/vendor/github.com/opencontainers/image-spec/specs-go/version.go b/vendor/github.com/opencontainers/image-spec/specs-go/version.go
index f4cda6e..e3eee29 100644
--- a/vendor/github.com/opencontainers/image-spec/specs-go/version.go
+++ b/vendor/github.com/opencontainers/image-spec/specs-go/version.go
@@ -25,7 +25,7 @@
VersionPatch = 0
// VersionDev indicates development branch. Releases will be empty string.
- VersionDev = "-rc6-dev"
+ VersionDev = ""
)
// Version is the specification version that the package types support.
diff --git a/vendor/github.com/vbatts/tar-split/README.md b/vendor/github.com/vbatts/tar-split/README.md
index 4c544d8..03e3ec4 100644
--- a/vendor/github.com/vbatts/tar-split/README.md
+++ b/vendor/github.com/vbatts/tar-split/README.md
@@ -1,6 +1,7 @@
# tar-split
[![Build Status](https://travis-ci.org/vbatts/tar-split.svg?branch=master)](https://travis-ci.org/vbatts/tar-split)
+[![Go Report Card](https://goreportcard.com/badge/github.com/vbatts/tar-split)](https://goreportcard.com/report/github.com/vbatts/tar-split)
Pristinely disassembling a tar archive, and stashing needed raw bytes and offsets to reassemble a validating original archive.
@@ -50,7 +51,7 @@
contiguous file, though the archive contents may be recorded in sparse format.
Therefore when adding the file payload to a reassembled tar, to achieve
identical output, the file payload would need be precisely re-sparsified. This
-is not something I seek to fix imediately, but would rather have an alert that
+is not something I seek to fix immediately, but would rather have an alert that
precise reassembly is not possible.
(see more http://www.gnu.org/software/tar/manual/html_node/Sparse-Formats.html)
diff --git a/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go b/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go
index 54ef23a..009b3f5 100644
--- a/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go
+++ b/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go
@@ -2,7 +2,6 @@
import (
"io"
- "io/ioutil"
"github.com/vbatts/tar-split/archive/tar"
"github.com/vbatts/tar-split/tar/storage"
@@ -119,20 +118,34 @@
}
}
- // it is allowable, and not uncommon that there is further padding on the
- // end of an archive, apart from the expected 1024 null bytes.
- remainder, err := ioutil.ReadAll(outputRdr)
- if err != nil && err != io.EOF {
- pW.CloseWithError(err)
- return
- }
- _, err = p.AddEntry(storage.Entry{
- Type: storage.SegmentType,
- Payload: remainder,
- })
- if err != nil {
- pW.CloseWithError(err)
- return
+ // It is allowable, and not uncommon that there is further padding on
+ // the end of an archive, apart from the expected 1024 null bytes. We
+ // do this in chunks rather than in one go to avoid cases where a
+ // maliciously crafted tar file tries to trick us into reading many GBs
+ // into memory.
+ const paddingChunkSize = 1024 * 1024
+ var paddingChunk [paddingChunkSize]byte
+ for {
+ var isEOF bool
+ n, err := outputRdr.Read(paddingChunk[:])
+ if err != nil {
+ if err != io.EOF {
+ pW.CloseWithError(err)
+ return
+ }
+ isEOF = true
+ }
+ _, err = p.AddEntry(storage.Entry{
+ Type: storage.SegmentType,
+ Payload: paddingChunk[:n],
+ })
+ if err != nil {
+ pW.CloseWithError(err)
+ return
+ }
+ if isEOF {
+ break
+ }
}
pW.Close()
}()