From f8e876d7616469d07b8b049ecb48967eeb8fa7a5 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Tue, 4 Sep 2018 15:49:09 +0100 Subject: [PATCH] Fix denial of service with large numbers in cpuset-cpus and cpuset-mems Using a value such as `--cpuset-mems=1-9223372036854775807` would cause `dockerd` to run out of memory allocating a map of the values in the validation code. Set limits to the normal limit of the number of CPUs, and improve the error handling. Reported by Huawei PSIRT. Signed-off-by: Justin Cormack Signed-off-by: Sebastiaan van Stijn --- daemon/daemon_unix.go | 4 ++-- pkg/parsers/parsers.go | 28 ++++++++++++++++++++++++++++ pkg/parsers/parsers_test.go | 13 +++++++++++++ pkg/sysinfo/sysinfo.go | 12 ++++++++++-- 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 07f88786fa..b69eede21c 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -482,14 +482,14 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi } cpusAvailable, err := sysInfo.IsCpusetCpusAvailable(resources.CpusetCpus) if err != nil { - return warnings, fmt.Errorf("Invalid value %s for cpuset cpus", resources.CpusetCpus) + return warnings, errors.Wrapf(err, "Invalid value %s for cpuset cpus", resources.CpusetCpus) } if !cpusAvailable { return warnings, fmt.Errorf("Requested CPUs are not available - requested %s, available: %s", resources.CpusetCpus, sysInfo.Cpus) } memsAvailable, err := sysInfo.IsCpusetMemsAvailable(resources.CpusetMems) if err != nil { - return warnings, fmt.Errorf("Invalid value %s for cpuset mems", resources.CpusetMems) + return warnings, errors.Wrapf(err, "Invalid value %s for cpuset mems", resources.CpusetMems) } if !memsAvailable { return warnings, fmt.Errorf("Requested memory nodes are not available - requested %s, available: %s", resources.CpusetMems, sysInfo.Mems) diff --git a/pkg/parsers/parsers.go b/pkg/parsers/parsers.go index c4186a4c0a..068e524807 100644 --- a/pkg/parsers/parsers.go +++ b/pkg/parsers/parsers.go @@ -18,6 +18,24 @@ func ParseKeyValueOpt(opt string) (string, string, error) { return strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]), nil } +// ParseUintListMaximum parses and validates the specified string as the value +// found in some cgroup file (e.g. `cpuset.cpus`, `cpuset.mems`), which could be +// one of the formats below. Note that duplicates are actually allowed in the +// input string. It returns a `map[int]bool` with available elements from `val` +// set to `true`. Values larger than `maximum` cause an error if max is non zero, +// in order to stop the map becoming excessively large. +// Supported formats: +// 7 +// 1-6 +// 0,3-4,7,8-10 +// 0-0,0,1-7 +// 03,1-3 <- this is gonna get parsed as [1,2,3] +// 3,2,1 +// 0-2,3,1 +func ParseUintListMaximum(val string, maximum int) (map[int]bool, error) { + return parseUintList(val, maximum) +} + // ParseUintList parses and validates the specified string as the value // found in some cgroup file (e.g. `cpuset.cpus`, `cpuset.mems`), which could be // one of the formats below. Note that duplicates are actually allowed in the @@ -32,6 +50,10 @@ func ParseKeyValueOpt(opt string) (string, string, error) { // 3,2,1 // 0-2,3,1 func ParseUintList(val string) (map[int]bool, error) { + return parseUintList(val, 0) +} + +func parseUintList(val string, maximum int) (map[int]bool, error) { if val == "" { return map[int]bool{}, nil } @@ -46,6 +68,9 @@ func ParseUintList(val string) (map[int]bool, error) { if err != nil { return nil, errInvalidFormat } + if maximum != 0 && v > maximum { + return nil, fmt.Errorf("value of out range, maximum is %d", maximum) + } availableInts[v] = true } else { split := strings.SplitN(r, "-", 2) @@ -60,6 +85,9 @@ func ParseUintList(val string) (map[int]bool, error) { if max < min { return nil, errInvalidFormat } + if maximum != 0 && max > maximum { + return nil, fmt.Errorf("value of out range, maximum is %d", maximum) + } for i := min; i <= max; i++ { availableInts[i] = true } diff --git a/pkg/parsers/parsers_test.go b/pkg/parsers/parsers_test.go index a70093f1c4..12e5969091 100644 --- a/pkg/parsers/parsers_test.go +++ b/pkg/parsers/parsers_test.go @@ -68,3 +68,16 @@ func TestParseUintList(t *testing.T) { } } } + +func TestParseUintListMaximumLimits(t *testing.T) { + v := "10,1000" + if _, err := ParseUintListMaximum(v, 0); err != nil { + t.Fatalf("Expected not to fail, got %v", err) + } + if _, err := ParseUintListMaximum(v, 1000); err != nil { + t.Fatalf("Expected not to fail, got %v", err) + } + if out, err := ParseUintListMaximum(v, 100); err == nil { + t.Fatalf("Expected failure with %s but got %v", v, out) + } +} diff --git a/pkg/sysinfo/sysinfo.go b/pkg/sysinfo/sysinfo.go index 8fc0ecc25e..0f327d5068 100644 --- a/pkg/sysinfo/sysinfo.go +++ b/pkg/sysinfo/sysinfo.go @@ -117,11 +117,19 @@ func (c cgroupCpusetInfo) IsCpusetMemsAvailable(provided string) (bool, error) { } func isCpusetListAvailable(provided, available string) (bool, error) { - parsedProvided, err := parsers.ParseUintList(provided) + parsedAvailable, err := parsers.ParseUintList(available) if err != nil { return false, err } - parsedAvailable, err := parsers.ParseUintList(available) + // 8192 is the normal maximum number of CPUs in Linux, so accept numbers up to this + // or more if we actually have more CPUs. + max := 8192 + for m := range parsedAvailable { + if m > max { + max = m + } + } + parsedProvided, err := parsers.ParseUintListMaximum(provided, max) if err != nil { return false, err }