Fix daemon panic when starting container with invalid device cgroup rule
This fixes a panic when an invalid "device cgroup rule" is passed, resulting in an "index out of range". This bug was introduced in the original implementation in1756af6faf
, but was not reproducible when using the CLI, because the same commit also added client-side validation on the flag before making an API request. The following example, uses an invalid rule (`c *:* rwm` - two spaces before the permissions); ```console $ docker run --rm --network=host --device-cgroup-rule='c *:* rwm' busybox invalid argument "c *:* rwm" for "--device-cgroup-rule" flag: invalid device cgroup format 'c *:* rwm' ``` Doing the same, but using the API results in a daemon panic when starting the container; Create a container with an invalid device cgroup rule: ```console curl -v \ --unix-socket /var/run/docker.sock \ "http://localhost/v1.41/containers/create?name=foobar" \ -H "Content-Type: application/json" \ -d '{"Image":"busybox:latest", "HostConfig":{"DeviceCgroupRules": ["c *:* rwm"]}}' ``` Start the container: ```console curl -v \ --unix-socket /var/run/docker.sock \ -X POST \ "http://localhost/v1.41/containers/foobar/start" ``` Observe the daemon logs: ``` 2021-01-22 12:53:03.313806 I | http: panic serving @: runtime error: index out of range [0] with length 0 goroutine 571 [running]: net/http.(*conn).serve.func1(0xc000cb2d20) /usr/local/go/src/net/http/server.go:1795 +0x13b panic(0x2f32380, 0xc000aebfc0) /usr/local/go/src/runtime/panic.go:679 +0x1b6 github.com/docker/docker/oci.AppendDevicePermissionsFromCgroupRules(0xc000175c00, 0x8, 0x8, 0xc0000bd380, 0x1, 0x4, 0x0, 0x0, 0xc0000e69c0, 0x0, ...) /go/src/github.com/docker/docker/oci/oci.go:34 +0x64f ``` This patch: - fixes the panic, allowing the daemon to return an error on container start - adds a unit-test to validate various permutations - adds a "todo" to verify the regular expression (and handling) of the "a" (all) value We should also consider performing this validation when _creating_ the container, so that an error is produced early. Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit5cc1753f2c
) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
parent
fae366b323
commit
1640d7b986
2 changed files with 174 additions and 1 deletions
|
@ -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
168
oci/oci_test.go
Normal 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})
|
||||
})
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue