Skip to content

Commit

Permalink
init: don't special-case logrus fds
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
cyphar committed Jan 23, 2024
1 parent 683ad2f commit e9665f4
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 17 deletions.
9 changes: 0 additions & 9 deletions libcontainer/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 0 additions & 8 deletions libcontainer/utils/utils_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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).
Expand Down

0 comments on commit e9665f4

Please sign in to comment.