amadan

carlos / platform Public

zdd plan 2 task 2: per-child cleanup; stop evidence members walk the whole parent

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude e94519fc9c44019fc4bb47a77472ad5f52bf20cd parent b6fc393
4 files changed, +217 −15
  • internal/cgroup/cleanup_linux.go +115 −14
  • internal/cgroup/cleanup_linux_test.go +89 −0
  • internal/cgroup/cleanup_other.go +8 −0
  • internal/stopevidence/evidence.go +5 −1
diff --git a/internal/cgroup/cleanup_linux.go b/internal/cgroup/cleanup_linux.go
index b5ceee2db..fbef3fd17 100644
--- a/internal/cgroup/cleanup_linux.go
+++ b/internal/cgroup/cleanup_linux.go
@@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"os"
+ "slices"
"strconv"
"strings"
"time"
@@ -34,14 +35,26 @@ func cleanupFilesystem(root *os.File) error {
}
func (m *Manager) OpenCleanup(host string) (*Cleanup, error) {
- return m.openCleanup(host, cleanupFilesystem)
+ return m.openCleanupAt(host, "", cleanupFilesystem)
}
-func (m *Manager) openCleanup(host string, checkFS func(*os.File) error) (out *Cleanup, err error) {
+// OpenCleanupChild pins one child of host's generation parent (zero-
+// downtime spec §7.3): its KillAndEmpty writes that child's cgroup.kill
+// and proves that child empty, and never touches a sibling. ErrNoCgroup
+// when the child is not there.
+func (m *Manager) OpenCleanupChild(host, child string) (*Cleanup, error) {
+ return m.openCleanupAt(host, child, cleanupFilesystem)
+}
+
+func (m *Manager) openCleanup(host string, checkFS func(*os.File) error) (*Cleanup, error) {
+ return m.openCleanupAt(host, "", checkFS)
+}
+
+func (m *Manager) openCleanupAt(host, child string, checkFS func(*os.File) error) (out *Cleanup, err error) {
if m == nil || m.root == "" {
return nil, ErrNoDelegation
}
- if !controlname.ValidHostShape(host) {
+ if !controlname.ValidHostShape(host) || (child != "" && !isChildName(child)) {
return nil, ErrStrictRefused
}
root, err := os.OpenFile(m.root, os.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW, 0)
@@ -68,6 +81,18 @@ func (m *Manager) openCleanup(host string, checkFS func(*os.File) error) (out *C
if err != nil {
return nil, err
}
+ if child != "" {
+ // The child is pinned beneath the parent's own descriptor, and its
+ // identity is checked against that descriptor from now on.
+ p.entryParent, p.entryName = p.dir, child
+ p.dir, err = strictOpen(p.entryParent, child, unix.O_RDONLY|unix.O_DIRECTORY)
+ if errors.Is(err, unix.ENOENT) {
+ return nil, ErrNoCgroup
+ }
+ if err != nil {
+ return nil, err
+ }
+ }
p.identity, err = strictID(p.dir)
if err != nil {
return nil, err
@@ -125,32 +150,108 @@ func (c *Cleanup) Members() ([]int, error) {
return c.membersLocked()
}
+// Cleanup's walk is bounded: a tree deeper or wider than this is refused
+// rather than vouched for unread.
+const (
+ maxCleanupDepth = 8
+ maxCleanupGroups = 1024
+)
+
+// membersLocked lists every process in the pinned cgroup AND in every
+// cgroup below it. A generation parent holds no process itself (zero-
+// downtime spec §7.3), so stop evidence taken on it must read its
+// generations'. A flat tenant runs as the agent's uid and can make cgroups
+// of its own under its generation, so the walk goes to any depth, bounded.
func (c *Cleanup) membersLocked() ([]int, error) {
p := c.state.pin
if err := p.identityOK(); err != nil {
return nil, err
}
- if _, err := c.state.procs.Seek(0, 0); err != nil {
+ members, err := readProcs(c.state.procs)
+ if err != nil {
+ return nil, err
+ }
+ groups := 0
+ if err := walkMembers(p.dir, 1, &groups, &members); err != nil {
+ return nil, err
+ }
+ slices.Sort(members)
+ if err := p.identityOK(); err != nil {
+ return nil, err
+ }
+ return members, nil
+}
+
+// readProcs reads every PID in one cgroup.procs. It may exceed
+// strictRead's 4096-byte bound, so every line is scanned.
+func readProcs(f *os.File) ([]int, error) {
+ if _, err := f.Seek(0, 0); err != nil {
return nil, err
}
- // cgroup.procs may exceed strictRead's 4096-byte bound. Scan every line.
- scan := bufio.NewScanner(c.state.procs)
- var members []int
+ scan := bufio.NewScanner(f)
+ var pids []int
for scan.Scan() {
raw := strings.TrimSpace(scan.Text())
pid, err := strconv.Atoi(raw)
if err != nil || pid <= 0 {
return nil, fmt.Errorf("%w: invalid cgroup member", ErrStrictRefused)
}
- members = append(members, pid)
+ pids = append(pids, pid)
}
- if err := scan.Err(); err != nil {
- return nil, err
+ return pids, scan.Err()
+}
+
+// walkMembers appends the members of every cgroup below dir. A child that
+// vanishes between the listing and its open was removed, so it is empty.
+func walkMembers(dir *os.File, depth int, groups *int, members *[]int) error {
+ fd, err := unix.Openat(int(dir.Fd()), ".", unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
+ if err != nil {
+ return err
}
- if err := p.identityOK(); err != nil {
- return nil, err
+ list := os.NewFile(uintptr(fd), dir.Name())
+ entries, err := list.ReadDir(-1)
+ list.Close()
+ if err != nil {
+ return err
}
- return members, nil
+ for _, e := range entries {
+ if !e.IsDir() {
+ continue
+ }
+ if depth > maxCleanupDepth {
+ return fmt.Errorf("%w: cgroup tree deeper than %d", ErrStrictRefused, maxCleanupDepth)
+ }
+ if *groups++; *groups > maxCleanupGroups {
+ return fmt.Errorf("%w: more than %d cgroups below one host", ErrStrictRefused, maxCleanupGroups)
+ }
+ child, err := strictOpen(dir, e.Name(), unix.O_RDONLY|unix.O_DIRECTORY)
+ if errors.Is(err, unix.ENOENT) {
+ continue
+ }
+ if err != nil {
+ return err
+ }
+ procs, err := strictOpen(child, "cgroup.procs", unix.O_RDONLY)
+ if errors.Is(err, unix.ENOENT) {
+ child.Close()
+ continue
+ }
+ if err != nil {
+ child.Close()
+ return err
+ }
+ pids, err := readProcs(procs)
+ procs.Close()
+ if err == nil {
+ *members = append(*members, pids...)
+ err = walkMembers(child, depth+1, groups, members)
+ }
+ child.Close()
+ if err != nil {
+ return err
+ }
+ }
+ return nil
}
func (c *Cleanup) KillAndEmpty(ctx context.Context) error {
@@ -227,7 +328,7 @@ func (c *Cleanup) Close() error {
}
p.closed = true
var errs []error
- for _, f := range []*os.File{c.state.procs, p.events, p.kill, p.dir, p.root} {
+ for _, f := range []*os.File{c.state.procs, p.events, p.kill, p.dir, p.entryParent, p.root} {
if f != nil {
errs = append(errs, f.Close())
}
diff --git a/internal/cgroup/cleanup_linux_test.go b/internal/cgroup/cleanup_linux_test.go
index 8e774dcc8..429b7d02f 100644
--- a/internal/cgroup/cleanup_linux_test.go
+++ b/internal/cgroup/cleanup_linux_test.go
@@ -8,6 +8,7 @@ import (
"os"
"path/filepath"
"reflect"
+ "strconv"
"testing"
"time"
)
@@ -186,3 +187,91 @@ func TestCleanupRejectsMissingControlsAndSymlinks(t *testing.T) {
})
}
}
+
+func mkCgroupFixture(t *testing.T, dir, procs, events string) {
+ t.Helper()
+ if err := os.MkdirAll(dir, 0o700); err != nil {
+ t.Fatal(err)
+ }
+ for name, body := range map[string]string{"cgroup.events": events, "cgroup.procs": procs, "cgroup.kill": ""} {
+ if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ }
+}
+
+func fixtureCleanupAt(t *testing.T, root, host, child string) *Cleanup {
+ t.Helper()
+ c, err := (&Manager{root: root}).openCleanupAt(host, child, func(*os.File) error { return nil })
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { _ = c.Close() })
+ return c
+}
+
+// Stop evidence is taken on the parent (spec §7.3). A generation parent's
+// own cgroup.procs is always empty; its generations' are not.
+func TestCleanupMembersReachEveryGeneration(t *testing.T) {
+ host, root := "gens.example", t.TempDir()
+ parent := filepath.Join(root, ExecName(host))
+ mkCgroupFixture(t, parent, "", "populated 1\n")
+ mkCgroupFixture(t, filepath.Join(parent, "g1"), "11\n", "populated 1\n")
+ mkCgroupFixture(t, filepath.Join(parent, "g2"), "22\n", "populated 1\n")
+ // Review focus 3: a flat tenant runs as the agent's uid and can make
+ // cgroups of its own under its generation.
+ mkCgroupFixture(t, filepath.Join(parent, "g2", "tenant"), "33\n", "populated 1\n")
+ c := fixtureCleanupAt(t, root, host, "")
+ members, err := c.Members()
+ if err != nil || !reflect.DeepEqual(members, []int{11, 22, 33}) {
+ t.Fatalf("members = %v, %v; want every generation's", members, err)
+ }
+}
+
+func TestCleanupMembersRefuseATreeTooDeep(t *testing.T) {
+ host, root := "deep.example", t.TempDir()
+ dir := filepath.Join(root, ExecName(host))
+ mkCgroupFixture(t, dir, "", "populated 1\n")
+ for i := range maxCleanupDepth + 1 {
+ dir = filepath.Join(dir, "d"+strconv.Itoa(i))
+ mkCgroupFixture(t, dir, "", "populated 0\n")
+ }
+ c := fixtureCleanupAt(t, root, host, "")
+ if _, err := c.Members(); !errors.Is(err, ErrStrictRefused) {
+ t.Fatalf("a tree deeper than %d: %v, want ErrStrictRefused", maxCleanupDepth, err)
+ }
+}
+
+// KillAndEmpty on a generation acts only on that generation's child.
+func TestCleanupChildKillsOnlyItsGeneration(t *testing.T) {
+ host, root := "kill.example", t.TempDir()
+ parent := filepath.Join(root, ExecName(host))
+ mkCgroupFixture(t, parent, "", "populated 1\n")
+ mkCgroupFixture(t, filepath.Join(parent, "g1"), "11\n", "populated 1\n")
+ mkCgroupFixture(t, filepath.Join(parent, "g2"), "", "populated 0\n")
+ c := fixtureCleanupAt(t, root, host, "g2")
+ ctx, cancel := context.WithTimeout(t.Context(), time.Second)
+ defer cancel()
+ if err := c.KillAndEmpty(ctx); err != nil {
+ t.Fatal(err)
+ }
+ for dir, want := range map[string]string{filepath.Join(parent, "g2"): "1", filepath.Join(parent, "g1"): "", parent: ""} {
+ if got := readFixture(t, filepath.Join(dir, "cgroup.kill")); got != want {
+ t.Errorf("%s/cgroup.kill = %q, want %q", dir, got, want)
+ }
+ }
+}
+
+func TestOpenCleanupChildRefusesForeignNamesAndMissingChildren(t *testing.T) {
+ host, root := "names.example", t.TempDir()
+ mkCgroupFixture(t, filepath.Join(root, ExecName(host)), "", "populated 0\n")
+ m := &Manager{root: root}
+ for _, child := range []string{"child", "../x", "g0"} {
+ if _, err := m.openCleanupAt(host, child, func(*os.File) error { return nil }); !errors.Is(err, ErrStrictRefused) {
+ t.Errorf("child %q: %v, want ErrStrictRefused", child, err)
+ }
+ }
+ if _, err := m.openCleanupAt(host, "g9", func(*os.File) error { return nil }); !errors.Is(err, ErrNoCgroup) {
+ t.Errorf("missing g9: %v, want ErrNoCgroup", err)
+ }
+}
diff --git a/internal/cgroup/cleanup_other.go b/internal/cgroup/cleanup_other.go
index 6b7c33abf..c76077b61 100644
--- a/internal/cgroup/cleanup_other.go
+++ b/internal/cgroup/cleanup_other.go
@@ -12,6 +12,14 @@ func (m *Manager) OpenCleanup(string) (*Cleanup, error) {
}
return nil, ErrStrictUnsupported
}
+
+func (m *Manager) OpenCleanupChild(string, string) (*Cleanup, error) {
+ if m == nil || m.root == "" {
+ return nil, ErrNoDelegation
+ }
+ return nil, ErrStrictUnsupported
+}
+
func (*Cleanup) Populated() (bool, error) { return false, ErrStrictUnsupported }
func (*Cleanup) Members() ([]int, error) { return nil, ErrStrictUnsupported }
func (*Cleanup) KillAndEmpty(context.Context) error { return ErrStrictUnsupported }
diff --git a/internal/stopevidence/evidence.go b/internal/stopevidence/evidence.go
index 4c26a13c8..ede38012e 100644
--- a/internal/stopevidence/evidence.go
+++ b/internal/stopevidence/evidence.go
@@ -25,7 +25,11 @@ type Inputs struct {
Host string
Generation uint64
Fence *activator.StopEvidence
- Cgroup interface {
+ // Cgroup is the host's PARENT handle (cgroup.Manager.OpenCleanup), never
+ // one generation's child: under the generation layout the parent is
+ // empty only when every generation is (zero-downtime spec §7.3), and its
+ // Members walks every cgroup below it.
+ Cgroup interface {
Populated() (bool, error)
Members() ([]int, error)
}