WIP DNM support Path in OCI spec's LinuxDeviceCgroup

libcontainer/cgroups/devices/devicefilter.go

@@ -29,11 +29,9 @@ func deviceFilter(rules []*devices.Rule) (asm.Instructions, string, error) {
 	// gives us a guarantee that the behaviour of devices filtering is the same
 	// as cgroupv1, including security hardenings to avoid misconfiguration
 	// (such as punching holes in wildcard rules).
-	emu := new(emulator)
-	for _, rule := range rules {
-		if err := emu.Apply(*rule); err != nil {
-			return nil, "", err
-		}
+	emu, err := emulatorFromRules(rules)
+	if err != nil {
+		return nil, "", err
 	}
 	cleanRules, err := emu.Rules()
 	if err != nil {

libcontainer/cgroups/devices/devices_emulator.go

@@ -369,6 +369,22 @@ func (source *emulator) Transition(target *emulator) ([]*devices.Rule, error) {
 	return transitionRules, nil
 }
 
+// emulatorFromRules creates a new emulator and adds the supplied device rules
+// to it.
+func emulatorFromRules(rules []*devices.Rule) (*emulator, error) {
+	// This defaults to a white-list -- which is what we want!
+	emu := &emulator{}
+	for _, rule := range rules {
+		if err := checkPath(rule); err != nil {
+			return nil, err
+		}
+		if err := emu.Apply(*rule); err != nil {
+			return nil, err
+		}
+	}
+	return emu, nil
+}
+
 // Rules returns the minimum set of rules necessary to convert a *deny-all*
 // cgroup to the emulated filter state (note that this is not the same as a
 // default cgroupv1 cgroup -- which is allow-all). This is effectively just a

libcontainer/cgroups/devices/path.go

@@ -0,0 +1,62 @@
+package devices
+
+import (
+	"errors"
+	"os"
+
+	"github.com/opencontainers/runc/libcontainer/devices"
+	"golang.org/x/sys/unix"
+)
+
+var (
+	errNotDev   = errors.New("not a block or character device")
+	errMismatch = errors.New("device type/major/minor specified do not match those of the actual device")
+)
+
+// checkPath checks the Path component of the cgroups device rule, if set. In
+// case device type/major/minor are also set, the device is checked to have the
+// same type and major:minor. In case those are not set, they are populated
+// from the actual device node.
+func checkPath(r *devices.Rule) error {
+	if r.Path == "" {
+		return nil
+	}
+
+	var stat unix.Stat_t
+	if err := unix.Lstat(r.Path, &stat); err != nil {
+		return &os.PathError{Op: "lstat", Path: r.Path, Err: err}
+	}
+
+	var (
+		devType   devices.Type
+		devNumber = uint64(stat.Rdev) //nolint:unconvert // Rdev is uint32 on e.g. MIPS.
+		major     = int64(unix.Major(devNumber))
+		minor     = int64(unix.Minor(devNumber))
+	)
+	switch stat.Mode & unix.S_IFMT {
+	case unix.S_IFBLK:
+		devType = devices.BlockDevice
+	case unix.S_IFCHR:
+		devType = devices.CharDevice
+	default:
+		return &os.PathError{Op: "pathCheck", Path: r.Path, Err: errNotDev}
+	}
+
+	if r.Minor == -1 && r.Major == -1 && r.Type == devices.WildcardDevice {
+		// Those are defaults in specconv.CreateCroupConfig, meaning
+		// these fields were not set in spec, so fill them in from the
+		// actual device.
+		r.Major = major
+		r.Minor = minor
+		r.Type = devType
+		return nil
+	}
+
+	// Otherwise (both Path and Type/Major/Minor are specified,
+	// which is redundant), do a sanity check.
+	if r.Major != major || r.Minor != minor || r.Type != devType {
+		return &os.PathError{Op: "pathCheck", Path: r.Path, Err: errMismatch}
+	}
+
+	return nil
+}

libcontainer/cgroups/devices/systemd.go

@@ -30,11 +30,9 @@ func systemdProperties(r *configs.Resources) ([]systemdDbus.Property, error) {
 	}
 
 	// Figure out the set of rules.
-	configEmu := emulator{}
-	for _, rule := range r.Devices {
-		if err := configEmu.Apply(*rule); err != nil {
-			return nil, fmt.Errorf("unable to apply rule for systemd: %w", err)
-		}
+	configEmu, err := emulatorFromRules(r.Devices)
+	if err != nil {
+		return nil, fmt.Errorf("unable to apply cgroup device rules to systemd: %w", err)
 	}
 	// systemd doesn't support blacklists. So we log a warning, and tell
 	// systemd to act as a deny-all whitelist. This ruleset will be replaced

libcontainer/cgroups/devices/v1.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
-	"github.com/opencontainers/runc/libcontainer/devices"
 	"github.com/opencontainers/runc/libcontainer/userns"
 )
 
@@ -23,7 +22,7 @@ func setV1(path string, r *configs.Resources) error {
 	if err != nil {
 		return err
 	}
-	target, err := buildEmulator(r.Devices)
+	target, err := emulatorFromRules(r.Devices)
 	if err != nil {
 		return err
 	}
@@ -71,14 +70,3 @@ func loadEmulator(path string) (*emulator, error) {
 	}
 	return emulatorFromList(bytes.NewBufferString(list))
 }
-
-func buildEmulator(rules []*devices.Rule) (*emulator, error) {
-	// This defaults to a white-list -- which is what we want!
-	emu := &emulator{}
-	for _, rule := range rules {
-		if err := emu.Apply(*rule); err != nil {
-			return nil, err
-		}
-	}
-	return emu, nil
-}

libcontainer/devices/device.go

@@ -13,9 +13,6 @@ const (
 type Device struct {
 	Rule
 
-	// Path to the device.
-	Path string `json:"path"`
-
 	// FileMode permission bits for the device.
 	FileMode os.FileMode `json:"file_mode"`
 
@@ -147,6 +144,9 @@ type Rule struct {
 	// Minor is the device's minor number.
 	Minor int64 `json:"minor"`
 
+	// Path is an optional absolute path to device file.
+	Path string
+
 	// Permissions is the set of permissions that this rule applies to (in the
 	// cgroupv1 format -- any combination of "rwm").
 	Permissions Permissions `json:"permissions"`

libcontainer/devices/device_unix.go

@@ -59,9 +59,9 @@ func DeviceFromPath(path, permissions string) (*Device, error) {
 			Type:        devType,
 			Major:       int64(major),
 			Minor:       int64(minor),
+			Path:        path,
 			Permissions: Permissions(permissions),
 		},
-		Path:     path,
 		FileMode: os.FileMode(mode &^ unix.S_IFMT),
 		Uid:      stat.Uid,
 		Gid:      stat.Gid,

libcontainer/specconv/spec_linux.go

@@ -207,11 +207,11 @@ var AllowedDevices = []*devices.Device{
 		},
 	},
 	{
-		Path:     "/dev/null",
 		FileMode: 0o666,
 		Uid:      0,
 		Gid:      0,
 		Rule: devices.Rule{
+			Path:        "/dev/null",
 			Type:        devices.CharDevice,
 			Major:       1,
 			Minor:       3,
@@ -220,11 +220,11 @@ var AllowedDevices = []*devices.Device{
 		},
 	},
 	{
-		Path:     "/dev/random",
 		FileMode: 0o666,
 		Uid:      0,
 		Gid:      0,
 		Rule: devices.Rule{
+			Path:        "/dev/random",
 			Type:        devices.CharDevice,
 			Major:       1,
 			Minor:       8,
@@ -233,11 +233,11 @@ var AllowedDevices = []*devices.Device{
 		},
 	},
 	{
-		Path:     "/dev/full",
 		FileMode: 0o666,
 		Uid:      0,
 		Gid:      0,
 		Rule: devices.Rule{
+			Path:        "/dev/full",
 			Type:        devices.CharDevice,
 			Major:       1,
 			Minor:       7,
@@ -246,11 +246,11 @@ var AllowedDevices = []*devices.Device{
 		},
 	},
 	{
-		Path:     "/dev/tty",
 		FileMode: 0o666,
 		Uid:      0,
 		Gid:      0,
 		Rule: devices.Rule{
+			Path:        "/dev/tty",
 			Type:        devices.CharDevice,
 			Major:       5,
 			Minor:       0,
@@ -259,11 +259,11 @@ var AllowedDevices = []*devices.Device{
 		},
 	},
 	{
-		Path:     "/dev/zero",
 		FileMode: 0o666,
 		Uid:      0,
 		Gid:      0,
 		Rule: devices.Rule{
+			Path:        "/dev/zero",
 			Type:        devices.CharDevice,
 			Major:       1,
 			Minor:       5,
@@ -272,11 +272,11 @@ var AllowedDevices = []*devices.Device{
 		},
 	},
 	{
-		Path:     "/dev/urandom",
 		FileMode: 0o666,
 		Uid:      0,
 		Gid:      0,
 		Rule: devices.Rule{
+			Path:        "/dev/urandom",
 			Type:        devices.CharDevice,
 			Major:       1,
 			Minor:       9,
@@ -296,6 +296,7 @@ var AllowedDevices = []*devices.Device{
 	},
 	{
 		Rule: devices.Rule{
+			Path:        "/dev/ptmx",
 			Type:        devices.CharDevice,
 			Major:       5,
 			Minor:       2,
@@ -700,6 +701,7 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 					Type:        dt,
 					Major:       major,
 					Minor:       minor,
+					Path:        d.Path,
 					Permissions: devices.Permissions(d.Access),
 					Allow:       d.Allow,
 				})
@@ -911,8 +913,8 @@ next:
 					Type:  dt,
 					Major: d.Major,
 					Minor: d.Minor,
+					Path:  d.Path,
 				},
-				Path:     d.Path,
 				FileMode: filemode,
 				Uid:      uid,
 				Gid:      gid,

libcontainer/specconv/spec_linux_test.go

@@ -893,11 +893,11 @@ func TestCreateDevices(t *testing.T) {
 	for _, configDev := range conf.Devices {
 		if configDev.Path == "/dev/tty" {
 			wantDev := &devices.Device{
-				Path:     "/dev/tty",
 				FileMode: 0o666,
 				Uid:      1000,
 				Gid:      1000,
 				Rule: devices.Rule{
+					Path:  "/dev/tty",
 					Type:  devices.CharDevice,
 					Major: 5,
 					Minor: 0,