diff --git a/exec.go b/exec.go index 16bbeebfbc2..e33b8d44ae0 100644 --- a/exec.go +++ b/exec.go @@ -117,6 +117,12 @@ following will output a list of processes running in the container: SkipArgReorder: true, } +// getSubCgroupPaths parses --cgroup arguments, which can either be +// - a single "path" argument (for cgroup v2); +// - one or more controller[,controller[,...]]:path arguments (for cgroup v1). +// +// Returns a controller to path map. For cgroup v2, it's a single entity map +// with empty controller value. func getSubCgroupPaths(args []string) (map[string]string, error) { if len(args) == 0 { return nil, nil @@ -124,20 +130,23 @@ func getSubCgroupPaths(args []string) (map[string]string, error) { paths := make(map[string]string, len(args)) for _, c := range args { // Split into controller:path. - cs := strings.SplitN(c, ":", 3) - if len(cs) > 2 { - return nil, fmt.Errorf("invalid --cgroup argument: %s", c) - } - if len(cs) == 1 { // no controller: prefix + if ctr, path, ok := strings.Cut(c, ":"); ok { + // There may be a few comma-separated controllers. + for _, ctrl := range strings.Split(ctr, ",") { + if ctrl == "" { + return nil, fmt.Errorf("invalid --cgroup argument: %s (empty prefix)", c) + } + if _, ok := paths[ctrl]; ok { + return nil, fmt.Errorf("invalid --cgroup argument(s): controller %s specified multiple times", ctrl) + } + paths[ctrl] = path + } + } else { + // No "controller:" prefix (cgroup v2, a single path). if len(args) != 1 { return nil, fmt.Errorf("invalid --cgroup argument: %s (missing : prefix)", c) } paths[""] = c - } else { - // There may be a few comma-separated controllers. - for _, ctrl := range strings.Split(cs[0], ",") { - paths[ctrl] = cs[1] - } } } return paths, nil diff --git a/libcontainer/cgroups/devices/systemd.go b/libcontainer/cgroups/devices/systemd.go index e1251c5ffb1..563a5e1954f 100644 --- a/libcontainer/cgroups/devices/systemd.go +++ b/libcontainer/cgroups/devices/systemd.go @@ -216,8 +216,7 @@ func findDeviceGroup(ruleType devices.Type, ruleMajor int64) (string, error) { continue } - group := strings.TrimPrefix(line, ruleMajorStr) - if len(group) < len(line) { // got it + if group, ok := strings.CutPrefix(line, ruleMajorStr); ok { return prefix + group, nil } } diff --git a/libcontainer/cgroups/file.go b/libcontainer/cgroups/file.go index 78c5bcf0d37..f37375f4ddb 100644 --- a/libcontainer/cgroups/file.go +++ b/libcontainer/cgroups/file.go @@ -151,8 +151,8 @@ func openFile(dir, file string, flags int) (*os.File, error) { if prepareOpenat2() != nil { return openFallback(path, flags, mode) } - relPath := strings.TrimPrefix(path, cgroupfsPrefix) - if len(relPath) == len(path) { // non-standard path, old system? + relPath, ok := strings.CutPrefix(path, cgroupfsPrefix) + if !ok { // Non-standard path, old system? return openFallback(path, flags, mode) } diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go index 88af8c568a2..53ffbb8e617 100644 --- a/libcontainer/cgroups/fs/cpuacct.go +++ b/libcontainer/cgroups/fs/cpuacct.go @@ -11,14 +11,7 @@ import ( ) const ( - cgroupCpuacctStat = "cpuacct.stat" - cgroupCpuacctUsageAll = "cpuacct.usage_all" - - nanosecondsInSecond = 1000000000 - - userModeColumn = 1 - kernelModeColumn = 2 - cuacctUsageAllColumnsNumber = 3 + nsInSec = 1000000000 // The value comes from `C.sysconf(C._SC_CLK_TCK)`, and // on Linux it's a constant which is safe to be hard coded, @@ -80,7 +73,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) { const ( userField = "user" systemField = "system" - file = cgroupCpuacctStat + file = "cpuacct.stat" ) // Expected format: @@ -102,7 +95,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) { return 0, 0, &parseError{Path: path, File: file, Err: err} } - return (userModeUsage * nanosecondsInSecond) / clockTicks, (kernelModeUsage * nanosecondsInSecond) / clockTicks, nil + return (userModeUsage * nsInSec) / clockTicks, (kernelModeUsage * nsInSec) / clockTicks, nil } func getPercpuUsage(path string) ([]uint64, error) { @@ -112,7 +105,6 @@ func getPercpuUsage(path string) ([]uint64, error) { if err != nil { return percpuUsage, err } - // TODO: use strings.SplitN instead. for _, value := range strings.Fields(data) { value, err := strconv.ParseUint(value, 10, 64) if err != nil { @@ -126,7 +118,7 @@ func getPercpuUsage(path string) ([]uint64, error) { func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) { usageKernelMode := []uint64{} usageUserMode := []uint64{} - const file = cgroupCpuacctUsageAll + const file = "cpuacct.usage_all" fd, err := cgroups.OpenFile(path, file, os.O_RDONLY) if os.IsNotExist(err) { @@ -140,22 +132,23 @@ func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) { scanner.Scan() // skipping header line for scanner.Scan() { - lineFields := strings.SplitN(scanner.Text(), " ", cuacctUsageAllColumnsNumber+1) - if len(lineFields) != cuacctUsageAllColumnsNumber { + // Each line is: cpu user system + fields := strings.SplitN(scanner.Text(), " ", 3) + if len(fields) != 3 { continue } - usageInKernelMode, err := strconv.ParseUint(lineFields[kernelModeColumn], 10, 64) + user, err := strconv.ParseUint(fields[1], 10, 64) if err != nil { return nil, nil, &parseError{Path: path, File: file, Err: err} } - usageKernelMode = append(usageKernelMode, usageInKernelMode) + usageUserMode = append(usageUserMode, user) - usageInUserMode, err := strconv.ParseUint(lineFields[userModeColumn], 10, 64) + kernel, err := strconv.ParseUint(fields[2], 10, 64) if err != nil { return nil, nil, &parseError{Path: path, File: file, Err: err} } - usageUserMode = append(usageUserMode, usageInUserMode) + usageKernelMode = append(usageKernelMode, kernel) } if err := scanner.Err(); err != nil { return nil, nil, &parseError{Path: path, File: file, Err: err} diff --git a/libcontainer/cgroups/fs/cpuacct_test.go b/libcontainer/cgroups/fs/cpuacct_test.go index d7d926d505c..116454e4881 100644 --- a/libcontainer/cgroups/fs/cpuacct_test.go +++ b/libcontainer/cgroups/fs/cpuacct_test.go @@ -53,8 +53,8 @@ func TestCpuacctStats(t *testing.T) { 962250696038415, 981956408513304, 1002658817529022, 994937703492523, 874843781648690, 872544369885276, 870104915696359, 870202363887496, }, - UsageInKernelmode: (uint64(291429664) * nanosecondsInSecond) / clockTicks, - UsageInUsermode: (uint64(452278264) * nanosecondsInSecond) / clockTicks, + UsageInKernelmode: (uint64(291429664) * nsInSec) / clockTicks, + UsageInUsermode: (uint64(452278264) * nsInSec) / clockTicks, } if !reflect.DeepEqual(expectedStats, actualStats.CpuStats.CpuUsage) { @@ -86,8 +86,8 @@ func TestCpuacctStatsWithoutUsageAll(t *testing.T) { }, PercpuUsageInKernelmode: []uint64{}, PercpuUsageInUsermode: []uint64{}, - UsageInKernelmode: (uint64(291429664) * nanosecondsInSecond) / clockTicks, - UsageInUsermode: (uint64(452278264) * nanosecondsInSecond) / clockTicks, + UsageInKernelmode: (uint64(291429664) * nsInSec) / clockTicks, + UsageInUsermode: (uint64(452278264) * nsInSec) / clockTicks, } if !reflect.DeepEqual(expectedStats, actualStats.CpuStats.CpuUsage) { diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index 4cf1f30ff6c..b51413ba811 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -48,26 +48,23 @@ func getCpusetStat(path string, file string) ([]uint16, error) { } for _, s := range strings.Split(fileContent, ",") { - sp := strings.SplitN(s, "-", 3) - switch len(sp) { - case 3: - return extracted, &parseError{Path: path, File: file, Err: errors.New("extra dash")} - case 2: - min, err := strconv.ParseUint(sp[0], 10, 16) + fromStr, toStr, ok := strings.Cut(s, "-") + if ok { + from, err := strconv.ParseUint(fromStr, 10, 16) if err != nil { return extracted, &parseError{Path: path, File: file, Err: err} } - max, err := strconv.ParseUint(sp[1], 10, 16) + to, err := strconv.ParseUint(toStr, 10, 16) if err != nil { return extracted, &parseError{Path: path, File: file, Err: err} } - if min > max { - return extracted, &parseError{Path: path, File: file, Err: errors.New("invalid values, min > max")} + if from > to { + return extracted, &parseError{Path: path, File: file, Err: errors.New("invalid values, from > to")} } - for i := min; i <= max; i++ { + for i := from; i <= to; i++ { extracted = append(extracted, uint16(i)) } - case 1: + } else { value, err := strconv.ParseUint(s, 10, 16) if err != nil { return extracted, &parseError{Path: path, File: file, Err: err} diff --git a/libcontainer/cgroups/fs2/defaultpath.go b/libcontainer/cgroups/fs2/defaultpath.go index 6637ef85c3c..6c44fd1972f 100644 --- a/libcontainer/cgroups/fs2/defaultpath.go +++ b/libcontainer/cgroups/fs2/defaultpath.go @@ -83,16 +83,9 @@ func parseCgroupFile(path string) (string, error) { func parseCgroupFromReader(r io.Reader) (string, error) { s := bufio.NewScanner(r) for s.Scan() { - var ( - text = s.Text() - parts = strings.SplitN(text, ":", 3) - ) - if len(parts) < 3 { - return "", fmt.Errorf("invalid cgroup entry: %q", text) - } - // text is like "0::/user.slice/user-1001.slice/session-1.scope" - if parts[0] == "0" && parts[1] == "" { - return parts[2], nil + // "0::/user.slice/user-1001.slice/session-1.scope" + if path, ok := strings.CutPrefix(s.Text(), "0::"); ok { + return path, nil } } if err := s.Err(); err != nil { diff --git a/libcontainer/cgroups/fs2/freezer.go b/libcontainer/cgroups/fs2/freezer.go index 7396ac02814..1f831500f23 100644 --- a/libcontainer/cgroups/fs2/freezer.go +++ b/libcontainer/cgroups/fs2/freezer.go @@ -104,9 +104,7 @@ func waitFrozen(dirPath string) (cgroups.FreezerState, error) { if i == maxIter { return cgroups.Undefined, fmt.Errorf("timeout of %s reached waiting for the cgroup to freeze", waitTime*maxIter) } - line := scanner.Text() - val := strings.TrimPrefix(line, "frozen ") - if val != line { // got prefix + if val, ok := strings.CutPrefix(scanner.Text(), "frozen "); ok { if val[0] == '1' { return cgroups.Frozen, nil } diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index 74dfcfdafcd..10e4a3ad80e 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -237,11 +237,10 @@ func (m *Manager) setUnified(res map[string]string) error { if errors.Is(err, os.ErrPermission) || errors.Is(err, os.ErrNotExist) { // Check if a controller is available, // to give more specific error if not. - sk := strings.SplitN(k, ".", 2) - if len(sk) != 2 { + c, _, ok := strings.Cut(k, ".") + if !ok { return fmt.Errorf("unified resource %q must be in the form CONTROLLER.PARAMETER", k) } - c := sk[0] if _, ok := m.controllers[c]; !ok && c != "cgroup" { return fmt.Errorf("unified resource %q can't be set: controller %q not available", k, c) } diff --git a/libcontainer/cgroups/fs2/psi.go b/libcontainer/cgroups/fs2/psi.go index 09f34888516..ac765affbaf 100644 --- a/libcontainer/cgroups/fs2/psi.go +++ b/libcontainer/cgroups/fs2/psi.go @@ -58,12 +58,12 @@ func statPSI(dirPath string, file string) (*cgroups.PSIStats, error) { func parsePSIData(psi []string) (cgroups.PSIData, error) { data := cgroups.PSIData{} for _, f := range psi { - kv := strings.SplitN(f, "=", 2) - if len(kv) != 2 { + key, val, ok := strings.Cut(f, "=") + if !ok { return data, fmt.Errorf("invalid psi data: %q", f) } var pv *float64 - switch kv[0] { + switch key { case "avg10": pv = &data.Avg10 case "avg60": @@ -71,16 +71,16 @@ func parsePSIData(psi []string) (cgroups.PSIData, error) { case "avg300": pv = &data.Avg300 case "total": - v, err := strconv.ParseUint(kv[1], 10, 64) + v, err := strconv.ParseUint(val, 10, 64) if err != nil { - return data, fmt.Errorf("invalid %s PSI value: %w", kv[0], err) + return data, fmt.Errorf("invalid %s PSI value: %w", key, err) } data.Total = v } if pv != nil { - v, err := strconv.ParseFloat(kv[1], 64) + v, err := strconv.ParseFloat(val, 64) if err != nil { - return data, fmt.Errorf("invalid %s PSI value: %w", kv[0], err) + return data, fmt.Errorf("invalid %s PSI value: %w", key, err) } *pv = v } diff --git a/libcontainer/cgroups/fscommon/rdma.go b/libcontainer/cgroups/fscommon/rdma.go index 93e05017cc7..70d92a00036 100644 --- a/libcontainer/cgroups/fscommon/rdma.go +++ b/libcontainer/cgroups/fscommon/rdma.go @@ -17,14 +17,12 @@ import ( func parseRdmaKV(raw string, entry *cgroups.RdmaEntry) error { var value uint32 - parts := strings.SplitN(raw, "=", 3) + k, v, ok := strings.Cut(raw, "=") - if len(parts) != 2 { + if !ok { return errors.New("Unable to parse RDMA entry") } - k, v := parts[0], parts[1] - if v == "max" { value = math.MaxUint32 } else { @@ -34,9 +32,10 @@ func parseRdmaKV(raw string, entry *cgroups.RdmaEntry) error { } value = uint32(val64) } - if k == "hca_handle" { + switch k { + case "hca_handle": entry.HcaHandles = value - } else if k == "hca_object" { + case "hca_object": entry.HcaObjects = value } diff --git a/libcontainer/cgroups/fscommon/utils.go b/libcontainer/cgroups/fscommon/utils.go index f4a51c9e56f..e11e038f25b 100644 --- a/libcontainer/cgroups/fscommon/utils.go +++ b/libcontainer/cgroups/fscommon/utils.go @@ -54,38 +54,39 @@ func ParseUint(s string, base, bitSize int) (uint64, error) { return value, nil } -// ParseKeyValue parses a space-separated "name value" kind of cgroup +// ParseKeyValue parses a space-separated "key value" kind of cgroup // parameter and returns its key as a string, and its value as uint64 -// (ParseUint is used to convert the value). For example, +// (using [ParseUint] to convert the value). For example, // "io_service_bytes 1234" will be returned as "io_service_bytes", 1234. func ParseKeyValue(t string) (string, uint64, error) { - parts := strings.SplitN(t, " ", 3) - if len(parts) != 2 { - return "", 0, fmt.Errorf("line %q is not in key value format", t) + key, val, ok := strings.Cut(t, " ") + if !ok || key == "" || val == "" { + return "", 0, fmt.Errorf(`line %q is not in "key value" format`, t) } - value, err := ParseUint(parts[1], 10, 64) + value, err := ParseUint(val, 10, 64) if err != nil { return "", 0, err } - return parts[0], value, nil + return key, value, nil } -// GetValueByKey reads a key-value pairs from the specified cgroup file, -// and returns a value of the specified key. ParseUint is used for value -// conversion. +// GetValueByKey reads space-separated "key value" pairs from the specified +// cgroup file, looking for a specified key, and returns its value as uint64, +// using [ParseUint] for conversion. If the value is not found, 0 is returned. func GetValueByKey(path, file, key string) (uint64, error) { content, err := cgroups.ReadFile(path, file) if err != nil { return 0, err } + key += " " lines := strings.Split(content, "\n") for _, line := range lines { - arr := strings.Split(line, " ") - if len(arr) == 2 && arr[0] == key { - val, err := ParseUint(arr[1], 10, 64) + v, ok := strings.CutPrefix(line, key) + if ok { + val, err := ParseUint(v, 10, 64) if err != nil { err = &ParseError{Path: path, File: file, Err: err} } @@ -103,7 +104,6 @@ func GetCgroupParamUint(path, file string) (uint64, error) { if err != nil { return 0, err } - contents = strings.TrimSpace(contents) if contents == "max" { return math.MaxUint64, nil } @@ -118,11 +118,10 @@ func GetCgroupParamUint(path, file string) (uint64, error) { // GetCgroupParamInt reads a single int64 value from specified cgroup file. // If the value read is "max", the math.MaxInt64 is returned. func GetCgroupParamInt(path, file string) (int64, error) { - contents, err := cgroups.ReadFile(path, file) + contents, err := GetCgroupParamString(path, file) if err != nil { return 0, err } - contents = strings.TrimSpace(contents) if contents == "max" { return math.MaxInt64, nil } diff --git a/libcontainer/cgroups/systemd/user.go b/libcontainer/cgroups/systemd/user.go index 1e18403bae1..4a4348e7070 100644 --- a/libcontainer/cgroups/systemd/user.go +++ b/libcontainer/cgroups/systemd/user.go @@ -61,8 +61,7 @@ func DetectUID() (int, error) { scanner := bufio.NewScanner(bytes.NewReader(b)) for scanner.Scan() { s := strings.TrimSpace(scanner.Text()) - if strings.HasPrefix(s, "OwnerUID=") { - uidStr := strings.TrimPrefix(s, "OwnerUID=") + if uidStr, ok := strings.CutPrefix(s, "OwnerUID="); ok { i, err := strconv.Atoi(uidStr) if err != nil { return -1, fmt.Errorf("could not detect the OwnerUID: %w", err) diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index d404647c8c0..9ef24b1ac6e 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -332,8 +332,8 @@ func getHugePageSizeFromFilenames(fileNames []string) ([]string, error) { for _, file := range fileNames { // example: hugepages-1048576kB - val := strings.TrimPrefix(file, "hugepages-") - if len(val) == len(file) { + val, ok := strings.CutPrefix(file, "hugepages-") + if !ok { // Unexpected file name: no prefix found, ignore it. continue } diff --git a/libcontainer/configs/validate/rootless.go b/libcontainer/configs/validate/rootless.go index 4507d4495e6..1d1295b216e 100644 --- a/libcontainer/configs/validate/rootless.go +++ b/libcontainer/configs/validate/rootless.go @@ -55,26 +55,27 @@ func rootlessEUIDMount(config *configs.Config) error { for _, mount := range config.Mounts { // Check that the options list doesn't contain any uid= or gid= entries // that don't resolve to root. + if !strings.Contains(mount.Data, "id=") { + continue + } for _, opt := range strings.Split(mount.Data, ",") { - if str := strings.TrimPrefix(opt, "uid="); len(str) < len(opt) { + if str, ok := strings.CutPrefix(opt, "uid="); ok { uid, err := strconv.Atoi(str) if err != nil { // Ignore unknown mount options. continue } if _, err := config.HostUID(uid); err != nil { - return fmt.Errorf("cannot specify uid=%d mount option for rootless container: %w", uid, err) + return fmt.Errorf("cannot specify %s mount option for rootless container: %w", opt, err) } - } - - if str := strings.TrimPrefix(opt, "gid="); len(str) < len(opt) { + } else if str, ok := strings.CutPrefix(opt, "gid="); ok { gid, err := strconv.Atoi(str) if err != nil { // Ignore unknown mount options. continue } if _, err := config.HostGID(gid); err != nil { - return fmt.Errorf("cannot specify gid=%d mount option for rootless container: %w", gid, err) + return fmt.Errorf("cannot specify %s mount option for rootless container: %w", opt, err) } } } diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 6e3e5bde371..bbee2d24f0f 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -685,8 +685,8 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) { var sp []systemdDbus.Property for k, v := range spec.Annotations { - name := strings.TrimPrefix(k, keyPrefix) - if len(name) == len(k) { // prefix not there + name, ok := strings.CutPrefix(k, keyPrefix) + if !ok { // prefix not there continue } if err := checkPropertyName(name); err != nil { diff --git a/libcontainer/utils/utils.go b/libcontainer/utils/utils.go index db420ea688d..d0243db870b 100644 --- a/libcontainer/utils/utils.go +++ b/libcontainer/utils/utils.go @@ -77,7 +77,7 @@ func stripRoot(root, path string) string { path = "/" case root == "/": // do nothing - case strings.HasPrefix(path, root+"/"): + default: path = strings.TrimPrefix(path, root+"/") } return CleanPath("/" + path) @@ -88,8 +88,8 @@ func stripRoot(root, path string) string { func SearchLabels(labels []string, key string) (string, bool) { key += "=" for _, s := range labels { - if strings.HasPrefix(s, key) { - return s[len(key):], true + if val, ok := strings.CutPrefix(s, key); ok { + return val, true } } return "", false