From 152f5ec27004318327c1a6b84d43570367723afc Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 1 Feb 2024 11:42:27 -0800 Subject: [PATCH] libct: don't close syncPipe Recent CVE-2024-21626 fix (commit f2f16213) broke a recently added test (commit 0bc4732c) because if exec fails, runc is unable to communicate the error back to the parent. Let's not close syncPipe. Signed-off-by: Kir Kolyshkin --- libcontainer/setns_init_linux.go | 2 +- libcontainer/standard_init_linux.go | 2 +- libcontainer/utils/utils_unix.go | 9 +++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index 4a458fcbeba..d6ab5e43804 100644 --- a/libcontainer/setns_init_linux.go +++ b/libcontainer/setns_init_linux.go @@ -154,7 +154,7 @@ func (l *linuxSetnsInit) Init() error { // that all O_CLOEXEC file descriptors have already been closed and thus // the second execve(2) from runc-dmz cannot access internal file // descriptors from runc. - if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil { + if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount+3, int(l.pipe.File().Fd())); err != nil { return err } return system.Exec(name, l.config.Args, os.Environ()) diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index c69e4e312a0..ba7edc91aec 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -296,7 +296,7 @@ func (l *linuxStandardInit) Init() error { // that all O_CLOEXEC file descriptors have already been closed and thus // the second execve(2) from runc-dmz cannot access internal file // descriptors from runc. - if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil { + if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount+3, int(l.pipe.File().Fd())); err != nil { return err } return system.Exec(name, l.config.Args, os.Environ()) diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index f57f0874a06..87055358558 100644 --- a/libcontainer/utils/utils_unix.go +++ b/libcontainer/utils/utils_unix.go @@ -120,17 +120,22 @@ func runtime_IsPollDescriptor(fd uintptr) bool //nolint:revive // UnsafeCloseFrom closes all file descriptors greater or equal to minFd in the // current process, except for those critical to Go's runtime (such as the -// netpoll management descriptors). +// netpoll management descriptors), and the explicitly specified ones. // // NOTE: That this function is incredibly dangerous to use in most Go code, as // closing file descriptors from underneath *os.File handles can lead to very // bad behaviour (the closed file descriptor can be re-used and then any // *os.File operations would apply to the wrong file). This function is only // intended to be called from the last stage of runc init. -func UnsafeCloseFrom(minFd int) error { +func UnsafeCloseFrom(minFd int, except ...int) error { // We cannot use close_range(2) even if it is available, because we must // not close some file descriptors. return fdRangeFrom(minFd, func(fd int) { + for _, ex := range except { + if fd == ex { + return + } + } if runtime_IsPollDescriptor(uintptr(fd)) { // These are the Go runtimes internal netpoll file descriptors. // These file descriptors are operated on deep in the Go scheduler,