container: remove setup pipe helper
All checks were successful
Test / Create distribution (push) Successful in 1m20s
Test / Sandbox (push) Successful in 3m10s
Test / Hakurei (push) Successful in 4m24s
Test / ShareFS (push) Successful in 4m34s
Test / Sandbox (race detector) (push) Successful in 5m46s
Test / Hakurei (race detector) (push) Successful in 6m53s
Test / Flake checks (push) Successful in 1m30s

The API forces use of finalizer to close the read end of the setup pipe, which is no longer considered acceptable. Exporting this as part of package container also imposes unnecessary maintenance burden.

Signed-off-by: Ophestra <cat@gensokyo.uk>
This commit is contained in:
2026-04-07 15:50:59 +09:00
parent e4355279a1
commit 062edb3487
5 changed files with 50 additions and 114 deletions

View File

@@ -19,7 +19,6 @@ import (
"encoding/gob" "encoding/gob"
"errors" "errors"
"fmt" "fmt"
"io"
"log" "log"
"os" "os"
"os/exec" "os/exec"
@@ -470,13 +469,14 @@ func _main(s ...string) (exitCode int) {
os.NewFile(uintptr(C.fuse_session_fd(se)), "fuse"), os.NewFile(uintptr(C.fuse_session_fd(se)), "fuse"),
)) ))
var setupWriter io.WriteCloser var setupPipe [2]*os.File
if fd, w, err := container.Setup(&z.ExtraFiles); err != nil { if r, w, err := os.Pipe(); err != nil {
log.Println(err) log.Println(err)
return 5 return 5
} else { } else {
z.Args = append(z.Args, "-osetup="+strconv.Itoa(fd)) z.Args = append(z.Args, "-osetup="+strconv.Itoa(3+len(z.ExtraFiles)))
setupWriter = w z.ExtraFiles = append(z.ExtraFiles, r)
setupPipe[0], setupPipe[1] = r, w
} }
if err := z.Start(); err != nil { if err := z.Start(); err != nil {
@@ -487,6 +487,9 @@ func _main(s ...string) (exitCode int) {
} }
return 5 return 5
} }
if err := setupPipe[0].Close(); err != nil {
log.Println(err)
}
if err := z.Serve(); err != nil { if err := z.Serve(); err != nil {
if m, ok := message.GetMessage(err); ok { if m, ok := message.GetMessage(err); ok {
log.Println(m) log.Println(m)
@@ -496,10 +499,10 @@ func _main(s ...string) (exitCode int) {
return 5 return 5
} }
if err := gob.NewEncoder(setupWriter).Encode(&setup); err != nil { if err := gob.NewEncoder(setupPipe[1]).Encode(&setup); err != nil {
log.Println(err) log.Println(err)
return 5 return 5
} else if err = setupWriter.Close(); err != nil { } else if err = setupPipe[1].Close(); err != nil {
log.Println(err) log.Println(err)
} }

View File

@@ -53,7 +53,7 @@ type (
ExtraFiles []*os.File ExtraFiles []*os.File
// Write end of a pipe connected to the init to deliver [Params]. // Write end of a pipe connected to the init to deliver [Params].
setup *os.File setup [2]*os.File
// Cancels the context passed to the underlying cmd. // Cancels the context passed to the underlying cmd.
cancel context.CancelFunc cancel context.CancelFunc
// Closed after Wait returns. Keeps the spawning thread alive. // Closed after Wait returns. Keeps the spawning thread alive.
@@ -287,14 +287,16 @@ func (p *Container) Start() error {
} }
// place setup pipe before user supplied extra files, this is later restored by init // place setup pipe before user supplied extra files, this is later restored by init
if fd, f, err := Setup(&p.cmd.ExtraFiles); err != nil { if r, w, err := os.Pipe(); err != nil {
return &StartError{ return &StartError{
Fatal: true, Fatal: true,
Step: "set up params stream", Step: "set up params stream",
Err: err, Err: err,
} }
} else { } else {
p.setup = f fd := 3 + len(p.cmd.ExtraFiles)
p.cmd.ExtraFiles = append(p.cmd.ExtraFiles, r)
p.setup[0], p.setup[1] = r, w
p.cmd.Env = []string{setupEnv + "=" + strconv.Itoa(fd)} p.cmd.Env = []string{setupEnv + "=" + strconv.Itoa(fd)}
} }
p.cmd.ExtraFiles = append(p.cmd.ExtraFiles, p.ExtraFiles...) p.cmd.ExtraFiles = append(p.cmd.ExtraFiles, p.ExtraFiles...)
@@ -428,14 +430,30 @@ func (p *Container) Start() error {
// Serve serves [Container.Params] to the container init. // Serve serves [Container.Params] to the container init.
// //
// Serve must only be called once. // Serve must only be called once.
func (p *Container) Serve() error { func (p *Container) Serve() (err error) {
if p.setup == nil { if p.setup[0] == nil || p.setup[1] == nil {
panic("invalid serve") panic("invalid serve")
} }
defer func() {
if closeErr := p.setup[1].Close(); err == nil {
err = closeErr
}
setup := p.setup if err != nil {
p.setup = nil p.cancel()
if err := setup.SetDeadline(time.Now().Add(initSetupTimeout)); err != nil { }
p.setup[0], p.setup[1] = nil, nil
}()
if err = p.setup[0].Close(); err != nil {
return &StartError{
Fatal: true,
Step: "close read end of init pipe",
Err: err,
Passthrough: true,
}
}
if err = p.setup[1].SetDeadline(time.Now().Add(initSetupTimeout)); err != nil {
return &StartError{ return &StartError{
Fatal: true, Fatal: true,
Step: "set init pipe deadline", Step: "set init pipe deadline",
@@ -445,7 +463,6 @@ func (p *Container) Serve() error {
} }
if p.Path == nil { if p.Path == nil {
p.cancel()
return &StartError{ return &StartError{
Step: "invalid executable pathname", Step: "invalid executable pathname",
Err: EINVAL, Err: EINVAL,
@@ -461,18 +478,13 @@ func (p *Container) Serve() error {
p.SeccompRules = make([]std.NativeRule, 0) p.SeccompRules = make([]std.NativeRule, 0)
} }
err := gob.NewEncoder(setup).Encode(&initParams{ return gob.NewEncoder(p.setup[1]).Encode(&initParams{
p.Params, p.Params,
Getuid(), Getuid(),
Getgid(), Getgid(),
len(p.ExtraFiles), len(p.ExtraFiles),
p.msg.IsVerbose(), p.msg.IsVerbose(),
}) })
_ = setup.Close()
if err != nil {
p.cancel()
}
return err
} }
// Wait blocks until the container init process to exit and releases any // Wait blocks until the container init process to exit and releases any

View File

@@ -8,17 +8,6 @@ import (
"syscall" "syscall"
) )
// Setup appends the read end of a pipe for setup params transmission and returns its fd.
func Setup(extraFiles *[]*os.File) (int, *os.File, error) {
if r, w, err := os.Pipe(); err != nil {
return -1, nil, err
} else {
fd := 3 + len(*extraFiles)
*extraFiles = append(*extraFiles, r)
return fd, w, nil
}
}
var ( var (
ErrReceiveEnv = errors.New("environment variable not set") ErrReceiveEnv = errors.New("environment variable not set")
) )

View File

@@ -1,10 +1,8 @@
package container_test package container_test
import ( import (
"encoding/gob"
"errors" "errors"
"os" "os"
"slices"
"strconv" "strconv"
"syscall" "syscall"
"testing" "testing"
@@ -52,74 +50,4 @@ func TestSetupReceive(t *testing.T) {
t.Errorf("Receive: error = %v, want %v", err, syscall.EDOM) t.Errorf("Receive: error = %v, want %v", err, syscall.EDOM)
} }
}) })
t.Run("setup receive", func(t *testing.T) {
check := func(t *testing.T, useNilFdp bool) {
const key = "TEST_SETUP_RECEIVE"
payload := []uint64{syscall.MS_MGC_VAL, syscall.MS_MGC_MSK, syscall.MS_ASYNC, syscall.MS_ACTIVE}
encoderDone := make(chan error, 1)
extraFiles := make([]*os.File, 0, 1)
deadline, _ := t.Deadline()
if fd, f, err := container.Setup(&extraFiles); err != nil {
t.Fatalf("Setup: error = %v", err)
} else if fd != 3 {
t.Fatalf("Setup: fd = %d, want 3", fd)
} else {
if err = f.SetDeadline(deadline); err != nil {
t.Fatal(err.Error())
}
go func() { encoderDone <- gob.NewEncoder(f).Encode(payload) }()
}
if len(extraFiles) != 1 {
t.Fatalf("extraFiles: len = %v, want 1", len(extraFiles))
}
var dupFd int
if fd, err := syscall.Dup(int(extraFiles[0].Fd())); err != nil {
t.Fatalf("Dup: error = %v", err)
} else {
syscall.CloseOnExec(fd)
dupFd = fd
t.Setenv(key, strconv.Itoa(fd))
}
var (
gotPayload []uint64
fdp *uintptr
)
if !useNilFdp {
fdp = new(uintptr)
}
var closeFile func() error
if f, err := container.Receive(key, &gotPayload, fdp); err != nil {
t.Fatalf("Receive: error = %v", err)
} else {
closeFile = f
if !slices.Equal(payload, gotPayload) {
t.Errorf("Receive: %#v, want %#v", gotPayload, payload)
}
}
if !useNilFdp {
if int(*fdp) != dupFd {
t.Errorf("Fd: %d, want %d", *fdp, dupFd)
}
}
if err := <-encoderDone; err != nil {
t.Errorf("Encode: error = %v", err)
}
if closeFile != nil {
if err := closeFile(); err != nil {
t.Errorf("Close: error = %v", err)
}
}
}
t.Run("fp", func(t *testing.T) { check(t, false) })
t.Run("nil", func(t *testing.T) { check(t, true) })
})
} }

View File

@@ -13,7 +13,6 @@ import (
"time" "time"
"hakurei.app/check" "hakurei.app/check"
"hakurei.app/container"
"hakurei.app/fhs" "hakurei.app/fhs"
"hakurei.app/hst" "hakurei.app/hst"
"hakurei.app/internal/info" "hakurei.app/internal/info"
@@ -372,17 +371,18 @@ func (k *outcome) start(ctx context.Context, msg message.Msg,
// shim runs in the same session as monitor; see shim.go for behaviour // shim runs in the same session as monitor; see shim.go for behaviour
cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGCONT) } cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGCONT) }
var shimPipe *os.File var shimPipe [2]*os.File
if fd, w, err := container.Setup(&cmd.ExtraFiles); err != nil { if r, w, err := os.Pipe(); err != nil {
return cmd, nil, &hst.AppError{Step: "create shim setup pipe", Err: err} return cmd, nil, &hst.AppError{Step: "create shim setup pipe", Err: err}
} else { } else {
shimPipe = w
cmd.Env = []string{ cmd.Env = []string{
// passed through to shim by hsu // passed through to shim by hsu
shimEnv + "=" + strconv.Itoa(fd), shimEnv + "=" + strconv.Itoa(3+len(cmd.ExtraFiles)),
// interpreted by hsu // interpreted by hsu
"HAKUREI_IDENTITY=" + k.state.identity.String(), "HAKUREI_IDENTITY=" + k.state.identity.String(),
} }
cmd.ExtraFiles = append(cmd.ExtraFiles, r)
shimPipe[0], shimPipe[1] = r, w
} }
if len(k.supp) > 0 { if len(k.supp) > 0 {
@@ -393,12 +393,16 @@ func (k *outcome) start(ctx context.Context, msg message.Msg,
msg.Verbosef("setuid helper at %s", hsuPath) msg.Verbosef("setuid helper at %s", hsuPath)
if err := cmd.Start(); err != nil { if err := cmd.Start(); err != nil {
_, _ = shimPipe[0].Close(), shimPipe[1].Close()
msg.Resume() msg.Resume()
return cmd, shimPipe, &hst.AppError{Step: "start setuid wrapper", Err: err} return cmd, nil, &hst.AppError{Step: "start setuid wrapper", Err: err}
}
if err := shimPipe[0].Close(); err != nil {
msg.Verbose(err)
} }
*startTime = time.Now().UTC() *startTime = time.Now().UTC()
return cmd, shimPipe, nil return cmd, shimPipe[1], nil
} }
// serveShim serves outcomeState through the shim setup pipe. // serveShim serves outcomeState through the shim setup pipe.
@@ -411,11 +415,11 @@ func serveShim(msg message.Msg, shimPipe *os.File, state *outcomeState) error {
msg.Verbose(err.Error()) msg.Verbose(err.Error())
} }
if err := gob.NewEncoder(shimPipe).Encode(state); err != nil { if err := gob.NewEncoder(shimPipe).Encode(state); err != nil {
_ = shimPipe.Close()
msg.Resume() msg.Resume()
return &hst.AppError{Step: "transmit shim config", Err: err} return &hst.AppError{Step: "transmit shim config", Err: err}
} }
_ = shimPipe.Close() return shimPipe.Close()
return nil
} }
// printMessageError prints the error message according to [message.GetMessage], // printMessageError prints the error message according to [message.GetMessage],