diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index 6f6f4b48ba..80ddcf096d 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -71,11 +71,10 @@ func parseBindMount(spec string, mountLabel string, config *runconfig.Config) (* case 3: bind.Destination = arr[1] mode := arr[2] - isValid, isRw := volume.ValidateMountMode(mode) - if !isValid { + if !volume.ValidMountMode(mode) { return nil, fmt.Errorf("invalid mode for volumes-from: %s", mode) } - bind.RW = isRw + bind.RW = volume.ReadWrite(mode) // Mode field is used by SELinux to decide whether to apply label bind.Mode = mode default: @@ -268,7 +267,7 @@ func parseVolumesFrom(spec string) (string, string, error) { if len(specParts) == 2 { mode = specParts[1] - if isValid, _ := volume.ValidateMountMode(mode); !isValid { + if !volume.ValidMountMode(mode) { return "", "", fmt.Errorf("invalid mode for volumes-from: %s", mode) } } diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 5edbb5218b..d8ac9d3e95 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -785,6 +785,20 @@ func (s *DockerSuite) TestRunAddingOptionalDevices(c *check.C) { } } +func (s *DockerSuite) TestRunAddingOptionalDevicesNoSrc(c *check.C) { + out, _ := dockerCmd(c, "run", "--device", "/dev/zero:rw", "busybox", "sh", "-c", "ls /dev/zero") + if actual := strings.Trim(out, "\r\n"); actual != "/dev/zero" { + c.Fatalf("expected output /dev/zero, received %s", actual) + } +} + +func (s *DockerSuite) TestRunAddingOptionalDevicesInvalideMode(c *check.C) { + _, _, err := dockerCmdWithError("run", "--device", "/dev/zero:ro", "busybox", "sh", "-c", "ls /dev/zero") + if err == nil { + c.Fatalf("run container with device mode ro should fail") + } +} + func (s *DockerSuite) TestRunModeHostname(c *check.C) { testRequires(c, SameHostDaemon) diff --git a/opts/opts.go b/opts/opts.go index 398b8b4f37..aa3de81a4d 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -170,11 +170,32 @@ func ValidateLink(val string) (string, error) { return val, nil } +// ValidDeviceMode checks if the mode for device is valid or not. +// Valid mode is a composition of r (read), w (write), and m (mknod). +func ValidDeviceMode(mode string) bool { + var legalDeviceMode = map[rune]bool{ + 'r': true, + 'w': true, + 'm': true, + } + if mode == "" { + return false + } + for _, c := range mode { + if !legalDeviceMode[c] { + return false + } + legalDeviceMode[c] = false + } + return true +} + // ValidateDevice Validate a path for devices // It will make sure 'val' is in the form: // [host-dir:]container-path[:mode] +// It will also validate the device mode. func ValidateDevice(val string) (string, error) { - return validatePath(val, false) + return validatePath(val, ValidDeviceMode) } // ValidatePath Validate a path for volumes @@ -182,27 +203,27 @@ func ValidateDevice(val string) (string, error) { // [host-dir:]container-path[:rw|ro] // It will also validate the mount mode. func ValidatePath(val string) (string, error) { - return validatePath(val, true) + return validatePath(val, volume.ValidMountMode) } -func validatePath(val string, validateMountMode bool) (string, error) { +func validatePath(val string, validator func(string) bool) (string, error) { var containerPath string var mode string if strings.Count(val, ":") > 2 { - return val, fmt.Errorf("bad format for volumes: %s", val) + return val, fmt.Errorf("bad format for path: %s", val) } splited := strings.SplitN(val, ":", 3) if splited[0] == "" { - return val, fmt.Errorf("bad format for volumes: %s", val) + return val, fmt.Errorf("bad format for path: %s", val) } switch len(splited) { case 1: containerPath = splited[0] val = path.Clean(containerPath) case 2: - if isValid, _ := volume.ValidateMountMode(splited[1]); validateMountMode && isValid { + if isValid := validator(splited[1]); isValid { containerPath = splited[0] mode = splited[1] val = fmt.Sprintf("%s:%s", path.Clean(containerPath), mode) @@ -213,8 +234,8 @@ func validatePath(val string, validateMountMode bool) (string, error) { case 3: containerPath = splited[1] mode = splited[2] - if isValid, _ := volume.ValidateMountMode(splited[2]); validateMountMode && !isValid { - return val, fmt.Errorf("bad mount mode specified : %s", mode) + if isValid := validator(splited[2]); !isValid { + return val, fmt.Errorf("bad mode specified: %s", mode) } val = fmt.Sprintf("%s:%s:%s", splited[0], containerPath, mode) } diff --git a/opts/opts_test.go b/opts/opts_test.go index f08df30be6..e3ff970b32 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -289,24 +289,24 @@ func TestValidatePath(t *testing.T) { "/rw:rw", } invalid := map[string]string{ - "": "bad format for volumes: ", + "": "bad format for path: ", "./": "./ is not an absolute path", "../": "../ is not an absolute path", "/:../": "../ is not an absolute path", "/:path": "path is not an absolute path", - ":": "bad format for volumes: :", + ":": "bad format for path: :", "/tmp:": " is not an absolute path", - ":test": "bad format for volumes: :test", - ":/test": "bad format for volumes: :/test", + ":test": "bad format for path: :test", + ":/test": "bad format for path: :/test", "tmp:": " is not an absolute path", - ":test:": "bad format for volumes: :test:", - "::": "bad format for volumes: ::", - ":::": "bad format for volumes: :::", - "/tmp:::": "bad format for volumes: /tmp:::", - ":/tmp::": "bad format for volumes: :/tmp::", + ":test:": "bad format for path: :test:", + "::": "bad format for path: ::", + ":::": "bad format for path: :::", + "/tmp:::": "bad format for path: /tmp:::", + ":/tmp::": "bad format for path: :/tmp::", "path:ro": "path is not an absolute path", - "/path:/path:sw": "bad mount mode specified : sw", - "/path:/path:rwz": "bad mount mode specified : rwz", + "/path:/path:sw": "bad mode specified: sw", + "/path:/path:rwz": "bad mode specified: rwz", } for _, path := range valid { @@ -333,27 +333,30 @@ func TestValidateDevice(t *testing.T) { "/with space", "/home:/with space", "relative:/absolute-path", - "hostPath:/containerPath:ro", + "hostPath:/containerPath:r", "/hostPath:/containerPath:rw", "/hostPath:/containerPath:mrw", } invalid := map[string]string{ - "": "bad format for volumes: ", + "": "bad format for path: ", "./": "./ is not an absolute path", "../": "../ is not an absolute path", "/:../": "../ is not an absolute path", "/:path": "path is not an absolute path", - ":": "bad format for volumes: :", + ":": "bad format for path: :", "/tmp:": " is not an absolute path", - ":test": "bad format for volumes: :test", - ":/test": "bad format for volumes: :/test", + ":test": "bad format for path: :test", + ":/test": "bad format for path: :/test", "tmp:": " is not an absolute path", - ":test:": "bad format for volumes: :test:", - "::": "bad format for volumes: ::", - ":::": "bad format for volumes: :::", - "/tmp:::": "bad format for volumes: /tmp:::", - ":/tmp::": "bad format for volumes: :/tmp::", + ":test:": "bad format for path: :test:", + "::": "bad format for path: ::", + ":::": "bad format for path: :::", + "/tmp:::": "bad format for path: /tmp:::", + ":/tmp::": "bad format for path: :/tmp::", "path:ro": "ro is not an absolute path", + "path:rr": "rr is not an absolute path", + "a:/b:ro": "bad mode specified: ro", + "a:/b:rr": "bad mode specified: rr", } for _, path := range valid { diff --git a/runconfig/parse.go b/runconfig/parse.go index 14aff6bb0c..1cb73f1a11 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -474,7 +474,11 @@ func ParseDevice(device string) (DeviceMapping, error) { permissions = arr[2] fallthrough case 2: - dst = arr[1] + if opts.ValidDeviceMode(arr[1]) { + permissions = arr[1] + } else { + dst = arr[1] + } fallthrough case 1: src = arr[0] diff --git a/runconfig/parse_test.go b/runconfig/parse_test.go index 2acefbfb3d..8fe4ab421e 100644 --- a/runconfig/parse_test.go +++ b/runconfig/parse_test.go @@ -317,15 +317,20 @@ func TestParseDevice(t *testing.T) { PathInContainer: "/dev/snd", CgroupPermissions: "rwm", }, + "/dev/snd:rw": { + PathOnHost: "/dev/snd", + PathInContainer: "/dev/snd", + CgroupPermissions: "rw", + }, "/dev/snd:/something": { PathOnHost: "/dev/snd", PathInContainer: "/something", CgroupPermissions: "rwm", }, - "/dev/snd:/something:ro": { + "/dev/snd:/something:rw": { PathOnHost: "/dev/snd", PathInContainer: "/something", - CgroupPermissions: "ro", + CgroupPermissions: "rw", }, } for device, deviceMapping := range valids { diff --git a/volume/volume.go b/volume/volume.go index 19c9d77ad9..cfc2207b99 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -49,13 +49,13 @@ var roModes = map[string]bool{ "Z,ro": true, } -// ValidateMountMode will make sure the mount mode is valid. -// returns if it's a valid mount mode and if it's read-write or not. -func ValidateMountMode(mode string) (bool, bool) { - return roModes[mode] || rwModes[mode], rwModes[mode] +// ValidMountMode will make sure the mount mode is valid. +// returns if it's a valid mount mode or not. +func ValidMountMode(mode string) bool { + return roModes[mode] || rwModes[mode] } -// ReadWrite tells you if a mode string is a valid read-only mode or not. +// ReadWrite tells you if a mode string is a valid read-write mode or not. func ReadWrite(mode string) bool { return rwModes[mode] }