Merge pull request opencontainers#3182 from cyphar/revert-3159

Revert "libct/devices: change devices.Type to be a string"

Author: thaJeztah

libcontainer/cgroups/devices/devices_emulator.go

@@ -89,22 +89,24 @@ func parseLine(line string) (*deviceRule, error) {
 
 	var (
 		rule  deviceRule
-		node  = devices.Type(fields[0])
+		node  = fields[0]
 		major = fields[1]
 		minor = fields[2]
 		perms = fields[3]
 	)
 
 	// Parse the node type.
 	switch node {
-	case devices.WildcardDevice:
+	case "a":
 		// Super-special case -- "a" always means every device with every
 		// access mode. In fact, for devices.list this actually indicates that
 		// the cgroup is in black-list mode.
 		// TODO: Double-check that the entire file is "a *:* rwm".
 		return nil, nil
-	case devices.BlockDevice, devices.CharDevice:
-		rule.meta.node = node
+	case "b":
+		rule.meta.node = devices.BlockDevice
+	case "c":
+		rule.meta.node = devices.CharDevice
 	default:
 		return nil, fmt.Errorf("unknown device type %q", node)
 	}
@@ -136,7 +138,6 @@ func parseLine(line string) (*deviceRule, error) {
 	if !rule.perms.IsValid() || rule.perms.IsEmpty() {
 		return nil, fmt.Errorf("parse access mode: contained unknown modes or is empty: %q", perms)
 	}
-
 	return &rule, nil
 }
 
@@ -318,10 +319,10 @@ func (source *Emulator) Transition(target *Emulator) ([]*devices.Rule, error) {
 	// black-list we also have to include a disruptive rule.
 	if source.IsBlacklist() || source.defaultAllow != target.defaultAllow {
 		transitionRules = append(transitionRules, &devices.Rule{
-			Type:        devices.WildcardDevice,
+			Type:        'a',
 			Major:       -1,
 			Minor:       -1,
-			Permissions: "rwm",
+			Permissions: devices.Permissions("rwm"),
 			Allow:       target.defaultAllow,
 		})
 		// The old rules are only relevant if we aren't starting out with a

libcontainer/cgroups/ebpf/devicefilter/devicefilter.go

@@ -119,7 +119,7 @@ func (p *program) appendRule(rule *devices.Rule) error {
 		bpfType = int32(unix.BPF_DEVCG_DEV_BLOCK)
 	default:
 		// We do not permit 'a', nor any other types we don't know about.
-		return fmt.Errorf("invalid type %q", rule.Type)
+		return fmt.Errorf("invalid type %q", string(rule.Type))
 	}
 	if rule.Major > math.MaxUint32 {
 		return fmt.Errorf("invalid major %d", rule.Major)

libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go

@@ -146,7 +146,7 @@ block-11:
 func TestDeviceFilter_Privileged(t *testing.T) {
 	devices := []*devices.Rule{
 		{
-			Type:        devices.WildcardDevice,
+			Type:        'a',
 			Major:       -1,
 			Minor:       -1,
 			Permissions: "rwm",
@@ -173,14 +173,14 @@ block-0:
 func TestDeviceFilter_PrivilegedExceptSingleDevice(t *testing.T) {
 	devices := []*devices.Rule{
 		{
-			Type:        devices.WildcardDevice,
+			Type:        'a',
 			Major:       -1,
 			Minor:       -1,
 			Permissions: "rwm",
 			Allow:       true,
 		},
 		{
-			Type:        devices.BlockDevice,
+			Type:        'b',
 			Major:       8,
 			Minor:       0,
 			Permissions: "rwm",
@@ -213,21 +213,21 @@ block-1:
 func TestDeviceFilter_Weird(t *testing.T) {
 	devices := []*devices.Rule{
 		{
-			Type:        devices.BlockDevice,
+			Type:        'b',
 			Major:       8,
 			Minor:       1,
 			Permissions: "rwm",
 			Allow:       false,
 		},
 		{
-			Type:        devices.WildcardDevice,
+			Type:        'a',
 			Major:       -1,
 			Minor:       -1,
 			Permissions: "rwm",
 			Allow:       true,
 		},
 		{
-			Type:        devices.BlockDevice,
+			Type:        'b',
 			Major:       8,
 			Minor:       2,
 			Permissions: "rwm",

libcontainer/devices/device.go

@@ -100,13 +100,13 @@ func (p Permissions) IsValid() bool {
 	return p == fromSet(p.toSet())
 }
 
-type Type string
+type Type rune
 
 const (
-	WildcardDevice Type = "a"
-	BlockDevice    Type = "b"
-	CharDevice     Type = "c" // or 'u'
-	FifoDevice     Type = "p"
+	WildcardDevice Type = 'a'
+	BlockDevice    Type = 'b'
+	CharDevice     Type = 'c' // or 'u'
+	FifoDevice     Type = 'p'
 )
 
 func (t Type) IsValid() bool {
@@ -166,7 +166,7 @@ func (d *Rule) CgroupString() string {
 	if d.Minor == Wildcard {
 		minor = "*"
 	}
-	return fmt.Sprintf("%s %s:%s %s", d.Type, major, minor, d.Permissions)
+	return fmt.Sprintf("%c %s:%s %s", d.Type, major, minor, d.Permissions)
 }
 
 func (d *Rule) Mkdev() (uint64, error) {

libcontainer/rootfs_linux.go

@@ -710,7 +710,7 @@ func mknodDevice(dest string, node *devices.Device) error {
 	case devices.FifoDevice:
 		fileMode |= unix.S_IFIFO
 	default:
-		return fmt.Errorf("%s is not a valid device type for device %s", node.Type, node.Path)
+		return fmt.Errorf("%c is not a valid device type for device %s", node.Type, node.Path)
 	}
 	dev, err := node.Mkdev()
 	if err != nil {

libcontainer/specconv/spec_linux.go

@@ -479,15 +479,12 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 		if r != nil {
 			for i, d := range spec.Linux.Resources.Devices {
 				var (
-					dt    = devices.WildcardDevice
+					t     = "a"
 					major = int64(-1)
 					minor = int64(-1)
 				)
 				if d.Type != "" {
-					dt = devices.Type(d.Type)
-					if !dt.CanCgroup() {
-						return nil, fmt.Errorf("invalid cgroup device type %q", d.Type)
-					}
+					t = d.Type
 				}
 				if d.Major != nil {
 					major = *d.Major
@@ -498,6 +495,10 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 				if d.Access == "" {
 					return nil, fmt.Errorf("device access at %d field cannot be empty", i)
 				}
+				dt, err := stringToCgroupDeviceRune(t)
+				if err != nil {
+					return nil, err
+				}
 				c.Resources.Devices = append(c.Resources.Devices, &devices.Rule{
 					Type:        dt,
 					Major:       major,
@@ -634,7 +635,20 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 	return c, nil
 }
 
-func stringToDeviceType(s string) (devices.Type, error) {
+func stringToCgroupDeviceRune(s string) (devices.Type, error) {
+	switch s {
+	case "a":
+		return devices.WildcardDevice, nil
+	case "b":
+		return devices.BlockDevice, nil
+	case "c":
+		return devices.CharDevice, nil
+	default:
+		return 0, fmt.Errorf("invalid cgroup device type %q", s)
+	}
+}
+
+func stringToDeviceRune(s string) (devices.Type, error) {
 	switch s {
 	case "p":
 		return devices.FifoDevice, nil
@@ -643,7 +657,7 @@ func stringToDeviceType(s string) (devices.Type, error) {
 	case "b":
 		return devices.BlockDevice, nil
 	default:
-		return "", fmt.Errorf("invalid device type %q", s)
+		return 0, fmt.Errorf("invalid device type %q", s)
 	}
 }
 
@@ -679,7 +693,7 @@ next:
 			if d.GID != nil {
 				gid = *d.GID
 			}
-			dt, err := stringToDeviceType(d.Type)
+			dt, err := stringToDeviceRune(d.Type)
 			if err != nil {
 				return nil, err
 			}