amadan
cgroup: the shim refuses a NUL in the env, as Start does
cmd.Environ() silently drops an entry that contains a NUL, where exec.Cmd.Start refuses the command with "exec: environment variable contains NUL" (Go issue #56284). Through the shim, a tenant with a stray NUL in its file env started without that variable. startShimCore now refuses first, with Start's own error: for any NUL in cmd.Env, or, with Env nil, in Dir (the PWD Start adds). It is the command's error, not a placement error, and nothing starts or is registered.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+79
−0
internal/cgroup/shim_linux.go+22 −0internal/cgroup/shim_linux_test.go+57 −0
diff --git a/internal/cgroup/shim_linux.go b/internal/cgroup/shim_linux.go| index 491bb7f3a..3fac628a9 100644 |
| --- a/internal/cgroup/shim_linux.go |
| +++ b/internal/cgroup/shim_linux.go |
| @@ -74,6 +74,21 @@ func (m *Manager) startViaShim(dir string, cmd *exec.Cmd, before func() error) ( |
| return m.startShimCore(dir, dirFile, "/"+rel, cmd, before) |
| } |
| +// envHasNUL reports whether Start would refuse cmd's environment for a NUL. |
| +// With Env nil, Start uses os.Environ (which cannot hold a NUL) plus |
| +// PWD=Dir made absolute, so a NUL in Dir is the only way in. |
| +func envHasNUL(cmd *exec.Cmd) bool { |
| + if cmd.Env == nil { |
| + return cmd.Dir != "" && strings.IndexByte(cmd.Dir, 0) >= 0 |
| + } |
| + for _, kv := range cmd.Env { |
| + if strings.IndexByte(kv, 0) >= 0 { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| // startShimCore is startViaShim after its checks: dirFile is dir opened, |
| // and expected is dir as /proc/self/cgroup will show it. It closes dirFile. |
| func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, cmd *exec.Cmd, before func() error) (refused bool, err error) { |
| @@ -86,6 +101,13 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| } |
| wd := cmd.Dir |
| keep := 3 + len(cmd.ExtraFiles) |
| + // cmd.Environ() silently drops an entry that contains a NUL, where |
| + // Start refuses the command (Go issue #56284). Refuse it the same way, |
| + // with Start's own error, before anything starts or is registered. |
| + if envHasNUL(cmd) { |
| + dirFile.Close() |
| + return false, errors.New("exec: environment variable contains NUL") |
| + } |
| spec, err := encodeShimSpec(shimSpec{ |
| Path: path, Dir: wd, Argv: argv, Env: cmd.Environ(), |
| Expected: expected, KeepFDs: keep, TestHold: shimTestHold, |
diff --git a/internal/cgroup/shim_linux_test.go b/internal/cgroup/shim_linux_test.go| index 3bfedeabd..32f3b5613 100644 |
| --- a/internal/cgroup/shim_linux_test.go |
| +++ b/internal/cgroup/shim_linux_test.go |
| @@ -150,6 +150,63 @@ func TestShimBeforeRefusalRegistersNothing(t *testing.T) { |
| } |
| } |
| +// TestShimRefusesNULInEnv: Start refuses an environment entry that holds a |
| +// NUL (Go issue #56284), where cmd.Environ() would silently drop it. The |
| +// core answers Start's own error, not a placement error, and starts and |
| +// registers nothing. With Env nil the NUL can arrive only through Dir, as |
| +// the PWD Start adds. |
| +func TestShimRefusesNULInEnv(t *testing.T) { |
| + const want = "exec: environment variable contains NUL" |
| + for _, c := range []struct { |
| + name string |
| + env []string |
| + dir string |
| + }{ |
| + {"env entry", []string{"A=1", "BAD=x\x00y", "B=2"}, ""}, |
| + {"env key", []string{"B\x00AD=x"}, ""}, |
| + {"PWD from dir", nil, "/no\x00where"}, |
| + } { |
| + t.Run(c.name, func(t *testing.T) { |
| + marker := filepath.Join(t.TempDir(), "ran") |
| + newCmd := func() *exec.Cmd { |
| + cmd := exec.Command("/bin/sh", "-c", "touch "+marker) |
| + cmd.Env = c.env |
| + cmd.Dir = c.dir |
| + return cmd |
| + } |
| + // The control: a direct start answers exactly this. |
| + direct := newCmd() |
| + if err := direct.Start(); err == nil || err.Error() != want { |
| + t.Fatalf("direct Start: err=%v, want %q", err, want) |
| + } |
| + |
| + m := &Manager{root: t.TempDir(), logf: t.Logf} |
| + f, err := os.Open(m.root) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer f.Close() |
| + cmd := newCmd() |
| + refused, err := m.startShimCore(m.root, f, "/nowhere", cmd, nil) |
| + if refused || err == nil || err.Error() != want || isPlacementError(err) { |
| + t.Fatalf("refused=%v err=%v, want the command's error %q", refused, err, want) |
| + } |
| + if cmd.Process != nil { |
| + t.Fatal("a process was started") |
| + } |
| + if cmd.Path != "/bin/sh" { |
| + t.Fatalf("cmd was rewritten to %s", cmd.Path) |
| + } |
| + if _, err := os.Stat(marker); err == nil { |
| + t.Fatal("tenant code ran") |
| + } |
| + if m.placementsUnder(m.root) != 0 { |
| + t.Fatal("a placement was registered") |
| + } |
| + }) |
| + } |
| +} |
| + |
| func TestShimStatusClassification(t *testing.T) { |
| for _, c := range []struct { |
| in string |