From dd4c53eefb232ac1b6e1dda40c02e433da14c2be Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 3 Nov 2026 02:04:43 +0100 Subject: [PATCH] cpu_top by unit: program path only, pid reuse skipped, cgroup path kept under the root Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../disk-load/cmd/disk-load/cpu_process.go | 28 +++++++++++-------- modules/disk-load/cmd/disk-load/cpu_test.go | 19 +++++++++++-- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/modules/disk-load/cmd/disk-load/cpu_process.go b/modules/disk-load/cmd/disk-load/cpu_process.go index 86be864d..af6f722c 100644 --- a/modules/disk-load/cmd/disk-load/cpu_process.go +++ b/modules/disk-load/cmd/disk-load/cpu_process.go @@ -25,22 +25,22 @@ type ProcessCPU struct { // procTicks is a process's utime+stime ticks and thread count from its /proc//stat, whose comm // field may hold spaces and parentheses, so the fields are counted from the last ")". -func parseProcStat(s string) (comm string, ticks uint64, threads int, ok bool) { +func parseProcStat(s string) (comm string, ticks uint64, threads int, start string, ok bool) { l, r := strings.Index(s, "("), strings.LastIndex(s, ")") if l < 0 || r < l { - return "", 0, 0, false + return "", 0, 0, "", false } f := strings.Fields(s[r+1:]) - if len(f) < 18 { - return "", 0, 0, false + if len(f) < 20 { + return "", 0, 0, "", false } ut, e1 := strconv.ParseUint(f[11], 10, 64) st, e2 := strconv.ParseUint(f[12], 10, 64) th, e3 := strconv.Atoi(f[17]) if e1 != nil || e2 != nil || e3 != nil { - return "", 0, 0, false + return "", 0, 0, "", false } - return s[l+1 : r], ut + st, th, true + return s[l+1 : r], ut + st, th, f[19], true } // cgroupOfUnit is the cgroup path (relative to /sys/fs/cgroup) a unit name or a path names. @@ -48,6 +48,9 @@ func (m *Machine) cgroupOfUnit(unit string) (string, error) { base := m.path("/sys/fs/cgroup") if strings.Contains(unit, "/") { rel := strings.Trim(strings.TrimPrefix(unit, "/sys/fs/cgroup"), "/") + if strings.Contains(rel, "..") || filepath.Clean("/"+rel) != "/"+rel { + return "", fmt.Errorf("cgroup %q is not a path under /sys/fs/cgroup", unit) + } if rel == "" || !m.exists("/sys/fs/cgroup/"+rel) { return "", fmt.Errorf("no cgroup %q", unit) } @@ -75,6 +78,7 @@ type procReading struct { comm string threads int pid int + start string // starttime, to tell a reused pid } // unitProcs reads every process in a cgroup and its child cgroups: its ticks, name and threads. @@ -90,9 +94,9 @@ func (m *Machine) unitProcs(cgroup string) map[int]procReading { if err != nil { continue } - comm, ticks, th, ok := parseProcStat(m.read("/proc/" + f + "/stat")) + comm, ticks, th, start, ok := parseProcStat(m.read("/proc/" + f + "/stat")) if ok { - out[pid] = procReading{ticks: ticks, comm: comm, threads: th, pid: pid} + out[pid] = procReading{ticks: ticks, comm: comm, threads: th, pid: pid, start: start} } } return nil @@ -100,9 +104,11 @@ func (m *Machine) unitProcs(cgroup string) map[int]procReading { return out } -// commandLine is a process's command line, its NULs as spaces, cut to 120 characters. +// commandLine is a process's program path (argv[0], cut to 120 characters), never its arguments: +// they can carry a password. func (m *Machine) commandLine(pid int) string { - s := strings.TrimSpace(strings.ReplaceAll(m.read("/proc/"+strconv.Itoa(pid)+"/cmdline"), "\x00", " ")) + s, _, _ := strings.Cut(m.read("/proc/"+strconv.Itoa(pid)+"/cmdline"), "\x00") + s = strings.TrimSpace(s) if r := []rune(s); len(r) > 120 { s = string(r[:120]) } @@ -116,7 +122,7 @@ func (m *Machine) topProcessCPU(a, b map[int]procReading, window time.Duration, var sum float64 for pid, end := range b { start, ok := a[pid] - if !ok || end.ticks <= start.ticks { + if !ok || end.start != start.start || end.ticks <= start.ticks { continue } pct := float64(end.ticks-start.ticks) / clockTicks / window.Seconds() * 100 diff --git a/modules/disk-load/cmd/disk-load/cpu_test.go b/modules/disk-load/cmd/disk-load/cpu_test.go index aa47db13..07799355 100644 --- a/modules/disk-load/cmd/disk-load/cpu_test.go +++ b/modules/disk-load/cmd/disk-load/cpu_test.go @@ -191,16 +191,19 @@ func TestCPUTopRanksAUnitsProcesses(t *testing.T) { f.then("/proc/"+itoa(pid)+"/stat", itoa(pid)+" ("+comm+") S 1 1 1 0 -1 0 0 0 0 0 "+itoa(ticks)+" 0 0 0 20 0 "+itoa(threads)+" 0 1 1 1\n") } for pid, c := range map[int][2]string{10: {"firefox", "1"}, 11: {"sh (x)", "1"}, 12: {"gone", "1"}} { - _ = c f.file("/proc/"+itoa(pid)+"/stat", itoa(pid)+" ("+c[0]+") S 1 1 1 0 -1 0 0 0 0 0 0 0 0 0 20 0 "+c[1]+" 0 1 1 1\n") } f.file("/proc/13/stat", "13 (deep) S 1 1 1 0 -1 0 0 0 0 0 0 0 0 0 20 0 4 0 1 1 1\n") f.file("/proc/10/cmdline", "/usr/bin/firefox\x00--new-window\x00") + f.file("/proc/11/cmdline", "mysql\x00--password\x00hunter2\x00") st(10, "firefox", 150, 40) // 75 % of one CPU over 2 s st(11, "sh (x)", 40, 1) // 20 % st(13, "deep", 10, 4) // 5 %, in the child cgroup f.then("/sys/fs/cgroup/user.slice/session-c1.scope/cgroup.procs", "10\n11\n14\n") - st(14, "newborn", 500, 1) // not in the first reading: skipped + f.file("/sys/fs/cgroup/user.slice/session-c1.scope/app/cgroup.procs", "13\n15\n") + f.file("/proc/15/stat", "15 (old) S 1 1 1 0 -1 0 0 0 0 0 0 0 0 0 20 0 1 0 1 1 1\n") + f.then("/proc/15/stat", "15 (reused) S 1 1 1 0 -1 0 0 0 0 0 900 0 0 0 20 0 1 0 77 1 1\n") // new starttime: another process + st(14, "newborn", 500, 1) // not in the first reading: skipped out, err := tool(t, f.machine(), "cpu_top").Run(map[string]any{"unit": "session-c1.scope", "seconds": 2.0, "limit": 3.0}) if err != nil { @@ -208,9 +211,19 @@ func TestCPUTopRanksAUnitsProcesses(t *testing.T) { } top := out.(map[string]any)["top"].([]ProcessCPU) if len(top) != 3 || top[2].PID != 13 || top[2].CPUPct != 5 || top[0].PID != 10 || top[0].CPUPct != 75 || top[0].SharePct != 75 || top[0].Threads != 40 || - top[0].Command != "firefox: /usr/bin/firefox --new-window" || top[1].PID != 11 || top[1].CPUPct != 20 || top[1].Command != "sh (x)" { + top[0].Command != "firefox: /usr/bin/firefox" || top[1].PID != 11 || top[1].CPUPct != 20 || top[1].Command != "sh (x): mysql" { t.Errorf("processes of the unit: %+v", top) } + for _, p := range top { + if p.PID == 15 || strings.Contains(p.Command, "hunter2") { + t.Errorf("a reused pid was counted, or an argument leaked: %+v", p) + } + } + for _, u := range []string{"../../etc", "/sys/fs/cgroup/../../proc", "user.slice/../.."} { + if _, err := tool(t, f.machine(), "cpu_top").Run(map[string]any{"unit": u}); err == nil { + t.Errorf("cgroup path %q escaping the root was accepted", u) + } + } if _, err := tool(t, f.machine(), "cpu_top").Run(map[string]any{"unit": "nothing.scope"}); err == nil { t.Error("an unknown unit was accepted") }