Without this patch, containers inherit the open file descriptors of the daemon, so my "exec 42>&2" allows us to "echo >&42 some nasty error with some bad advice" directly into the daemon log. :)
Also, "hack/dind" was already doing this due to issues caused by the inheritance, so I'm removing that hack too since this patch obsoletes it by generalizing it for all containers.
Docker-DCO-1.1-Signed-off-by: Andrew Page <admwiggin@gmail.com> (github: tianon)
| ... | ... |
@@ -5,6 +5,7 @@ import ( |
| 5 | 5 |
"github.com/dotcloud/docker/daemon/execdriver" |
| 6 | 6 |
"github.com/dotcloud/docker/pkg/cgroups" |
| 7 | 7 |
"github.com/dotcloud/docker/pkg/label" |
| 8 |
+ "github.com/dotcloud/docker/pkg/system" |
|
| 8 | 9 |
"github.com/dotcloud/docker/utils" |
| 9 | 10 |
"io/ioutil" |
| 10 | 11 |
"log" |
| ... | ... |
@@ -42,6 +43,10 @@ func init() {
|
| 42 | 42 |
return err |
| 43 | 43 |
} |
| 44 | 44 |
|
| 45 |
+ if err := system.CloseFdsFrom(3); err != nil {
|
|
| 46 |
+ return err |
|
| 47 |
+ } |
|
| 48 |
+ |
|
| 45 | 49 |
if err := changeUser(args); err != nil {
|
| 46 | 50 |
return err |
| 47 | 51 |
} |
| ... | ... |
@@ -70,22 +70,6 @@ grep -q :devices: /proc/1/cgroup || |
| 70 | 70 |
grep -qw devices /proc/1/cgroup || |
| 71 | 71 |
echo "WARNING: it looks like the 'devices' cgroup is not mounted." |
| 72 | 72 |
|
| 73 |
-# Now, close extraneous file descriptors. |
|
| 74 |
-pushd /proc/self/fd >/dev/null |
|
| 75 |
-for FD in * |
|
| 76 |
-do |
|
| 77 |
- case "$FD" in |
|
| 78 |
- # Keep stdin/stdout/stderr |
|
| 79 |
- [012]) |
|
| 80 |
- ;; |
|
| 81 |
- # Nuke everything else |
|
| 82 |
- *) |
|
| 83 |
- eval exec "$FD>&-" |
|
| 84 |
- ;; |
|
| 85 |
- esac |
|
| 86 |
-done |
|
| 87 |
-popd >/dev/null |
|
| 88 |
- |
|
| 89 | 73 |
# Mount /tmp |
| 90 | 74 |
mount -t tmpfs none /tmp |
| 91 | 75 |
|
| ... | ... |
@@ -19,6 +19,9 @@ bundle_test_integration_cli() {
|
| 19 | 19 |
false |
| 20 | 20 |
fi |
| 21 | 21 |
|
| 22 |
+ # intentionally open a couple bogus file descriptors to help test that they get scrubbed in containers |
|
| 23 |
+ exec 41>&1 42>&2 |
|
| 24 |
+ |
|
| 22 | 25 |
( set -x; exec \ |
| 23 | 26 |
docker --daemon --debug \ |
| 24 | 27 |
--storage-driver "$DOCKER_GRAPHDRIVER" \ |
| ... | ... |
@@ -91,6 +91,22 @@ func TestDockerRunEchoNamedContainer(t *testing.T) {
|
| 91 | 91 |
logDone("run - echo with named container")
|
| 92 | 92 |
} |
| 93 | 93 |
|
| 94 |
+// docker run should not leak file descriptors |
|
| 95 |
+func TestDockerRunLeakyFileDescriptors(t *testing.T) {
|
|
| 96 |
+ runCmd := exec.Command(dockerBinary, "run", "busybox", "ls", "-C", "/proc/self/fd") |
|
| 97 |
+ out, _, _, err := runCommandWithStdoutStderr(runCmd) |
|
| 98 |
+ errorOut(err, t, out) |
|
| 99 |
+ |
|
| 100 |
+ // normally, we should only get 0, 1, and 2, but 3 gets created by "ls" when it does "opendir" on the "fd" directory |
|
| 101 |
+ if out != "0 1 2 3\n" {
|
|
| 102 |
+ t.Errorf("container should've printed '0 1 2 3', not: %s", out)
|
|
| 103 |
+ } |
|
| 104 |
+ |
|
| 105 |
+ deleteAllContainers() |
|
| 106 |
+ |
|
| 107 |
+ logDone("run - check file descriptor leakage")
|
|
| 108 |
+} |
|
| 109 |
+ |
|
| 94 | 110 |
// it should be possible to ping Google DNS resolver |
| 95 | 111 |
// this will fail when Internet access is unavailable |
| 96 | 112 |
func TestDockerRunPingGoogle(t *testing.T) {
|
| ... | ... |
@@ -130,12 +130,16 @@ func setupNetwork(container *libcontainer.Container, context libcontainer.Contex |
| 130 | 130 |
return nil |
| 131 | 131 |
} |
| 132 | 132 |
|
| 133 |
-// finalizeNamespace drops the caps and sets the correct user |
|
| 134 |
-// and working dir before execing the command inside the namespace |
|
| 133 |
+// finalizeNamespace drops the caps, sets the correct user |
|
| 134 |
+// and working dir, and closes any leaky file descriptors |
|
| 135 |
+// before execing the command inside the namespace |
|
| 135 | 136 |
func finalizeNamespace(container *libcontainer.Container) error {
|
| 136 | 137 |
if err := capabilities.DropCapabilities(container); err != nil {
|
| 137 | 138 |
return fmt.Errorf("drop capabilities %s", err)
|
| 138 | 139 |
} |
| 140 |
+ if err := system.CloseFdsFrom(3); err != nil {
|
|
| 141 |
+ return fmt.Errorf("close open file descriptors %s", err)
|
|
| 142 |
+ } |
|
| 139 | 143 |
if err := setupUser(container); err != nil {
|
| 140 | 144 |
return fmt.Errorf("setup user %s", err)
|
| 141 | 145 |
} |
| 142 | 146 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,38 @@ |
| 0 |
+package system |
|
| 1 |
+ |
|
| 2 |
+import ( |
|
| 3 |
+ "io/ioutil" |
|
| 4 |
+ "strconv" |
|
| 5 |
+ "syscall" |
|
| 6 |
+) |
|
| 7 |
+ |
|
| 8 |
+// Works similarly to OpenBSD's "closefrom(2)": |
|
| 9 |
+// The closefrom() call deletes all descriptors numbered fd and higher from |
|
| 10 |
+// the per-process file descriptor table. It is effectively the same as |
|
| 11 |
+// calling close(2) on each descriptor. |
|
| 12 |
+// http://www.openbsd.org/cgi-bin/man.cgi?query=closefrom&sektion=2 |
|
| 13 |
+// |
|
| 14 |
+// See also http://stackoverflow.com/a/918469/433558 |
|
| 15 |
+func CloseFdsFrom(minFd int) error {
|
|
| 16 |
+ fdList, err := ioutil.ReadDir("/proc/self/fd")
|
|
| 17 |
+ if err != nil {
|
|
| 18 |
+ return err |
|
| 19 |
+ } |
|
| 20 |
+ for _, fi := range fdList {
|
|
| 21 |
+ fd, err := strconv.Atoi(fi.Name()) |
|
| 22 |
+ if err != nil {
|
|
| 23 |
+ // ignore non-numeric file names |
|
| 24 |
+ continue |
|
| 25 |
+ } |
|
| 26 |
+ |
|
| 27 |
+ if fd < minFd {
|
|
| 28 |
+ // ignore descriptors lower than our specified minimum |
|
| 29 |
+ continue |
|
| 30 |
+ } |
|
| 31 |
+ |
|
| 32 |
+ // intentionally ignore errors from syscall.Close |
|
| 33 |
+ syscall.Close(fd) |
|
| 34 |
+ // the cases where this might fail are basically file descriptors that have already been closed (including and especially the one that was created when ioutil.ReadDir did the "opendir" syscall) |
|
| 35 |
+ } |
|
| 36 |
+ return nil |
|
| 37 |
+} |