Merge pull request #42001 from thaJeztah/20.10_backport_fix_cgroup_rule_panic

[20.10 backport] Fix daemon panic when starting container with invalid device cgroup rule
This commit is contained in:
Tibor Vass 2021-02-18 12:27:38 -08:00 committed by GitHub
commit 5bb85a962a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 174 additions and 1 deletions

View file

@ -8,6 +8,11 @@ import (
specs "github.com/opencontainers/runtime-spec/specs-go"
)
// TODO verify if this regex is correct for "a" (all); the docs (https://github.com/torvalds/linux/blob/v5.10/Documentation/admin-guide/cgroup-v1/devices.rst) describe:
// "'all' means it applies to all types and all major and minor numbers", and shows an example
// that *only* passes `a` as value: `echo a > /sys/fs/cgroup/1/devices.allow, which would be
// the "implicit" equivalent of "a *:* rwm". Source-code also looks to confirm this, and returns
// early for "a" (all); https://github.com/torvalds/linux/blob/v5.10/security/device_cgroup.c#L614-L642
// nolint: gosimple
var deviceCgroupRuleRegex = regexp.MustCompile("^([acb]) ([0-9]+|\\*):([0-9]+|\\*) ([rwm]{1,3})$")
@ -31,7 +36,7 @@ func SetCapabilities(s *specs.Spec, caplist []string) error {
func AppendDevicePermissionsFromCgroupRules(devPermissions []specs.LinuxDeviceCgroup, rules []string) ([]specs.LinuxDeviceCgroup, error) {
for _, deviceCgroupRule := range rules {
ss := deviceCgroupRuleRegex.FindAllStringSubmatch(deviceCgroupRule, -1)
if len(ss[0]) != 5 {
if len(ss) == 0 || len(ss[0]) != 5 {
return nil, fmt.Errorf("invalid device cgroup rule format: '%s'", deviceCgroupRule)
}
matches := ss[0]

168
oci/oci_test.go Normal file
View file

@ -0,0 +1,168 @@
package oci
import (
"testing"
"github.com/opencontainers/runtime-spec/specs-go"
"gotest.tools/v3/assert"
)
func TestAppendDevicePermissionsFromCgroupRules(t *testing.T) {
ptr := func(i int64) *int64 { return &i }
tests := []struct {
doc string
rule string
expected specs.LinuxDeviceCgroup
expectedErr string
}{
{
doc: "empty rule",
rule: "",
expectedErr: `invalid device cgroup rule format: ''`,
},
{
doc: "multiple spaces after first column",
rule: "c 1:1 rwm",
expectedErr: `invalid device cgroup rule format: 'c 1:1 rwm'`,
},
{
doc: "multiple spaces after second column",
rule: "c 1:1 rwm",
expectedErr: `invalid device cgroup rule format: 'c 1:1 rwm'`,
},
{
doc: "leading spaces",
rule: " c 1:1 rwm",
expectedErr: `invalid device cgroup rule format: ' c 1:1 rwm'`,
},
{
doc: "trailing spaces",
rule: "c 1:1 rwm ",
expectedErr: `invalid device cgroup rule format: 'c 1:1 rwm '`,
},
{
doc: "unknown device type",
rule: "z 1:1 rwm",
expectedErr: `invalid device cgroup rule format: 'z 1:1 rwm'`,
},
{
doc: "invalid device type",
rule: "zz 1:1 rwm",
expectedErr: `invalid device cgroup rule format: 'zz 1:1 rwm'`,
},
{
doc: "missing colon",
rule: "c 11 rwm",
expectedErr: `invalid device cgroup rule format: 'c 11 rwm'`,
},
{
doc: "invalid device major-minor",
rule: "c a:a rwm",
expectedErr: `invalid device cgroup rule format: 'c a:a rwm'`,
},
{
doc: "negative major device",
rule: "c -1:1 rwm",
expectedErr: `invalid device cgroup rule format: 'c -1:1 rwm'`,
},
{
doc: "negative minor device",
rule: "c 1:-1 rwm",
expectedErr: `invalid device cgroup rule format: 'c 1:-1 rwm'`,
},
{
doc: "missing permissions",
rule: "c 1:1",
expectedErr: `invalid device cgroup rule format: 'c 1:1'`,
},
{
doc: "invalid permissions",
rule: "c 1:1 x",
expectedErr: `invalid device cgroup rule format: 'c 1:1 x'`,
},
{
doc: "too many permissions",
rule: "c 1:1 rwmrwm",
expectedErr: `invalid device cgroup rule format: 'c 1:1 rwmrwm'`,
},
{
doc: "major out of range",
rule: "c 18446744073709551616:1 rwm",
expectedErr: `invalid major value in device cgroup rule format: 'c 18446744073709551616:1 rwm'`,
},
{
doc: "minor out of range",
rule: "c 1:18446744073709551616 rwm",
expectedErr: `invalid minor value in device cgroup rule format: 'c 1:18446744073709551616 rwm'`,
},
{
doc: "all (a) devices",
rule: "a 1:1 rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "a", Major: ptr(1), Minor: ptr(1), Access: "rwm"},
},
{
doc: "char (c) devices",
rule: "c 1:1 rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(1), Minor: ptr(1), Access: "rwm"},
},
{
doc: "block (b) devices",
rule: "b 1:1 rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "b", Major: ptr(1), Minor: ptr(1), Access: "rwm"},
},
{
doc: "char device with rwm permissions",
rule: "c 7:128 rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(7), Minor: ptr(128), Access: "rwm"},
},
{
doc: "wildcard major",
rule: "c *:1 rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(-1), Minor: ptr(1), Access: "rwm"},
},
{
doc: "wildcard minor",
rule: "c 1:* rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(1), Minor: ptr(-1), Access: "rwm"},
},
{
doc: "wildcard major and minor",
rule: "c *:* rwm",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(-1), Minor: ptr(-1), Access: "rwm"},
},
{
doc: "read (r) permission",
rule: "c 1:1 r",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(1), Minor: ptr(1), Access: "r"},
},
{
doc: "write (w) permission",
rule: "c 1:1 w",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(1), Minor: ptr(1), Access: "w"},
},
{
doc: "mknod (m) permission",
rule: "c 1:1 m",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(1), Minor: ptr(1), Access: "m"},
},
{
doc: "mknod (m) and read (r) permission",
rule: "c 1:1 mr",
expected: specs.LinuxDeviceCgroup{Allow: true, Type: "c", Major: ptr(1), Minor: ptr(1), Access: "mr"},
},
}
for _, tc := range tests {
tc := tc
t.Run(tc.doc, func(t *testing.T) {
out, err := AppendDevicePermissionsFromCgroupRules([]specs.LinuxDeviceCgroup{}, []string{tc.rule})
if tc.expectedErr != "" {
assert.Error(t, err, tc.expectedErr)
return
}
assert.NilError(t, err)
assert.DeepEqual(t, out, []specs.LinuxDeviceCgroup{tc.expected})
})
}
}