From e9665f4d606b64bf9c4652ab2510da368bfbd951 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sat, 20 Jan 2024 16:56:38 +1100 Subject: [PATCH 7/8] init: don't special-case logrus fds We close the logfd before execve so there's no need to special case it. In addition, it turns out that (*os.File).Fd() doesn't handle the case where the file was closed and so it seems suspect to use that kind of check. Signed-off-by: Aleksa Sarai --- libcontainer/logs/logs.go | 9 --------- libcontainer/utils/utils_unix.go | 8 -------- 2 files changed, 17 deletions(-) diff --git a/libcontainer/logs/logs.go b/libcontainer/logs/logs.go index 349a18ed3839..95deb0d6ca7a 100644 --- a/libcontainer/logs/logs.go +++ b/libcontainer/logs/logs.go @@ -4,19 +4,10 @@ import ( "bufio" "encoding/json" "io" - "os" "github.com/sirupsen/logrus" ) -// IsLogrusFd returns whether the provided fd matches the one that logrus is -// currently outputting to. This should only ever be called by UnsafeCloseFrom -// from `runc init`. -func IsLogrusFd(fd uintptr) bool { - file, ok := logrus.StandardLogger().Out.(*os.File) - return ok && file.Fd() == fd -} - func ForwardLogs(logPipe io.ReadCloser) chan error { done := make(chan error, 1) s := bufio.NewScanner(logPipe) diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index 842f9b0a6d26..bf3237a29118 100644 --- a/libcontainer/utils/utils_unix.go +++ b/libcontainer/utils/utils_unix.go @@ -10,8 +10,6 @@ import ( _ "unsafe" // for go:linkname "golang.org/x/sys/unix" - - "github.com/opencontainers/runc/libcontainer/logs" ) // EnsureProcHandle returns whether or not the given file handle is on procfs. @@ -102,12 +100,6 @@ func UnsafeCloseFrom(minFd int) error { // don't have any choice. return } - if logs.IsLogrusFd(uintptr(fd)) { - // Do not close the logrus output fd. We cannot exec a pipe, and - // the contents are quite limited (very little attacker control, - // JSON-encoded) making shellcode attacks unlikely. - return - } // There's nothing we can do about errors from close(2), and the // only likely error to be seen is EBADF which indicates the fd was // already closed (in which case, we got what we wanted). -- 2.43.0