Skip to content

Commit 24388be

Browse files
cypharcyphar
committed
configs: use different types for .Devices and .Resources.Devices
Making them the same type is simply confusing, but also means that you could accidentally use one in the wrong context. This eliminates that problem. This also includes a whole bunch of cleanups for the types within DeviceRule, so that they can be used more ergonomically. Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 60e21ec commit 24388be

10 files changed

Lines changed: 327 additions & 206 deletions

File tree

‎libcontainer/cgroups/ebpf/devicefilter/devicefilter.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const (
2222
)
2323

2424
// DeviceFilter returns eBPF device filter program and its license string
25-
func DeviceFilter(devices []*configs.Device) (asm.Instructions, string, error) {
25+
func DeviceFilter(devices []*configs.DeviceRule) (asm.Instructions, string, error) {
2626
p := &program{}
2727
p.init()
2828
for i := len(devices) - 1; i >= 0; i-- {
@@ -68,7 +68,7 @@ func (p *program) init() {
6868
}
6969

7070
// appendDevice needs to be called from the last element of OCI linux.resources.devices to the head element.
71-
func (p *program) appendDevice(dev *configs.Device) error {
71+
func (p *program) appendDevice(dev *configs.DeviceRule) error {
7272
if p.blockID < 0 {
7373
return errors.New("the program is finalized")
7474
}

‎libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func hash(s, comm string) string {
2020
return strings.Join(res, "\n")
2121
}
2222

23-
func testDeviceFilter(t testing.TB, devices []*configs.Device, expectedStr string) {
23+
func testDeviceFilter(t testing.TB, devices []*configs.DeviceRule, expectedStr string) {
2424
insts, _, err := DeviceFilter(devices)
2525
if err != nil {
2626
t.Fatalf("%s: %v (devices: %+v)", t.Name(), err, devices)
@@ -137,11 +137,15 @@ block-11:
137137
62: Mov32Imm dst: r0 imm: 0
138138
63: Exit
139139
`
140-
testDeviceFilter(t, specconv.AllowedDevices, expected)
140+
var devices []*configs.DeviceRule
141+
for _, device := range specconv.AllowedDevices {
142+
devices = append(devices, &device.DeviceRule)
143+
}
144+
testDeviceFilter(t, devices, expected)
141145
}
142146

143147
func TestDeviceFilter_Privileged(t *testing.T) {
144-
devices := []*configs.Device{
148+
devices := []*configs.DeviceRule{
145149
{
146150
Type: 'a',
147151
Major: -1,
@@ -168,7 +172,7 @@ block-0:
168172
}
169173

170174
func TestDeviceFilter_PrivilegedExceptSingleDevice(t *testing.T) {
171-
devices := []*configs.Device{
175+
devices := []*configs.DeviceRule{
172176
{
173177
Type: 'a',
174178
Major: -1,
@@ -208,7 +212,7 @@ block-1:
208212
}
209213

210214
func TestDeviceFilter_Weird(t *testing.T) {
211-
devices := []*configs.Device{
215+
devices := []*configs.DeviceRule{
212216
{
213217
Type: 'b',
214218
Major: 8,

‎libcontainer/cgroups/fs/devices_test.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,12 @@ func TestDevicesSetAllow(t *testing.T) {
1818
"devices.deny": "",
1919
})
2020

21-
helper.CgroupData.config.Resources.Devices = []*configs.Device{
21+
helper.CgroupData.config.Resources.Devices = []*configs.DeviceRule{
2222
{
23-
Path: "/dev/zero",
24-
Type: 'c',
23+
Type: configs.CharDevice,
2524
Major: 1,
2625
Minor: 5,
27-
Permissions: "rwm",
28-
FileMode: 0666,
26+
Permissions: configs.DevicePermissions("rwm"),
2927
Allow: true,
3028
},
3129
}

‎libcontainer/cgroups/fs2/devices.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ import (
1010
"golang.org/x/sys/unix"
1111
)
1212

13-
func isRWM(cgroupPermissions string) bool {
14-
r := false
15-
w := false
16-
m := false
17-
for _, rn := range cgroupPermissions {
18-
switch rn {
13+
func isRWM(perms configs.DevicePermissions) bool {
14+
var r, w, m bool
15+
for _, perm := range perms {
16+
switch perm {
1917
case 'r':
2018
r = true
2119
case 'w':

‎libcontainer/configs/cgroup_linux.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ type Cgroup struct {
4242

4343
type Resources struct {
4444
// Devices is the set of access rules for devices in the container.
45-
Devices []*Device `json:"devices"`
45+
Devices []*DeviceRule `json:"devices"`
4646

4747
// Memory limit (in bytes)
4848
Memory int64 `json:"memory"`

‎libcontainer/configs/device.go‎

Lines changed: 147 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package configs
22

33
import (
4+
"errors"
45
"fmt"
56
"os"
7+
"strconv"
8+
9+
"golang.org/x/sys/unix"
610
)
711

812
const (
@@ -12,21 +16,11 @@ const (
1216
// TODO Windows: This can be factored out in the future
1317

1418
type Device struct {
15-
// Device type, block, char, etc.
16-
Type rune `json:"type"`
19+
DeviceRule
1720

1821
// Path to the device.
1922
Path string `json:"path"`
2023

21-
// Major is the device's major number.
22-
Major int64 `json:"major"`
23-
24-
// Minor is the device's minor number.
25-
Minor int64 `json:"minor"`
26-
27-
// Cgroup permissions format, rwm.
28-
Permissions string `json:"permissions"`
29-
3024
// FileMode permission bits for the device.
3125
FileMode os.FileMode `json:"file_mode"`
3226

@@ -35,23 +29,154 @@ type Device struct {
3529

3630
// Gid of the device.
3731
Gid uint32 `json:"gid"`
32+
}
3833

39-
// Write the file to the allowed list
40-
Allow bool `json:"allow"`
34+
// DevicePermissions is a cgroupv1-style string to represent device access. It
35+
// has to be a string for backward compatibility reasons, hence why it has
36+
// methods to do set operations.
37+
type DevicePermissions string
38+
39+
const (
40+
deviceRead uint = (1 << iota)
41+
deviceWrite
42+
deviceMknod
43+
)
44+
45+
func (p DevicePermissions) toSet() uint {
46+
var set uint
47+
for _, perm := range p {
48+
switch perm {
49+
case 'r':
50+
set |= deviceRead
51+
case 'w':
52+
set |= deviceWrite
53+
case 'm':
54+
set |= deviceMknod
55+
}
56+
}
57+
return set
58+
}
59+
60+
func fromSet(set uint) DevicePermissions {
61+
var perm string
62+
if set&deviceRead == deviceRead {
63+
perm += "r"
64+
}
65+
if set&deviceWrite == deviceWrite {
66+
perm += "w"
67+
}
68+
if set&deviceMknod == deviceMknod {
69+
perm += "m"
70+
}
71+
return DevicePermissions(perm)
72+
}
73+
74+
// Union returns the union of the two sets of DevicePermissions.
75+
func (p DevicePermissions) Union(o DevicePermissions) DevicePermissions {
76+
lhs := p.toSet()
77+
rhs := o.toSet()
78+
return fromSet(lhs | rhs)
79+
}
80+
81+
// Difference returns the set difference of the two sets of DevicePermissions.
82+
// In set notation, A.Difference(B) gives you A\B.
83+
func (p DevicePermissions) Difference(o DevicePermissions) DevicePermissions {
84+
lhs := p.toSet()
85+
rhs := o.toSet()
86+
return fromSet(lhs &^ rhs)
87+
}
88+
89+
// Intersection computes the intersection of the two sets of DevicePermissions.
90+
func (p DevicePermissions) Intersection(o DevicePermissions) DevicePermissions {
91+
lhs := p.toSet()
92+
rhs := o.toSet()
93+
return fromSet(lhs & rhs)
94+
}
95+
96+
// IsEmpty returns whether the set of permissions in a DevicePermissions is
97+
// empty.
98+
func (p DevicePermissions) IsEmpty() bool {
99+
return p == DevicePermissions("")
100+
}
101+
102+
// IsValid returns whether the set of permissions is a subset of valid
103+
// permissions (namely, {r,w,m}).
104+
func (p DevicePermissions) IsValid() bool {
105+
return p == fromSet(p.toSet())
106+
}
107+
108+
type DeviceType rune
109+
110+
const (
111+
WildcardDevice DeviceType = 'a'
112+
BlockDevice DeviceType = 'b'
113+
CharDevice DeviceType = 'c' // or 'u'
114+
FifoDevice DeviceType = 'p'
115+
)
116+
117+
func (t DeviceType) IsValid() bool {
118+
switch t {
119+
case WildcardDevice, BlockDevice, CharDevice, FifoDevice:
120+
return true
121+
default:
122+
return false
123+
}
41124
}
42125

43-
func (d *Device) CgroupString() string {
44-
return fmt.Sprintf("%c %s:%s %s", d.Type, deviceNumberString(d.Major), deviceNumberString(d.Minor), d.Permissions)
126+
func (t DeviceType) CanMknod() bool {
127+
switch t {
128+
case BlockDevice, CharDevice, FifoDevice:
129+
return true
130+
default:
131+
return false
132+
}
133+
}
134+
135+
func (t DeviceType) CanCgroup() bool {
136+
switch t {
137+
case WildcardDevice, BlockDevice, CharDevice:
138+
return true
139+
default:
140+
return false
141+
}
45142
}
46143

47-
func (d *Device) Mkdev() int {
48-
return int((d.Major << 8) | (d.Minor & 0xff) | ((d.Minor & 0xfff00) << 12))
144+
type DeviceRule struct {
145+
// Type of device ('c' for char, 'b' for block). If set to 'a', this rule
146+
// acts as a wildcard and all fields other than Allow are ignored.
147+
Type DeviceType `json:"type"`
148+
149+
// Major is the device's major number.
150+
Major int64 `json:"major"`
151+
152+
// Minor is the device's minor number.
153+
Minor int64 `json:"minor"`
154+
155+
// Permissions is the set of permissions that this rule applies to (in the
156+
// cgroupv1 format -- any combination of "rwm").
157+
Permissions DevicePermissions `json:"permissions"`
158+
159+
// Allow specifies whether this rule is allowed.
160+
Allow bool `json:"allow"`
161+
}
162+
163+
func (d *DeviceRule) CgroupString() string {
164+
var (
165+
major = strconv.FormatInt(d.Major, 10)
166+
minor = strconv.FormatInt(d.Minor, 10)
167+
)
168+
if d.Major == Wildcard {
169+
major = "*"
170+
}
171+
if d.Minor == Wildcard {
172+
minor = "*"
173+
}
174+
return fmt.Sprintf("%c %s:%s %s", d.Type, major, minor, d.Permissions)
49175
}
50176

51-
// deviceNumberString converts the device number to a string return result.
52-
func deviceNumberString(number int64) string {
53-
if number == Wildcard {
54-
return "*"
177+
func (d *DeviceRule) Mkdev() (uint64, error) {
178+
if d.Major == Wildcard || d.Minor == Wildcard {
179+
return 0, errors.New("cannot mkdev() device with wildcards")
55180
}
56-
return fmt.Sprint(number)
181+
return unix.Mkdev(uint32(d.Major), uint32(d.Minor)), nil
57182
}

‎libcontainer/devices/devices.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,33 +31,33 @@ func DeviceFromPath(path, permissions string) (*configs.Device, error) {
3131
}
3232

3333
var (
34+
devType configs.DeviceType
35+
mode = stat.Mode
3436
devNumber = uint64(stat.Rdev)
3537
major = unix.Major(devNumber)
3638
minor = unix.Minor(devNumber)
3739
)
38-
if major == 0 {
39-
return nil, ErrNotADevice
40-
}
41-
42-
var (
43-
devType rune
44-
mode = stat.Mode
45-
)
4640
switch {
4741
case mode&unix.S_IFBLK == unix.S_IFBLK:
48-
devType = 'b'
42+
devType = configs.BlockDevice
4943
case mode&unix.S_IFCHR == unix.S_IFCHR:
50-
devType = 'c'
44+
devType = configs.CharDevice
45+
case mode&unix.S_IFIFO == unix.S_IFIFO:
46+
devType = configs.FifoDevice
47+
default:
48+
return nil, ErrNotADevice
5149
}
5250
return &configs.Device{
53-
Type: devType,
54-
Path: path,
55-
Major: int64(major),
56-
Minor: int64(minor),
57-
Permissions: permissions,
58-
FileMode: os.FileMode(mode),
59-
Uid: stat.Uid,
60-
Gid: stat.Gid,
51+
DeviceRule: configs.DeviceRule{
52+
Type: devType,
53+
Major: int64(major),
54+
Minor: int64(minor),
55+
Permissions: configs.DevicePermissions(permissions),
56+
},
57+
Path: path,
58+
FileMode: os.FileMode(mode),
59+
Uid: stat.Uid,
60+
Gid: stat.Gid,
6161
}, nil
6262
}
6363

‎libcontainer/integration/template_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ const defaultMountFlags = unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NODEV
2121
// it uses a network strategy of just setting a loopback interface
2222
// and the default setup for devices
2323
func newTemplateConfig(rootfs string) *configs.Config {
24+
var allowedDevices []*configs.DeviceRule
25+
for _, device := range specconv.AllowedDevices {
26+
allowedDevices = append(allowedDevices, &device.DeviceRule)
27+
}
2428
return &configs.Config{
2529
Rootfs: rootfs,
2630
Capabilities: &configs.Capabilities{
@@ -116,7 +120,7 @@ func newTemplateConfig(rootfs string) *configs.Config {
116120
Path: "integration/test",
117121
Resources: &configs.Resources{
118122
MemorySwappiness: nil,
119-
Devices: specconv.AllowedDevices,
123+
Devices: allowedDevices,
120124
},
121125
},
122126
MaskPaths: []string{

0 commit comments

Comments
 (0)