Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:zzndb001:test
runc
3402.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
