cgroupv1: devices: use minimal transition rules with devices.Emulator

Now that all of the infrastructure for devices.Emulator is in place, we
can finally implement minimal transition rules for devices cgroups. This
allows for minimal disruption to running containers if a rule update is
requested. Only in very rare circumstances (black-list cgroups and mode
switching) will a clear-all rule be written. As a result, containers
should no longer see spurious errors.

A similar issue affects the cgroupv2 devices setup, but that is a topic
for another time (as the solution is drastically different).

Signed-off-by: Aleksa Sarai <[email protected]>

libcontainer/cgroups/fs/devices.go

@@ -3,13 +3,19 @@
 package fs
 
 import (
+	"bytes"
+	"fmt"
+	"reflect"
+
 	"github.com/opencontainers/runc/libcontainer/cgroups"
+	"github.com/opencontainers/runc/libcontainer/cgroups/devices"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/system"
 )
 
 type DevicesGroup struct {
+	testingSkipFinalCheck bool
 }
 
 func (s *DevicesGroup) Name() string {
@@ -26,29 +32,74 @@ func (s *DevicesGroup) Apply(d *cgroupData) error {
 	return nil
 }
 
+func loadEmulator(path string) (*devices.Emulator, error) {
+	list, err := fscommon.ReadFile(path, "devices.list")
+	if err != nil {
+		return nil, err
+	}
+	return devices.EmulatorFromList(bytes.NewBufferString(list))
+}
+
+func buildEmulator(rules []*configs.DeviceRule) (*devices.Emulator, error) {
+	// This defaults to a white-list -- which is what we want!
+	emu := &devices.Emulator{}
+	for _, rule := range rules {
+		if err := emu.Apply(*rule); err != nil {
+			return nil, err
+		}
+	}
+	return emu, nil
+}
+
 func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error {
 	if system.RunningInUserNS() {
 		return nil
 	}
 
-	// The devices list is a whitelist, so we must first deny all devices.
-	// XXX: This is incorrect for device list updates as it will result in
-	//      spurrious errors in the container, but we will solve that
-	//      separately.
-	if err := fscommon.WriteFile(path, "devices.deny", "a"); err != nil {
+	// Generate two emulators, one for the current state of the cgroup and one
+	// for the requested state by the user.
+	current, err := loadEmulator(path)
+	if err != nil {
+		return err
+	}
+	target, err := buildEmulator(cgroup.Resources.Devices)
+	if err != nil {
 		return err
 	}
 
-	devices := cgroup.Resources.Devices
-	for _, dev := range devices {
+	// Compute the minimal set of transition rules needed to achieve the
+	// requested state.
+	transitionRules, err := current.Transition(target)
+	if err != nil {
+		return err
+	}
+	for _, rule := range transitionRules {
 		file := "devices.deny"
-		if dev.Allow {
+		if rule.Allow {
 			file = "devices.allow"
 		}
-		if err := fscommon.WriteFile(path, file, dev.CgroupString()); err != nil {
+		if err := fscommon.WriteFile(path, file, rule.CgroupString()); err != nil {
 			return err
 		}
 	}
+
+	// Final safety check -- ensure that the resulting state is what was
+	// requested. This is only really correct for white-lists, but for
+	// black-lists we can at least check that the cgroup is in the right mode.
+	//
+	// This safety-check is skipped for the unit tests because we cannot
+	// currently mock devices.list correctly.
+	if !s.testingSkipFinalCheck {
+		currentAfter, err := loadEmulator(path)
+		if err != nil {
+			return err
+		}
+		if !target.IsBlacklist() && !reflect.DeepEqual(currentAfter, target) {
+			return fmt.Errorf("resulting devices cgroup doesn't precisely match target")
+		} else if target.IsBlacklist() != currentAfter.IsBlacklist() {
+			return fmt.Errorf("resulting devices cgroup doesn't match target mode")
+		}
+	}
 	return nil
 }
 

libcontainer/cgroups/fs/devices_test.go

@@ -16,6 +16,7 @@ func TestDevicesSetAllow(t *testing.T) {
 	helper.writeFileContents(map[string]string{
 		"devices.allow": "",
 		"devices.deny":  "",
+		"devices.list":  "a *:* rwm",
 	})
 
 	helper.CgroupData.config.Resources.Devices = []*configs.DeviceRule{
@@ -28,7 +29,7 @@ func TestDevicesSetAllow(t *testing.T) {
 		},
 	}
 
-	devices := &DevicesGroup{}
+	devices := &DevicesGroup{testingSkipFinalCheck: true}
 	if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}