File 3402.patch of Package runc
From a5bf5cfde84381ce99bb8faead0ef6f122843a2a Mon Sep 17 00:00:00 2001
From: Stefan Agner <stefan@agner.ch>
Date: Thu, 3 Mar 2022 14:24:37 +0100
Subject: [PATCH 1/4] Separate Device handling for default AllowDevices
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
libcontainer/devices/device.go | 3 +
libcontainer/specconv/spec_linux.go | 96 ++++++++++++++++--------
libcontainer/specconv/spec_linux_test.go | 8 +-
3 files changed, 70 insertions(+), 37 deletions(-)
diff --git a/libcontainer/devices/device.go b/libcontainer/devices/device.go
index c2c2b3bb7c0..9540a89c55b 100644
--- a/libcontainer/devices/device.go
+++ b/libcontainer/devices/device.go
@@ -24,6 +24,9 @@ type Device struct {
// Gid of the device.
Gid uint32 `json:"gid"`
+
+ // Is Default Device
+ IsDefault bool `json:"is_default"`
}
// Permissions is a cgroupv1-style string to represent device access. It
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index fbb68c24d5d..c8d3e3c6c59 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -201,6 +201,7 @@ func KnownMountOptions() []string {
var AllowedDevices = []*devices.Device{
// allow mknod for any device
{
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: devices.Wildcard,
@@ -210,6 +211,7 @@ var AllowedDevices = []*devices.Device{
},
},
{
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.BlockDevice,
Major: devices.Wildcard,
@@ -219,10 +221,11 @@ var AllowedDevices = []*devices.Device{
},
},
{
- Path: "/dev/null",
- FileMode: 0o666,
- Uid: 0,
- Gid: 0,
+ Path: "/dev/null",
+ FileMode: 0o666,
+ Uid: 0,
+ Gid: 0,
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 1,
@@ -232,10 +235,11 @@ var AllowedDevices = []*devices.Device{
},
},
{
- Path: "/dev/random",
- FileMode: 0o666,
- Uid: 0,
- Gid: 0,
+ Path: "/dev/random",
+ FileMode: 0o666,
+ Uid: 0,
+ Gid: 0,
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 1,
@@ -245,10 +249,11 @@ var AllowedDevices = []*devices.Device{
},
},
{
- Path: "/dev/full",
- FileMode: 0o666,
- Uid: 0,
- Gid: 0,
+ Path: "/dev/full",
+ FileMode: 0o666,
+ Uid: 0,
+ Gid: 0,
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 1,
@@ -258,10 +263,11 @@ var AllowedDevices = []*devices.Device{
},
},
{
- Path: "/dev/tty",
- FileMode: 0o666,
- Uid: 0,
- Gid: 0,
+ Path: "/dev/tty",
+ FileMode: 0o666,
+ Uid: 0,
+ Gid: 0,
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 5,
@@ -271,10 +277,11 @@ var AllowedDevices = []*devices.Device{
},
},
{
- Path: "/dev/zero",
- FileMode: 0o666,
- Uid: 0,
- Gid: 0,
+ Path: "/dev/zero",
+ FileMode: 0o666,
+ Uid: 0,
+ Gid: 0,
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 1,
@@ -284,10 +291,11 @@ var AllowedDevices = []*devices.Device{
},
},
{
- Path: "/dev/urandom",
- FileMode: 0o666,
- Uid: 0,
- Gid: 0,
+ Path: "/dev/urandom",
+ FileMode: 0o666,
+ Uid: 0,
+ Gid: 0,
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 1,
@@ -298,6 +306,7 @@ var AllowedDevices = []*devices.Device{
},
// /dev/pts/ - pts namespaces are "coming soon"
{
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 136,
@@ -307,6 +316,7 @@ var AllowedDevices = []*devices.Device{
},
},
{
+ IsDefault: true,
Rule: devices.Rule{
Type: devices.CharDevice,
Major: 5,
@@ -380,12 +390,14 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
config.Mounts = append(config.Mounts, cm)
}
- defaultDevs, err := createDevices(spec, config)
+ err = createDevices(spec, config)
if err != nil {
return nil, err
}
- c, err := CreateCgroupConfig(opts, defaultDevs)
+ defaultAllowedDevices := createDefaultDevicesCgroups(config)
+
+ c, err := CreateCgroupConfig(opts, defaultAllowedDevices)
if err != nil {
return nil, err
}
@@ -941,21 +953,22 @@ func stringToDeviceRune(s string) (devices.Type, error) {
}
}
-func createDevices(spec *specs.Spec, config *configs.Config) ([]*devices.Device, error) {
+func createDevices(spec *specs.Spec, config *configs.Config) error {
// If a spec device is redundant with a default device, remove that default
// device (the spec one takes priority).
- dedupedAllowDevs := []*devices.Device{}
-
next:
for _, ad := range AllowedDevices {
- if ad.Path != "" && spec.Linux != nil {
+ if ad.Path == "" {
+ continue next
+ }
+
+ if spec.Linux != nil {
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)
}
@@ -975,7 +988,7 @@ next:
}
dt, err := stringToDeviceRune(d.Type)
if err != nil {
- return nil, err
+ return err
}
if d.FileMode != nil {
filemode = *d.FileMode &^ unix.S_IFMT
@@ -995,7 +1008,24 @@ next:
}
}
- return dedupedAllowDevs, nil
+ return nil
+}
+
+func createDefaultDevicesCgroups(config *configs.Config) []*devices.Device {
+ defaultAllowedDevices := []*devices.Device{}
+next:
+ for _, ad := range AllowedDevices {
+ if ad.Path != "" {
+ for _, device := range config.Devices {
+ if ad.Path == device.Path && !device.IsDefault {
+ continue next
+ }
+ }
+ }
+ defaultAllowedDevices = append(defaultAllowedDevices, ad)
+ }
+
+ return defaultAllowedDevices
}
func setupUserNamespace(spec *specs.Spec, config *configs.Config) error {
diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go
index 8c7fb774f97..965ec4ffbb4 100644
--- a/libcontainer/specconv/spec_linux_test.go
+++ b/libcontainer/specconv/spec_linux_test.go
@@ -886,17 +886,17 @@ func TestCreateDevices(t *testing.T) {
conf := &configs.Config{}
- defaultDevs, err := createDevices(spec, conf)
+ 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
+ // Verify the returned devices has the /dev/tty entry deduplicated
found := false
- for _, d := range defaultDevs {
+ for _, d := range conf.Devices {
if d.Path == "/dev/tty" {
if found {
- t.Errorf("createDevices failed: returned a duplicated device entry: %v", defaultDevs)
+ t.Errorf("createDevices failed: returned a duplicated device entry: %v", conf.Devices)
}
found = true
}
From 53506ab025f3e684039ec1142de541374b991172 Mon Sep 17 00:00:00 2001
From: Stefan Agner <stefan@agner.ch>
Date: Thu, 3 Mar 2022 14:55:53 +0100
Subject: [PATCH 2/4] Implement common function to create DeviceCgroup rules
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
libcontainer/specconv/spec_linux.go | 84 +++++++++++++++++------------
1 file changed, 49 insertions(+), 35 deletions(-)
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index c8d3e3c6c59..73623e9eccc 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -704,6 +704,48 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) {
return sp, nil
}
+func CreateCgroupDeviceConfig(r *configs.Resources, specr *specs.LinuxResources, defaultDevs []*devices.Device) error {
+ if specr != nil {
+ for i, d := range specr.Devices {
+ var (
+ t = "a"
+ major = int64(-1)
+ minor = int64(-1)
+ )
+ if d.Type != "" {
+ t = d.Type
+ }
+ if d.Major != nil {
+ major = *d.Major
+ }
+ if d.Minor != nil {
+ minor = *d.Minor
+ }
+ if d.Access == "" {
+ return fmt.Errorf("device access at %d field cannot be empty", i)
+ }
+ dt, err := stringToCgroupDeviceRune(t)
+ if err != nil {
+ return err
+ }
+ r.Devices = append(r.Devices, &devices.Rule{
+ Type: dt,
+ Major: major,
+ Minor: minor,
+ Permissions: devices.Permissions(d.Access),
+ Allow: d.Allow,
+ })
+ }
+ }
+
+ // Append the default allowed devices to the end of the list.
+ for _, device := range defaultDevs {
+ r.Devices = append(r.Devices, &device.Rule)
+ }
+
+ return nil
+}
+
func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*configs.Cgroup, error) {
var (
myCgroupPath string
@@ -760,39 +802,10 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
// In rootless containers, any attempt to make cgroup changes is likely to fail.
// libcontainer will validate this but ignores the error.
+ var r *specs.LinuxResources
if spec.Linux != nil {
- r := spec.Linux.Resources
+ r = spec.Linux.Resources
if r != nil {
- for i, d := range r.Devices {
- var (
- t = "a"
- major = int64(-1)
- minor = int64(-1)
- )
- if d.Type != "" {
- t = d.Type
- }
- if d.Major != nil {
- major = *d.Major
- }
- if d.Minor != nil {
- minor = *d.Minor
- }
- 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,
- Minor: minor,
- Permissions: devices.Permissions(d.Access),
- Allow: d.Allow,
- })
- }
if r.Memory != nil {
if r.Memory.Limit != nil {
c.Resources.Memory = *r.Memory.Limit
@@ -918,12 +931,13 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
}
}
}
- }
- // Append the default allowed devices to the end of the list.
- for _, device := range defaultDevs {
- c.Resources.Devices = append(c.Resources.Devices, &device.Rule)
+ err := CreateCgroupDeviceConfig(c.Resources, r, defaultDevs)
+ if err != nil {
+ return nil, err
+ }
}
+
return c, nil
}
From 2958e81addf2081a72631149b12af448eb569e54 Mon Sep 17 00:00:00 2001
From: Stefan Agner <stefan@agner.ch>
Date: Thu, 3 Mar 2022 15:43:10 +0100
Subject: [PATCH 3/4] Implement Device Resources updates
Add support to update Device Resources with runc update.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
libcontainer/specconv/spec_linux.go | 4 ++--
update.go | 26 +++++++++++++++++++-------
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index 73623e9eccc..ba9468d802d 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -395,7 +395,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
return nil, err
}
- defaultAllowedDevices := createDefaultDevicesCgroups(config)
+ defaultAllowedDevices := CreateDefaultDevicesCgroups(config)
c, err := CreateCgroupConfig(opts, defaultAllowedDevices)
if err != nil {
@@ -1025,7 +1025,7 @@ next:
return nil
}
-func createDefaultDevicesCgroups(config *configs.Config) []*devices.Device {
+func CreateDefaultDevicesCgroups(config *configs.Config) []*devices.Device {
defaultAllowedDevices := []*devices.Device{}
next:
for _, ad := range AllowedDevices {
diff --git a/update.go b/update.go
index fc2d656abbf..12b377793b3 100644
--- a/update.go
+++ b/update.go
@@ -8,6 +8,7 @@ import (
"strconv"
"github.com/opencontainers/runc/libcontainer/cgroups"
+ "github.com/opencontainers/runc/libcontainer/specconv"
"github.com/sirupsen/logrus"
"github.com/docker/go-units"
@@ -322,6 +323,24 @@ other options are ignored.
config.Cgroups.Resources.PidsLimit = r.Pids.Limit
config.Cgroups.Resources.Unified = r.Unified
+ if len(r.Devices) > 0 {
+ config.Cgroups.Resources.Devices = nil
+ defaultAllowedDevices := specconv.CreateDefaultDevicesCgroups(&config)
+
+ err = specconv.CreateCgroupDeviceConfig(config.Cgroups.Resources, &r, defaultAllowedDevices)
+ if err != nil {
+ return err
+ }
+ config.Cgroups.SkipDevices = false
+ } else {
+ // If "runc update" is not changing device configuration, add
+ // this to skip device update.
+ // This helps in case an extra plugin (nvidia GPU) applies some
+ // configuration on top of what runc does.
+ // Note this field is not saved into container's state.json.
+ config.Cgroups.SkipDevices = true
+ }
+
// Update Intel RDT
l3CacheSchema := context.String("l3-cache-schema")
memBwSchema := context.String("mem-bw-schema")
@@ -353,13 +372,6 @@ other options are ignored.
config.IntelRdt.MemBwSchema = memBwSchema
}
- // XXX(kolyshkin@): currently "runc update" is unable to change
- // device configuration, so add this to skip device update.
- // This helps in case an extra plugin (nvidia GPU) applies some
- // configuration on top of what runc does.
- // Note this field is not saved into container's state.json.
- config.Cgroups.SkipDevices = true
-
return container.Set(config)
},
}
From f17333d7c0ffc0cf5964622bcd84a9de3705e7f0 Mon Sep 17 00:00:00 2001
From: Stefan Agner <stefan@agner.ch>
Date: Fri, 5 Aug 2022 13:03:21 +0200
Subject: [PATCH 4/4] Add integration tests for device updates
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
tests/integration/update.bats | 62 +++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index 5a3dc7f0563..ef5d4cfd1e2 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -810,6 +810,68 @@ EOF
[ -z "$(<"$CONTAINER_OUTPUT")" ]
}
+@test "update devices" {
+ requires root
+
+ # Create a container without access to /dev/kmsg. Verify that accessing fails
+ # and update the container to add access from it.
+ #
+ # Finally, remove access again by passing an empty array of devices.
+ update_config ' .linux.resources.devices = [{"allow": false, "access": "rwm"}]
+ | .linux.devices = [{"path": "/dev/kmsg", "type": "c", "major": 1, "minor": 11}]
+ | .process.capabilities.bounding += ["CAP_SYSLOG"]
+ | .process.capabilities.effective += ["CAP_SYSLOG"]
+ | .process.capabilities.inheritable += ["CAP_SYSLOG"]
+ | .process.capabilities.permitted += ["CAP_SYSLOG"]'
+
+ # Run the container in the background.
+ runc run -d --console-socket "$CONSOLE_SOCKET" test_update_devices
+ [ "$status" -eq 0 ]
+
+ runc exec test_update_devices head -c 100 /dev/kmsg
+ [ "$status" -eq 1 ]
+
+ # Make sure default devices still work
+ runc exec test_update_devices cat /dev/null
+ [ "$status" -eq 0 ]
+
+ runc update --resources - test_update_devices <<EOF
+ {
+ "devices": [
+ {"allow": true, "type": "c", "major": 1, "minor": 11, "access": "rwa"}
+ ]
+ }
+EOF
+
+ runc exec test_update_devices head -c 100 /dev/kmsg
+ [ "$status" -eq 0 ]
+
+ # Make sure default devices still work
+ runc exec test_update_devices cat /dev/null
+ [ "$status" -eq 0 ]
+
+ # Not updating devices should keep device access
+ # For backwards compatibility
+ runc update --resources - test_update_devices <<EOF
+ { }
+EOF
+
+ runc exec test_update_devices head -c 100 /dev/kmsg
+ [ "$status" -eq 0 ]
+
+ # Explicitly removing access should remove access
+ runc update --resources - test_update_devices <<EOF
+ {
+ "devices": [
+ {"allow": false, "type": "c", "major": 1, "minor": 11, "access": "rwa"}
+ ]
+ }
+EOF
+
+ runc exec test_update_devices head -c 100 /dev/kmsg
+ [ "$status" -eq 1 ]
+}
+
@test "update paused container" {
requires cgroups_freezer
[ $EUID -ne 0 ] && requires rootless_cgroup
