Browse code

Add mode check for device

This fixes two problems:
1. docker run --device /dev/sda:rw ubuntu bash doesn't work
2. --device /dev/zero:/dev/noro:ro doesn't show clear error message,
but fail when writing to cgroup file.

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

Qiang Huang authored on 2015/08/24 18:57:12
Showing 5 changed files
... ...
@@ -858,6 +858,20 @@ func (s *DockerSuite) TestRunAddingOptionalDevices(c *check.C) {
858 858
 	}
859 859
 }
860 860
 
861
+func (s *DockerSuite) TestRunAddingOptionalDevicesNoSrc(c *check.C) {
862
+	out, _ := dockerCmd(c, "run", "--device", "/dev/zero:rw", "busybox", "sh", "-c", "ls /dev/zero")
863
+	if actual := strings.Trim(out, "\r\n"); actual != "/dev/zero" {
864
+		c.Fatalf("expected output /dev/zero, received %s", actual)
865
+	}
866
+}
867
+
868
+func (s *DockerSuite) TestRunAddingOptionalDevicesInvalideMode(c *check.C) {
869
+	_, _, err := dockerCmdWithError("run", "--device", "/dev/zero:ro", "busybox", "sh", "-c", "ls /dev/zero")
870
+	if err == nil {
871
+		c.Fatalf("run container with device mode ro should fail")
872
+	}
873
+}
874
+
861 875
 func (s *DockerSuite) TestRunModeHostname(c *check.C) {
862 876
 	testRequires(c, SameHostDaemon)
863 877
 
... ...
@@ -170,11 +170,32 @@ func ValidateLink(val string) (string, error) {
170 170
 	return val, nil
171 171
 }
172 172
 
173
+// ValidDeviceMode checks if the mode for device is valid or not.
174
+// Valid mode is a composition of r (read), w (write), and m (mknod).
175
+func ValidDeviceMode(mode string) bool {
176
+	var legalDeviceMode = map[rune]bool{
177
+		'r': true,
178
+		'w': true,
179
+		'm': true,
180
+	}
181
+	if mode == "" {
182
+		return false
183
+	}
184
+	for _, c := range mode {
185
+		if !legalDeviceMode[c] {
186
+			return false
187
+		}
188
+		legalDeviceMode[c] = false
189
+	}
190
+	return true
191
+}
192
+
173 193
 // ValidateDevice Validate a path for devices
174 194
 // It will make sure 'val' is in the form:
175 195
 //    [host-dir:]container-path[:mode]
196
+// It will also validate the device mode.
176 197
 func ValidateDevice(val string) (string, error) {
177
-	return validatePath(val, false)
198
+	return validatePath(val, ValidDeviceMode)
178 199
 }
179 200
 
180 201
 // ValidatePath Validate a path for volumes
... ...
@@ -182,27 +203,27 @@ func ValidateDevice(val string) (string, error) {
182 182
 //    [host-dir:]container-path[:rw|ro]
183 183
 // It will also validate the mount mode.
184 184
 func ValidatePath(val string) (string, error) {
185
-	return validatePath(val, true)
185
+	return validatePath(val, volume.ValidMountMode)
186 186
 }
187 187
 
188
-func validatePath(val string, validateMountMode bool) (string, error) {
188
+func validatePath(val string, validator func(string) bool) (string, error) {
189 189
 	var containerPath string
190 190
 	var mode string
191 191
 
192 192
 	if strings.Count(val, ":") > 2 {
193
-		return val, fmt.Errorf("bad format for volumes: %s", val)
193
+		return val, fmt.Errorf("bad format for path: %s", val)
194 194
 	}
195 195
 
196 196
 	splited := strings.SplitN(val, ":", 3)
197 197
 	if splited[0] == "" {
198
-		return val, fmt.Errorf("bad format for volumes: %s", val)
198
+		return val, fmt.Errorf("bad format for path: %s", val)
199 199
 	}
200 200
 	switch len(splited) {
201 201
 	case 1:
202 202
 		containerPath = splited[0]
203 203
 		val = path.Clean(containerPath)
204 204
 	case 2:
205
-		if isValid := volume.ValidMountMode(splited[1]); validateMountMode && isValid {
205
+		if isValid := validator(splited[1]); isValid {
206 206
 			containerPath = splited[0]
207 207
 			mode = splited[1]
208 208
 			val = fmt.Sprintf("%s:%s", path.Clean(containerPath), mode)
... ...
@@ -213,8 +234,8 @@ func validatePath(val string, validateMountMode bool) (string, error) {
213 213
 	case 3:
214 214
 		containerPath = splited[1]
215 215
 		mode = splited[2]
216
-		if isValid := volume.ValidMountMode(splited[2]); validateMountMode && !isValid {
217
-			return val, fmt.Errorf("bad mount mode specified : %s", mode)
216
+		if isValid := validator(splited[2]); !isValid {
217
+			return val, fmt.Errorf("bad mode specified: %s", mode)
218 218
 		}
219 219
 		val = fmt.Sprintf("%s:%s:%s", splited[0], containerPath, mode)
220 220
 	}
... ...
@@ -289,24 +289,24 @@ func TestValidatePath(t *testing.T) {
289 289
 		"/rw:rw",
290 290
 	}
291 291
 	invalid := map[string]string{
292
-		"":                "bad format for volumes: ",
292
+		"":                "bad format for path: ",
293 293
 		"./":              "./ is not an absolute path",
294 294
 		"../":             "../ is not an absolute path",
295 295
 		"/:../":           "../ is not an absolute path",
296 296
 		"/:path":          "path is not an absolute path",
297
-		":":               "bad format for volumes: :",
297
+		":":               "bad format for path: :",
298 298
 		"/tmp:":           " is not an absolute path",
299
-		":test":           "bad format for volumes: :test",
300
-		":/test":          "bad format for volumes: :/test",
299
+		":test":           "bad format for path: :test",
300
+		":/test":          "bad format for path: :/test",
301 301
 		"tmp:":            " is not an absolute path",
302
-		":test:":          "bad format for volumes: :test:",
303
-		"::":              "bad format for volumes: ::",
304
-		":::":             "bad format for volumes: :::",
305
-		"/tmp:::":         "bad format for volumes: /tmp:::",
306
-		":/tmp::":         "bad format for volumes: :/tmp::",
302
+		":test:":          "bad format for path: :test:",
303
+		"::":              "bad format for path: ::",
304
+		":::":             "bad format for path: :::",
305
+		"/tmp:::":         "bad format for path: /tmp:::",
306
+		":/tmp::":         "bad format for path: :/tmp::",
307 307
 		"path:ro":         "path is not an absolute path",
308
-		"/path:/path:sw":  "bad mount mode specified : sw",
309
-		"/path:/path:rwz": "bad mount mode specified : rwz",
308
+		"/path:/path:sw":  "bad mode specified: sw",
309
+		"/path:/path:rwz": "bad mode specified: rwz",
310 310
 	}
311 311
 
312 312
 	for _, path := range valid {
... ...
@@ -333,27 +333,30 @@ func TestValidateDevice(t *testing.T) {
333 333
 		"/with space",
334 334
 		"/home:/with space",
335 335
 		"relative:/absolute-path",
336
-		"hostPath:/containerPath:ro",
336
+		"hostPath:/containerPath:r",
337 337
 		"/hostPath:/containerPath:rw",
338 338
 		"/hostPath:/containerPath:mrw",
339 339
 	}
340 340
 	invalid := map[string]string{
341
-		"":        "bad format for volumes: ",
341
+		"":        "bad format for path: ",
342 342
 		"./":      "./ is not an absolute path",
343 343
 		"../":     "../ is not an absolute path",
344 344
 		"/:../":   "../ is not an absolute path",
345 345
 		"/:path":  "path is not an absolute path",
346
-		":":       "bad format for volumes: :",
346
+		":":       "bad format for path: :",
347 347
 		"/tmp:":   " is not an absolute path",
348
-		":test":   "bad format for volumes: :test",
349
-		":/test":  "bad format for volumes: :/test",
348
+		":test":   "bad format for path: :test",
349
+		":/test":  "bad format for path: :/test",
350 350
 		"tmp:":    " is not an absolute path",
351
-		":test:":  "bad format for volumes: :test:",
352
-		"::":      "bad format for volumes: ::",
353
-		":::":     "bad format for volumes: :::",
354
-		"/tmp:::": "bad format for volumes: /tmp:::",
355
-		":/tmp::": "bad format for volumes: :/tmp::",
351
+		":test:":  "bad format for path: :test:",
352
+		"::":      "bad format for path: ::",
353
+		":::":     "bad format for path: :::",
354
+		"/tmp:::": "bad format for path: /tmp:::",
355
+		":/tmp::": "bad format for path: :/tmp::",
356 356
 		"path:ro": "ro is not an absolute path",
357
+		"path:rr": "rr is not an absolute path",
358
+		"a:/b:ro": "bad mode specified: ro",
359
+		"a:/b:rr": "bad mode specified: rr",
357 360
 	}
358 361
 
359 362
 	for _, path := range valid {
... ...
@@ -474,7 +474,11 @@ func ParseDevice(device string) (DeviceMapping, error) {
474 474
 		permissions = arr[2]
475 475
 		fallthrough
476 476
 	case 2:
477
-		dst = arr[1]
477
+		if opts.ValidDeviceMode(arr[1]) {
478
+			permissions = arr[1]
479
+		} else {
480
+			dst = arr[1]
481
+		}
478 482
 		fallthrough
479 483
 	case 1:
480 484
 		src = arr[0]
... ...
@@ -317,15 +317,20 @@ func TestParseDevice(t *testing.T) {
317 317
 			PathInContainer:   "/dev/snd",
318 318
 			CgroupPermissions: "rwm",
319 319
 		},
320
+		"/dev/snd:rw": {
321
+			PathOnHost:        "/dev/snd",
322
+			PathInContainer:   "/dev/snd",
323
+			CgroupPermissions: "rw",
324
+		},
320 325
 		"/dev/snd:/something": {
321 326
 			PathOnHost:        "/dev/snd",
322 327
 			PathInContainer:   "/something",
323 328
 			CgroupPermissions: "rwm",
324 329
 		},
325
-		"/dev/snd:/something:ro": {
330
+		"/dev/snd:/something:rw": {
326 331
 			PathOnHost:        "/dev/snd",
327 332
 			PathInContainer:   "/something",
328
-			CgroupPermissions: "ro",
333
+			CgroupPermissions: "rw",
329 334
 		},
330 335
 	}
331 336
 	for device, deviceMapping := range valids {