Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init: use *os.File for passed file descriptors #4175

Merged
merged 1 commit into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions libcontainer/init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,26 @@ func startInitialization() (retErr error) {
logrus.SetLevel(logrus.Level(logLevel))
}

logFD, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_LOGPIPE"))
logFd, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_LOGPIPE"))
if err != nil {
return fmt.Errorf("unable to convert _LIBCONTAINER_LOGPIPE: %w", err)
}
logPipe := os.NewFile(uintptr(logFd), "logpipe")

logrus.SetOutput(os.NewFile(uintptr(logFD), "logpipe"))
logrus.SetOutput(logPipe)
logrus.SetFormatter(new(logrus.JSONFormatter))
logrus.Debug("child process in init()")

// Only init processes have FIFOFD.
fifofd := -1
var fifoFile *os.File
envInitType := os.Getenv("_LIBCONTAINER_INITTYPE")
it := initType(envInitType)
if it == initStandard {
envFifoFd := os.Getenv("_LIBCONTAINER_FIFOFD")
if fifofd, err = strconv.Atoi(envFifoFd); err != nil {
fifoFd, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_FIFOFD"))
if err != nil {
return fmt.Errorf("unable to convert _LIBCONTAINER_FIFOFD: %w", err)
}
fifoFile = os.NewFile(uintptr(fifoFd), "initfifo")
}

var consoleSocket *os.File
Expand Down Expand Up @@ -208,10 +210,10 @@ func startInitialization() (retErr error) {
}

// If init succeeds, it will not return, hence none of the defers will be called.
return containerInit(it, &config, syncPipe, consoleSocket, pidfdSocket, fifofd, logFD, dmzExe)
return containerInit(it, &config, syncPipe, consoleSocket, pidfdSocket, fifoFile, logPipe, dmzExe)
}

func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket *os.File, fifoFd, logFd int, dmzExe *os.File) error {
func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe, dmzExe *os.File) error {
if err := populateProcessEnvironment(config.Env); err != nil {
return err
}
Expand All @@ -223,7 +225,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
consoleSocket: consoleSocket,
pidfdSocket: pidfdSocket,
config: config,
logFd: logFd,
logPipe: logPipe,
dmzExe: dmzExe,
}
return i.Init()
Expand All @@ -234,8 +236,8 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
pidfdSocket: pidfdSocket,
parentPid: unix.Getppid(),
config: config,
fifoFd: fifoFd,
logFd: logFd,
fifoFile: fifoFile,
logPipe: logPipe,
dmzExe: dmzExe,
}
return i.Init()
Expand Down
6 changes: 3 additions & 3 deletions libcontainer/mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ func mountViaFds(source string, srcFile *mountSource, target, dstFd, fstype stri
// mount(2), we need to get a safe handle to /proc/thread-self. This
// isn't needed for move_mount(2) because in that case the path is just
// a dummy string used for error info.
fdStr := strconv.Itoa(int(srcFile.file.Fd()))
srcFileFd := srcFile.file.Fd()
if isMoveMount {
src = "/proc/self/fd/" + fdStr
src = "/proc/self/fd/" + strconv.Itoa(int(srcFileFd))
} else {
var closer utils.ProcThreadSelfCloser
src, closer = utils.ProcThreadSelf("fd/" + fdStr)
src, closer = utils.ProcThreadSelfFd(srcFileFd)
defer closer()
}
}
Expand Down
7 changes: 3 additions & 4 deletions libcontainer/setns_init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"
"os/exec"
"strconv"

"github.com/opencontainers/selinux/go-selinux"
"github.com/sirupsen/logrus"
Expand All @@ -24,7 +23,7 @@ type linuxSetnsInit struct {
consoleSocket *os.File
pidfdSocket *os.File
config *initConfig
logFd int
logPipe *os.File
dmzExe *os.File
}

Expand Down Expand Up @@ -131,8 +130,8 @@ func (l *linuxSetnsInit) Init() error {

// Close the log pipe fd so the parent's ForwardLogs can exit.
logrus.Debugf("setns_init: about to exec")
if err := unix.Close(l.logFd); err != nil {
return &os.PathError{Op: "close log pipe", Path: "fd " + strconv.Itoa(l.logFd), Err: err}
if err := l.logPipe.Close(); err != nil {
return fmt.Errorf("close log pipe: %w", err)
}

if l.dmzExe != nil {
Expand Down
13 changes: 6 additions & 7 deletions libcontainer/standard_init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"
"os/exec"
"strconv"

"github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/selinux/go-selinux"
Expand All @@ -25,8 +24,8 @@ type linuxStandardInit struct {
consoleSocket *os.File
pidfdSocket *os.File
parentPid int
fifoFd int
logFd int
fifoFile *os.File
logPipe *os.File
dmzExe *os.File
config *initConfig
}
Expand Down Expand Up @@ -244,11 +243,11 @@ func (l *linuxStandardInit) Init() error {

// Close the log pipe fd so the parent's ForwardLogs can exit.
logrus.Debugf("init: about to wait on exec fifo")
if err := unix.Close(l.logFd); err != nil {
return &os.PathError{Op: "close log pipe", Path: "fd " + strconv.Itoa(l.logFd), Err: err}
if err := l.logPipe.Close(); err != nil {
return fmt.Errorf("close log pipe: %w", err)
}

fifoPath, closer := utils.ProcThreadSelf("fd/" + strconv.Itoa(l.fifoFd))
fifoPath, closer := utils.ProcThreadSelfFd(l.fifoFile.Fd())
defer closer()

// Wait for the FIFO to be opened on the other side before exec-ing the
Expand All @@ -269,7 +268,7 @@ func (l *linuxStandardInit) Init() error {
// N.B. the core issue itself (passing dirfds to the host filesystem) has
// since been resolved.
// https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318
_ = unix.Close(l.fifoFd)
_ = l.fifoFile.Close()

s := l.config.SpecState
s.Pid = unix.Getpid()
Expand Down
9 changes: 9 additions & 0 deletions libcontainer/utils/utils_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,12 @@ func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) {
}
return threadSelf + subpath, runtime.UnlockOSThread
}

// ProcThreadSelfFd is small wrapper around ProcThreadSelf to make it easier to
// create a /proc/thread-self handle for given file descriptor.
//
// It is basically equivalent to ProcThreadSelf(fmt.Sprintf("fd/%d", fd)), but
// without using fmt.Sprintf to avoid unneeded overhead.
func ProcThreadSelfFd(fd uintptr) (string, ProcThreadSelfCloser) {
return ProcThreadSelf("fd/" + strconv.FormatUint(uint64(fd), 10))
}
Loading