amadan

carlos / platform Public

zdd plan 2 task 3 fix: one child-pinning helper for role and generation children

Extracted pinChild helper to eliminate duplication between PrepareGen and
prepareRoleChild. The helper handles the security-sensitive fd operations
(FcntlInt, strictOpen, strictID, reservation, events/kill opening, population
check) while each caller handles its distinct steps (intent callback for role,
tier writes for generation).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude af809b0a60db437925eb6867c496a100380721f5 parent a0b4df3
2 files changed, +96 −84
  • internal/cgroup/role_linux.go +13 −47
  • internal/cgroup/strict_gen_linux.go +83 −37
diff --git a/internal/cgroup/role_linux.go b/internal/cgroup/role_linux.go
index ff5ded117..d753c2f42 100644
--- a/internal/cgroup/role_linux.go
+++ b/internal/cgroup/role_linux.go
@@ -4,8 +4,6 @@ package cgroup
import (
"errors"
- "os"
- "os/exec"
"github.com/carlosframework/platform/internal/controlname"
"golang.org/x/sys/unix"
@@ -54,61 +52,29 @@ func (p *Prepared) prepareRoleChild(role string, intent func(ContainmentIdentity
return nil, err
}
name := "role-" + role
- if intent != nil {
- var st unix.Stat_t
- if e := unix.Fstatat(int(p.dir.Fd()), name, &st, unix.AT_SYMLINK_NOFOLLOW); !errors.Is(e, unix.ENOENT) {
- return nil, ErrStrictRefused
- }
- if err = intent(ContainmentIdentity{Device: p.identity.device, Inode: p.identity.inode}); err != nil {
- return nil, err
+ beforeMkdir := func() error {
+ if intent != nil {
+ var st unix.Stat_t
+ if e := unix.Fstatat(int(p.dir.Fd()), name, &st, unix.AT_SYMLINK_NOFOLLOW); !errors.Is(e, unix.ENOENT) {
+ return ErrStrictRefused
+ }
+ if e := intent(ContainmentIdentity{Device: p.identity.device, Inode: p.identity.inode}); e != nil {
+ return e
+ }
}
+ return nil
}
- if err = unix.Mkdirat(int(p.dir.Fd()), name, 0755); err != nil && (intent != nil || !errors.Is(err, unix.EEXIST)) {
- return nil, ErrStrictRefused
+ child, err := pinChild(p, name, beforeMkdir)
+ if err != nil {
+ return nil, err
}
- child := &Prepared{strictPreparedState: &strictPreparedState{parent: p, host: p.host, limits: map[string]*os.File{}, expected: map[string]string{}, entryName: name, start: func(cmd *exec.Cmd) error { return cmd.Start() }}}
defer func() {
if err != nil {
child.closeFiles()
- }
- }()
- fd, e := unix.FcntlInt(p.dir.Fd(), unix.F_DUPFD_CLOEXEC, 0)
- if e != nil {
- return nil, ErrStrictUnsupported
- }
- child.entryParent = os.NewFile(uintptr(fd), name+"-parent")
- child.dir, e = strictOpen(p.dir, name, unix.O_RDONLY|unix.O_DIRECTORY)
- if e != nil {
- return nil, ErrStrictRefused
- }
- child.identity, e = strictID(child.dir)
- if e != nil {
- return nil, ErrStrictRefused
- }
- strictReservations.Lock()
- if strictReservations.held[child.identity] {
- strictReservations.Unlock()
- return nil, ErrStrictState
- }
- strictReservations.held[child.identity] = true
- strictReservations.Unlock()
- defer func() {
- if err != nil {
strictReservations.Lock()
delete(strictReservations.held, child.identity)
strictReservations.Unlock()
}
}()
- child.events, e = strictOpen(child.dir, "cgroup.events", unix.O_RDONLY)
- if e != nil {
- return nil, ErrStrictUnsupported
- }
- child.kill, e = strictOpen(child.dir, "cgroup.kill", unix.O_WRONLY)
- if e != nil {
- return nil, ErrStrictUnsupported
- }
- if populated, e := strictPopulation(child.events); e != nil || populated {
- return nil, ErrStrictRefused
- }
return child, nil
}
diff --git a/internal/cgroup/strict_gen_linux.go b/internal/cgroup/strict_gen_linux.go
index 56a392eed..006bcab5e 100644
--- a/internal/cgroup/strict_gen_linux.go
+++ b/internal/cgroup/strict_gen_linux.go
@@ -26,6 +26,81 @@ func tierExpected(l Limit, oomGroup string) map[string]string {
}
}
+// pinChild pins a child cgroup under parent: mkdir, create child struct, FcntlInt,
+// strictOpen (dir, events, kill), strictID, reservation, and population check.
+// beforeMkdir is called before mkdir (for role intent persistence; may be nil).
+// On success, returns a fully pinned, reserved child.
+// On error, all cleanup is done internally; caller needs only to set up a defer
+// for cleanup of any subsequent operations (e.g. tier writes for generation children).
+func pinChild(parent *Prepared, name string, beforeMkdir func() error) (*Prepared, error) {
+ if beforeMkdir != nil {
+ if err := beforeMkdir(); err != nil {
+ return nil, err
+ }
+ }
+ if err := unix.Mkdirat(int(parent.dir.Fd()), name, 0755); err != nil && !errors.Is(err, unix.EEXIST) {
+ return nil, ErrStrictRefused
+ }
+ child := &Prepared{strictPreparedState: &strictPreparedState{parent: parent, host: parent.host, limits: map[string]*os.File{}, expected: map[string]string{}, entryName: name, start: func(cmd *exec.Cmd) error { return cmd.Start() }}}
+
+ fd, e := unix.FcntlInt(parent.dir.Fd(), unix.F_DUPFD_CLOEXEC, 0)
+ if e != nil {
+ return nil, ErrStrictUnsupported
+ }
+ child.entryParent = os.NewFile(uintptr(fd), name+"-parent")
+
+ var err error
+ child.dir, err = strictOpen(parent.dir, name, unix.O_RDONLY|unix.O_DIRECTORY)
+ if err != nil {
+ child.closeFiles()
+ return nil, ErrStrictRefused
+ }
+
+ child.identity, err = strictID(child.dir)
+ if err != nil {
+ child.closeFiles()
+ return nil, ErrStrictRefused
+ }
+
+ strictReservations.Lock()
+ if strictReservations.held[child.identity] {
+ strictReservations.Unlock()
+ child.closeFiles()
+ return nil, ErrStrictState
+ }
+ strictReservations.held[child.identity] = true
+ strictReservations.Unlock()
+
+ child.events, err = strictOpen(child.dir, "cgroup.events", unix.O_RDONLY)
+ if err != nil {
+ child.closeFiles()
+ strictReservations.Lock()
+ delete(strictReservations.held, child.identity)
+ strictReservations.Unlock()
+ return nil, ErrStrictUnsupported
+ }
+
+ child.kill, err = strictOpen(child.dir, "cgroup.kill", unix.O_WRONLY)
+ if err != nil {
+ child.closeFiles()
+ strictReservations.Lock()
+ delete(strictReservations.held, child.identity)
+ strictReservations.Unlock()
+ return nil, ErrStrictUnsupported
+ }
+
+ populated, err := strictPopulation(child.events)
+ if err != nil || populated {
+ child.closeFiles()
+ strictReservations.Lock()
+ delete(strictReservations.held, child.identity)
+ strictReservations.Unlock()
+ return nil, ErrStrictRefused
+ }
+
+ return child, nil
+}
+
// delegateStrict delegates a generation parent's controllers to its
// children and proves memory is among them: a generation child without it
// could carry no limit.
@@ -85,50 +160,21 @@ func (p *Prepared) PrepareGen(n uint64, tier Limit) (out *Prepared, err error) {
return nil, err
}
name := GenChild(n)
- if err = unix.Mkdirat(int(p.dir.Fd()), name, 0755); err != nil && !errors.Is(err, unix.EEXIST) {
- return nil, ErrStrictRefused
+ child, err := pinChild(p, name, nil)
+ if err != nil {
+ return nil, err
}
- child := &Prepared{strictPreparedState: &strictPreparedState{parent: p, host: p.host, tier: tier, limits: map[string]*os.File{}, expected: map[string]string{}, entryName: name, start: func(cmd *exec.Cmd) error { return cmd.Start() }}}
- reserved := false
defer func() {
if err != nil {
child.closeFiles()
- if reserved {
- strictReservations.Lock()
- delete(strictReservations.held, child.identity)
- strictReservations.Unlock()
- }
+ strictReservations.Lock()
+ delete(strictReservations.held, child.identity)
+ strictReservations.Unlock()
}
}()
- fd, e := unix.FcntlInt(p.dir.Fd(), unix.F_DUPFD_CLOEXEC, 0)
- if e != nil {
- return nil, ErrStrictUnsupported
- }
- child.entryParent = os.NewFile(uintptr(fd), name+"-parent")
- if child.dir, e = strictOpen(p.dir, name, unix.O_RDONLY|unix.O_DIRECTORY); e != nil {
- return nil, ErrStrictRefused
- }
- if child.identity, e = strictID(child.dir); e != nil {
- return nil, ErrStrictRefused
- }
- strictReservations.Lock()
- if strictReservations.held[child.identity] {
- strictReservations.Unlock()
- return nil, ErrStrictState
- }
- strictReservations.held[child.identity] = true
- reserved = true
- strictReservations.Unlock()
- if child.events, e = strictOpen(child.dir, "cgroup.events", unix.O_RDONLY); e != nil {
- return nil, ErrStrictUnsupported
- }
- if child.kill, e = strictOpen(child.dir, "cgroup.kill", unix.O_WRONLY); e != nil {
- return nil, ErrStrictUnsupported
- }
- if populated, e := strictPopulation(child.events); e != nil || populated {
- return nil, ErrStrictRefused
- }
+ child.tier = tier
want := tierExpected(tier, "1")
+ var e error
for name := range want {
if child.limits[name], e = strictOpen(child.dir, name, unix.O_RDWR); e != nil {
return nil, ErrStrictUnsupported