1. docker
  2. Commit 818d55c34bed99b108e6102ef6aa6ef181567ce3
Browse code

Fix `/proc/<pid>/oom_score_adj: invalid argument` error caused by empty env name

This fix is part of the fix for issue 25099. In 25099, if an env
has a empty name, then `docker run` will throw out an error:
```
ubuntu@ubuntu:~/docker$ docker run -e =A busybox true
docker: Error response from daemon: invalid header field value "oci runtime error:
container_linux.go:247: starting container process caused \"process_linux.go:295:
setting oom score for ready process caused \\\"write /proc/83582/oom_score_adj:
invalid argument\\\"\"\n".
```

This fix validates the Env in the container spec before it is sent
to containerd/runc.

Integration tests have been created to cover the changes.

This fix is part of fix for 25099 (not complete yet, non-utf case
may require a fix in `runc`).
This fix is related to 25300.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Yong Tang authored on 2016/11/06 03:53:54
Showing 4 changed files
daemon/container.go
History View file @ 818d55c
... ... 
@@ -15,6 +15,7 @@ import (
15 15 
 	"github.com/docker/docker/pkg/signal"
16 16 
 	"github.com/docker/docker/pkg/system"
17 17 
 	"github.com/docker/docker/pkg/truncindex"
18 
+	"github.com/docker/docker/runconfig/opts"
18 19 
 	"github.com/docker/go-connections/nat"
19 20 
 )
20 21
... ... 
@@ -232,6 +233,13 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
232 232 
 				return nil, fmt.Errorf("invalid hostname format: %s", config.Hostname)
233 233 
 			}
234 234 
 		}
235 
+
236 
+		// Validate if Env contains empty variable or not (e.g., ``, `=foo`)
237 
+		for _, env := range config.Env {
238 
+			if _, err := opts.ValidateEnv(env); err != nil {
239 
+				return nil, err
240 
+			}
241 
+		}
235 242 
 	}
236 243
237 244 
 	if hostConfig == nil {
integration-cli/docker_api_create_test.go
History View file @ 818d55c
... ... 
@@ -42,3 +42,43 @@ func (s *DockerSuite) TestAPICreateWithNotExistImage(c *check.C) {
42 42 
 	c.Assert(getErrorMessage(c, body), checker.Equals, expected)
43 43
44 44 
 }
45 
+
46 
+// Test for #25099
47 
+func (s *DockerSuite) TestAPICreateEmptyEnv(c *check.C) {
48 
+	name := "test1"
49 
+	config := map[string]interface{}{
50 
+		"Image": "busybox",
51 
+		"Env":   []string{"", "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
52 
+		"Cmd":   []string{"true"},
53 
+	}
54 
+
55 
+	status, body, err := sockRequest("POST", "/containers/create?name="+name, config)
56 
+	c.Assert(err, check.IsNil)
57 
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
58 
+	expected := "invalid environment variable:"
59 
+	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
60 
+
61 
+	name = "test2"
62 
+	config = map[string]interface{}{
63 
+		"Image": "busybox",
64 
+		"Env":   []string{"=", "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
65 
+		"Cmd":   []string{"true"},
66 
+	}
67 
+	status, body, err = sockRequest("POST", "/containers/create?name="+name, config)
68 
+	c.Assert(err, check.IsNil)
69 
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
70 
+	expected = "invalid environment variable: ="
71 
+	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
72 
+
73 
+	name = "test3"
74 
+	config = map[string]interface{}{
75 
+		"Image": "busybox",
76 
+		"Env":   []string{"=foo", "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
77 
+		"Cmd":   []string{"true"},
78 
+	}
79 
+	status, body, err = sockRequest("POST", "/containers/create?name="+name, config)
80 
+	c.Assert(err, check.IsNil)
81 
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
82 
+	expected = "invalid environment variable: =foo"
83 
+	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
84 
+}
integration-cli/docker_cli_run_test.go
History View file @ 818d55c
... ... 
@@ -4830,3 +4830,22 @@ func (s *DockerSuite) TestRunHypervIsolationWithCPUCountCPUSharesAndCPUPercent(c
4830 4830 
 	out = inspectField(c, "test", "HostConfig.CPUPercent")
4831 4831 
 	c.Assert(out, check.Equals, "80")
4832 4832 
 }
4833 
+
4834 
+// Test for #25099
4835 
+func (s *DockerSuite) TestRunEmptyEnv(c *check.C) {
4836 
+	testRequires(c, DaemonIsLinux)
4837 
+
4838 
+	expectedOutput := "invalid environment variable:"
4839 
+
4840 
+	out, _, err := dockerCmdWithError("run", "-e", "", "busybox", "true")
4841 
+	c.Assert(err, checker.NotNil)
4842 
+	c.Assert(out, checker.Contains, expectedOutput)
4843 
+
4844 
+	out, _, err = dockerCmdWithError("run", "-e", "=", "busybox", "true")
4845 
+	c.Assert(err, checker.NotNil)
4846 
+	c.Assert(out, checker.Contains, expectedOutput)
4847 
+
4848 
+	out, _, err = dockerCmdWithError("run", "-e", "=foo", "busybox", "true")
4849 
+	c.Assert(err, checker.NotNil)
4850 
+	c.Assert(out, checker.Contains, expectedOutput)
4851 
+}
runconfig/opts/opts.go
History View file @ 818d55c
... ... 
@@ -26,8 +26,13 @@ func ValidateAttach(val string) (string, error) {
26 26 
 // As on ParseEnvFile and related to #16585, environment variable names
27 27 
 // are not validate what so ever, it's up to application inside docker
28 28 
 // to validate them or not.
29 
+//
30 
+// The only validation here is to check if name is empty, per #25099
29 31 
 func ValidateEnv(val string) (string, error) {
30 32 
 	arr := strings.Split(val, "=")
33 
+	if arr[0] == "" {
34 
+		return "", fmt.Errorf("invalid environment variable: %s", val)
35 
+	}
31 36 
 	if len(arr) > 1 {
32 37 
 		return val, nil
33 38 
 	}