Skip to content

Commit

Permalink
[RFC] Add a systemd-based implementation to the "devices" subsystem
Browse files Browse the repository at this point in the history
I tested this with Podman using:

  $ podman --runtime ~/go/src/github.com/opencontainers/runc/runc run -t fedora:29 echo hello

And also bringing up a container and checking the contents of
"device.list" in the cgroup subtree:

  $ podman --runtime ~/go/src/github.com/opencontainers/runc/runc run -t fedora:29 sleep 1h
  $ cat /sys/fs/cgroup/devices/machine.slice/libpod-12fc7bd62fd6*/devices.list
  c 10:200 rwm
  c 5:2 rwm
  c 136:* rwm
  c 5:1 rwm
  c 1:9 rwm
  c 1:5 rwm
  c 5:0 rwm
  c 1:7 rwm
  c 1:8 rwm
  c 1:3 rwm
  b *:* m
  c *:* m

This matches the output of devices.list when using the official "runc"
binary, only difference being the lines are inverted in order (again, we
can fix that on a second step.)

Querying systemd for this unit also works as expected:

  $ systemctl show libpod-12fc7bd62fd66ff62fa1b045c2d717c7b2076c072c20de14f5c1ad86b78865eb.scope -p DevicePolicy -p DeviceAllow
  DevicePolicy=strict
  DeviceAllow=/dev/net/tun rwm
  DeviceAllow=/dev/ptmx rwm
  DeviceAllow=char-136 rwm
  DeviceAllow=/dev/console rwm
  DeviceAllow=/dev/urandom rwm
  DeviceAllow=/dev/zero rwm
  DeviceAllow=/dev/tty rwm
  DeviceAllow=/dev/full rwm
  DeviceAllow=/dev/random rwm
  DeviceAllow=/dev/null rwm
  DeviceAllow=block-* m
  DeviceAllow=char-* m

v2: Now using systemd support for /dev/{char,block}/<major>:<minor>
instead, which results into these properties being set instead:

  DevicePolicy=strict
  DeviceAllow=/dev/char/10:200 rwm
  DeviceAllow=/dev/char/5:2 rwm
  DeviceAllow=char-136 rwm
  DeviceAllow=/dev/char/5:1 rwm
  DeviceAllow=/dev/char/1:9 rwm
  DeviceAllow=/dev/char/1:5 rwm
  DeviceAllow=/dev/char/5:0 rwm
  DeviceAllow=/dev/char/1:7 rwm
  DeviceAllow=/dev/char/1:8 rwm
  DeviceAllow=/dev/char/1:3 rwm
  DeviceAllow=block-* m
  DeviceAllow=char-* m

Unclear whether we should really be using this unconditionally, or only
in the cases where a device name hasn't been supplied through d.Path.

Signed-off-by: Filipe Brandenburger <[email protected]>
  • Loading branch information
filbranden committed Mar 16, 2019
1 parent 162f6f0 commit a347be1
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 0 deletions.
72 changes: 72 additions & 0 deletions libcontainer/cgroups/fs/devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
package fs

import (
"fmt"

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/system"

systemdDbus "github.com/coreos/go-systemd/dbus"
"github.com/godbus/dbus"
)

type DevicesGroup struct {
Expand Down Expand Up @@ -68,9 +73,76 @@ func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error {
}
}

return s.SetCgroupv1(path, cgroup)
}

func (s *DevicesGroup) SetCgroupv1(path string, cgroup *configs.Cgroup) error {
return nil
}

type deviceAllow struct {
Path string
Permissions string
}

func (s *DevicesGroup) ToSystemdProperties(cgroup *configs.Cgroup) ([]systemdDbus.Property, error) {
var devAllows []deviceAllow
devPolicy := "strict"

devices := cgroup.Resources.Devices
if len(devices) > 0 {
blockedAll := false
for _, dev := range devices {
if !blockedAll {
// Expect the first rule to block all, in which
// case we can translate this cgroup config to
// something systemd will understand.
if dev.Type == 'a' && !dev.Allow {
blockedAll = true
} else {
return []systemdDbus.Property{}, fmt.Errorf("systemd only supports a whitelist on device cgroup, please use AllowedDevices instead.")
}
continue
}
// Ok, now we're handling the second+ device rules to
// whitelist the items that matter to us.
if !dev.Allow {
// We already blocked all, so continue...
continue
}
if devPath := dev.SystemdCgroupPath(); devPath != "" {
devAllows = append(devAllows, deviceAllow{
Path: devPath,
Permissions: dev.Permissions,
})
}
}
} else if cgroup.Resources.AllowAllDevices != nil {
if *cgroup.Resources.AllowAllDevices {
devPolicy = "auto"
} else {
for _, dev := range cgroup.Resources.AllowedDevices {
if devPath := dev.SystemdCgroupPath(); devPath != "" {
devAllows = append(devAllows, deviceAllow{
Path: devPath,
Permissions: dev.Permissions,
})
}
}
}
}
return []systemdDbus.Property{
{
Name: "DevicePolicy",
Value: dbus.MakeVariant(devPolicy),
},
{
Name: "DeviceAllow",
Value: dbus.MakeVariant(devAllows),
},
}, nil
}

func (s *DevicesGroup) Remove(d *cgroupData) error {
return removePath(d.path("devices"))
}
Expand Down
16 changes: 16 additions & 0 deletions libcontainer/configs/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@ func (d *Device) CgroupString() string {
return fmt.Sprintf("%c %s:%s %s", d.Type, deviceNumberString(d.Major), deviceNumberString(d.Minor), d.Permissions)
}

func (d *Device) SystemdCgroupPath() string {
sdType := "char"
if d.Type == 'b' {
sdType = "block"
} else if d.Type != 'c' {
// TODO: Invalid d.Type, do something about it.
return ""
}
// Start looking for wildcards, blocking a whole major.
if d.Minor == Wildcard {
return fmt.Sprintf("%s-%s", sdType, deviceNumberString(d.Major))
}
// Systemd uses /dev/char/x:y or /dev/block/x:y for devices by major/minor.
return fmt.Sprintf("/dev/%s/%s:%s", sdType, deviceNumberString(d.Major), deviceNumberString(d.Minor))
}

func (d *Device) Mkdev() int {
return int((d.Major << 8) | (d.Minor & 0xff) | ((d.Minor & 0xfff00) << 12))
}
Expand Down

0 comments on commit a347be1

Please sign in to comment.