Remove runc default devices that overlap with spec devices.

libcontainer/rootfs_linux.go

@@ -19,6 +19,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/utils"
 	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/selinux/go-selinux/label"
 
@@ -589,6 +590,12 @@ func createDevices(config *configs.Config) error {
 	useBindMount := system.RunningInUserNS() || config.Namespaces.Contains(configs.NEWUSER)
 	oldMask := unix.Umask(0000)
 	for _, node := range config.Devices {
+
+		// The /dev/ptmx device is setup by setupPtmx()
+		if utils.CleanPath(node.Path) == "/dev/ptmx" {
+			continue
+		}
+
 		// containers running in a user namespace are not allowed to mknod
 		// devices so we can just bind mount it from the host.
 		if err := createDeviceNode(config.Rootfs, node, useBindMount); err != nil {

libcontainer/specconv/spec_linux.go

@@ -241,13 +241,17 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 	for _, m := range spec.Mounts {
 		config.Mounts = append(config.Mounts, createLibcontainerMount(cwd, m))
 	}
-	if err := createDevices(spec, config); err != nil {
+
+	defaultDevs, err := createDevices(spec, config)
+	if err != nil {
 		return nil, err
 	}
-	c, err := CreateCgroupConfig(opts)
+
+	c, err := CreateCgroupConfig(opts, defaultDevs)
 	if err != nil {
 		return nil, err
 	}
+
 	config.Cgroups = c
 	// set linux-specific config
 	if spec.Linux != nil {
@@ -410,7 +414,7 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) {
 	return sp, nil
 }
 
-func CreateCgroupConfig(opts *CreateOpts) (*configs.Cgroup, error) {
+func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*configs.Device) (*configs.Cgroup, error) {
 	var (
 		myCgroupPath string
 
@@ -616,9 +620,9 @@ func CreateCgroupConfig(opts *CreateOpts) (*configs.Cgroup, error) {
 			}
 		}
 	}
+
 	// Append the default allowed devices to the end of the list.
-	// XXX: Really this should be prefixed...
-	for _, device := range AllowedDevices {
+	for _, device := range defaultDevs {
 		c.Resources.Devices = append(c.Resources.Devices, &device.DeviceRule)
 	}
 	return c, nil
@@ -650,13 +654,26 @@ func stringToDeviceRune(s string) (configs.DeviceType, error) {
 	}
 }
 
-func createDevices(spec *specs.Spec, config *configs.Config) error {
-	// Add default set of devices.
-	for _, device := range AllowedDevices {
-		if device.Path != "" {
-			config.Devices = append(config.Devices, device)
+func createDevices(spec *specs.Spec, config *configs.Config) ([]*configs.Device, error) {
+	// If a spec device is redundant with a default device, remove that default
+	// device (the spec one takes priority).
+	dedupedAllowDevs := []*configs.Device{}
+
+next:
+	for _, ad := range AllowedDevices {
+		if ad.Path != "" {
+			for _, sd := range spec.Linux.Devices {
+				if sd.Path == ad.Path {
+					continue next
+				}
+			}
+		}
+		dedupedAllowDevs = append(dedupedAllowDevs, ad)
+		if ad.Path != "" {
+			config.Devices = append(config.Devices, ad)
 		}
 	}
+
 	// Merge in additional devices from the spec.
 	if spec.Linux != nil {
 		for _, d := range spec.Linux.Devices {
@@ -671,7 +688,7 @@ func createDevices(spec *specs.Spec, config *configs.Config) error {
 			}
 			dt, err := stringToDeviceRune(d.Type)
 			if err != nil {
-				return err
+				return nil, err
 			}
 			if d.FileMode != nil {
 				filemode = *d.FileMode
@@ -690,7 +707,8 @@ func createDevices(spec *specs.Spec, config *configs.Config) error {
 			config.Devices = append(config.Devices, device)
 		}
 	}
-	return nil
+
+	return dedupedAllowDevs, nil
 }
 
 func setupUserNamespace(spec *specs.Spec, config *configs.Config) error {

libcontainer/specconv/spec_linux_test.go

@@ -259,7 +259,7 @@ func TestLinuxCgroupWithMemoryResource(t *testing.T) {
 		Spec:             spec,
 	}
 
-	cgroup, err := CreateCgroupConfig(opts)
+	cgroup, err := CreateCgroupConfig(opts, nil)
 	if err != nil {
 		t.Errorf("Couldn't create Cgroup config: %v", err)
 	}
@@ -303,7 +303,7 @@ func TestLinuxCgroupSystemd(t *testing.T) {
 		Spec:             spec,
 	}
 
-	cgroup, err := CreateCgroupConfig(opts)
+	cgroup, err := CreateCgroupConfig(opts, nil)
 
 	if err != nil {
 		t.Errorf("Couldn't create Cgroup config: %v", err)
@@ -339,7 +339,7 @@ func TestLinuxCgroupSystemdWithEmptyPath(t *testing.T) {
 		Spec:             spec,
 	}
 
-	cgroup, err := CreateCgroupConfig(opts)
+	cgroup, err := CreateCgroupConfig(opts, nil)
 
 	if err != nil {
 		t.Errorf("Couldn't create Cgroup config: %v", err)
@@ -374,7 +374,7 @@ func TestLinuxCgroupSystemdWithInvalidPath(t *testing.T) {
 		Spec:             spec,
 	}
 
-	_, err := CreateCgroupConfig(opts)
+	_, err := CreateCgroupConfig(opts, nil)
 	if err == nil {
 		t.Error("Expected to produce an error if not using the correct format for cgroup paths belonging to systemd")
 	}
@@ -393,7 +393,7 @@ func TestLinuxCgroupsPathSpecified(t *testing.T) {
 		Spec:             spec,
 	}
 
-	cgroup, err := CreateCgroupConfig(opts)
+	cgroup, err := CreateCgroupConfig(opts, nil)
 	if err != nil {
 		t.Errorf("Couldn't create Cgroup config: %v", err)
 	}
@@ -411,7 +411,7 @@ func TestLinuxCgroupsPathNotSpecified(t *testing.T) {
 		Spec:             spec,
 	}
 
-	cgroup, err := CreateCgroupConfig(opts)
+	cgroup, err := CreateCgroupConfig(opts, nil)
 	if err != nil {
 		t.Errorf("Couldn't create Cgroup config: %v", err)
 	}
@@ -648,3 +648,107 @@ func TestNullProcess(t *testing.T) {
 		t.Errorf("Null process should be forbidden")
 	}
 }
+
+func TestCreateDevices(t *testing.T) {
+	spec := Example()
+
+	// dummy uid/gid for /dev/tty; will enable the test to check if createDevices()
+	// preferred the spec's device over the redundant default device
+	ttyUid := uint32(1000)
+	ttyGid := uint32(1000)
+	fm := os.FileMode(0666)
+
+	spec.Linux = &specs.Linux{
+		Devices: []specs.LinuxDevice{
+			{
+				// This is purposely redundant with one of runc's default devices
+				Path:     "/dev/tty",
+				Type:     "c",
+				Major:    5,
+				Minor:    0,
+				FileMode: &fm,
+				UID:      &ttyUid,
+				GID:      &ttyGid,
+			},
+			{
+				// This is purposely not redundant with one of runc's default devices
+				Path:  "/dev/ram0",
+				Type:  "b",
+				Major: 1,
+				Minor: 0,
+			},
+		},
+	}
+
+	conf := &configs.Config{}
+
+	defaultDevs, err := createDevices(spec, conf)
+	if err != nil {
+		t.Errorf("failed to create devices: %v", err)
+	}
+
+	// Verify the returned default devices has the /dev/tty entry deduplicated
+	found := false
+	for _, d := range defaultDevs {
+		if d.Path == "/dev/tty" {
+			if found {
+				t.Errorf("createDevices failed: returned a duplicated device entry: %v", defaultDevs)
+			}
+			found = true
+		}
+	}
+
+	// Verify that createDevices() placed all default devices in the config
+	for _, allowedDev := range AllowedDevices {
+		if allowedDev.Path == "" {
+			continue
+		}
+
+		found := false
+		for _, configDev := range conf.Devices {
+			if configDev.Path == allowedDev.Path {
+				found = true
+			}
+		}
+		if !found {
+			configDevPaths := []string{}
+			for _, configDev := range conf.Devices {
+				configDevPaths = append(configDevPaths, configDev.Path)
+			}
+			t.Errorf("allowedDevice %s was not found in the config's devices: %v", allowedDev.Path, configDevPaths)
+		}
+	}
+
+	// Verify that createDevices() deduplicated the /dev/tty entry in the config
+	for _, configDev := range conf.Devices {
+		if configDev.Path == "/dev/tty" {
+			wantDev := &configs.Device{
+				Path:     "/dev/tty",
+				FileMode: 0666,
+				Uid:      1000,
+				Gid:      1000,
+				DeviceRule: configs.DeviceRule{
+					Type:  configs.CharDevice,
+					Major: 5,
+					Minor: 0,
+				},
+			}
+
+			if *configDev != *wantDev {
+				t.Errorf("redundant dev was not deduplicated correctly: want %v, got %v", wantDev, configDev)
+			}
+		}
+	}
+
+	// Verify that createDevices() added the entry for /dev/ram0 in the config
+	found = false
+	for _, configDev := range conf.Devices {
+		if configDev.Path == "/dev/ram0" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("device /dev/ram0 not found in config devices; got %v", conf.Devices)
+	}
+}

tests/integration/dev.bats

@@ -0,0 +1,35 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	teardown_busybox
+	setup_busybox
+}
+
+function teardown() {
+	teardown_busybox
+}
+
+@test "runc run [redundant default /dev/tty]" {
+	update_config ' .linux.devices += [{"path": "/dev/tty", "type": "c", "major": 5, "minor": 0}]
+		      | .process.args |= ["ls", "-lL", "/dev/tty"]'
+
+	runc run test_dev
+	[ "$status" -eq 0 ]
+
+	if [[ "$ROOTLESS" -ne 0 ]]; then
+		[[ "${lines[0]}" =~ "crw-rw-rw".+"1".+"nobody".+"nogroup".+"5,".+"0".+"/dev/tty" ]]
+	else
+		[[ "${lines[0]}" =~ "crw-rw-rw".+"1".+"root".+"root".+"5,".+"0".+"/dev/tty" ]]
+	fi
+}
+
+@test "runc run [redundant default /dev/ptmx]" {
+	update_config ' .linux.devices += [{"path": "/dev/ptmx", "type": "c", "major": 5, "minor": 2}]
+		      | .process.args |= ["ls", "-lL", "/dev/ptmx"]'
+
+	runc run test_dev
+	[ "$status" -eq 0 ]
+	[[ "${lines[0]}" =~ "crw-rw-rw".+"1".+"root".+"root".+"5,".+"2".+"/dev/ptmx" ]]
+}