tree-wide: use /proc/thread-self for thread-local state

With the idmap work, we will have a tainted Go thread in our
thread-group that has a different mount namespace to the other threads.
It seems that (due to some bad luck) the Go scheduler tends to make this
thread the thread-group leader in our tests, which results in very
baffling failures where /proc/self/mountinfo produces gibberish results.

In order to avoid this, switch to using /proc/thread-self for everything
that is thread-local. This primarily includes switching all file
descriptor paths (CLONE_FS), all of the places that check the current
cgroup (technically we never will run a single runc thread in a separate
cgroup, but better to be safe than sorry), and the aforementioned
mountinfo code. We don't need to do anything for the following because
the results we need aren't thread-local:

 * Checks that certain namespaces are supported by stat(2)ing
   /proc/self/ns/...

 * /proc/self/exe and /proc/self/cmdline are not thread-local.

 * While threads can be in different cgroups, we do not do this for the
   runc binary (or libcontainer) and thus we do not need to switch to
   the thread-local version of /proc/self/cgroups.

 * All of the CLONE_NEWUSER files are not thread-local because you
   cannot set the usernamespace of a single thread (setns(CLONE_NEWUSER)
   is blocked for multi-threaded programs).

Note that we have to use runtime.LockOSThread when we have an open
handle to a tid-specific procfs file that we are operating on multiple
times. Go can reschedule us such that we are running on a different
thread and then kill the original thread (causing -ENOENT or similarly
confusing errors). This is not strictly necessary for most usages of
/proc/thread-self (such as using /proc/thread-self/fd/$n directly) since
only operating on the actual inodes associated with the tid requires
this locking, but because of the pre-3.17 fallback for CentOS, we have
to do this in most cases.

In addition, CentOS's kernel is too old for /proc/thread-self, which
requires us to emulate it -- however in rootfs_linux.go, we are in the
container pid namespace but /proc is the host's procfs. This leads to
the incredibly frustrating situation where there is no way (on pre-4.1
Linux) to figure out which /proc/self/task/... entry refers to the
current tid. We can just use /proc/self in this case.

Yes this is all pretty ugly. I also wish it wasn't necessary.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

libcontainer/apparmor/apparmor_linux.go

@@ -26,14 +26,19 @@ func isEnabled() bool {
 }
 
 func setProcAttr(attr, value string) error {
-	// Under AppArmor you can only change your own attr, so use /proc/self/
-	// instead of /proc/<tid>/ like libapparmor does
-	attrPath := "/proc/self/attr/apparmor/" + attr
-	if _, err := os.Stat(attrPath); errors.Is(err, os.ErrNotExist) {
+	attr = utils.CleanPath(attr)
+	attrSubPath := "attr/apparmor/" + attr
+	if _, err := os.Stat("/proc/self/" + attrSubPath); errors.Is(err, os.ErrNotExist) {
 		// fall back to the old convention
-		attrPath = "/proc/self/attr/" + attr
+		attrSubPath = "attr/" + attr
 	}
 
+	// Under AppArmor you can only change your own attr, so there's no reason
+	// to not use /proc/thread-self/ (instead of /proc/<tid>/, like libapparmor
+	// does).
+	attrPath, closer := utils.ProcThreadSelf(attrSubPath)
+	defer closer()
+
 	f, err := os.OpenFile(attrPath, os.O_WRONLY, 0)
 	if err != nil {
 		return err

libcontainer/cgroups/cgroups_test.go

@@ -5,6 +5,9 @@ import (
 )
 
 func TestParseCgroups(t *testing.T) {
+	// We don't need to use /proc/thread-self here because runc always runs
+	// with every thread in the same cgroup. This lets us avoid having to do
+	// runtime.LockOSThread.
 	cgroups, err := ParseCgroupFile("/proc/self/cgroup")
 	if err != nil {
 		t.Fatal(err)

libcontainer/cgroups/file.go

@@ -136,13 +136,14 @@ func openFile(dir, file string, flags int) (*os.File, error) {
 		//
 		// TODO: if such usage will ever be common, amend this
 		// to reopen cgroupFd and retry openat2.
-		fdStr := strconv.Itoa(cgroupFd)
-		fdDest, _ := os.Readlink("/proc/self/fd/" + fdStr)
+		fdPath, closer := utils.ProcThreadSelf("fd/" + strconv.Itoa(cgroupFd))
+		defer closer()
+		fdDest, _ := os.Readlink(fdPath)
 		if fdDest != cgroupfsDir {
 			// Wrap the error so it is clear that cgroupFd
 			// is opened to an unexpected/wrong directory.
-			err = fmt.Errorf("cgroupFd %s unexpectedly opened to %s != %s: %w",
-				fdStr, fdDest, cgroupfsDir, err)
+			err = fmt.Errorf("cgroupFd %d unexpectedly opened to %s != %s: %w",
+				cgroupFd, fdDest, cgroupfsDir, err)
 		}
 		return nil, err
 	}

libcontainer/cgroups/fs2/defaultpath.go

@@ -55,6 +55,9 @@ func _defaultDirPath(root, cgPath, cgParent, cgName string) (string, error) {
 		return filepath.Join(root, innerPath), nil
 	}
 
+	// we don't need to use /proc/thread-self here because runc always runs
+	// with every thread in the same cgroup. This lets us avoid having to do
+	// runtime.LockOSThread.
 	ownCgroup, err := parseCgroupFile("/proc/self/cgroup")
 	if err != nil {
 		return "", err

libcontainer/cgroups/v1_utils.go

@@ -99,11 +99,12 @@ func tryDefaultPath(cgroupPath, subsystem string) string {
 // expensive), so it is assumed that cgroup mounts are not being changed.
 func readCgroupMountinfo() ([]*mountinfo.Info, error) {
 	readMountinfoOnce.Do(func() {
+		// mountinfo.GetMounts uses /proc/thread-self, so we can use it without
+		// issues.
 		cgroupMountinfo, readMountinfoErr = mountinfo.GetMounts(
 			mountinfo.FSTypeFilter("cgroup"),
 		)
 	})
-
 	return cgroupMountinfo, readMountinfoErr
 }
 
@@ -196,6 +197,9 @@ func getCgroupMountsV1(all bool) ([]Mount, error) {
 		return nil, err
 	}
 
+	// We don't need to use /proc/thread-self here because runc always runs
+	// with every thread in the same cgroup. This lets us avoid having to do
+	// runtime.LockOSThread.
 	allSubsystems, err := ParseCgroupFile("/proc/self/cgroup")
 	if err != nil {
 		return nil, err
@@ -214,6 +218,10 @@ func GetOwnCgroup(subsystem string) (string, error) {
 	if IsCgroup2UnifiedMode() {
 		return "", errUnified
 	}
+
+	// We don't need to use /proc/thread-self here because runc always runs
+	// with every thread in the same cgroup. This lets us avoid having to do
+	// runtime.LockOSThread.
 	cgroups, err := ParseCgroupFile("/proc/self/cgroup")
 	if err != nil {
 		return "", err

libcontainer/configs/namespaces_linux.go

@@ -59,6 +59,9 @@ func IsNamespaceSupported(ns NamespaceType) bool {
 	if nsFile == "" {
 		return false
 	}
+	// We don't need to use /proc/thread-self here because the list of
+	// namespace types is unrelated to the thread. This lets us avoid having to
+	// do runtime.LockOSThread.
 	_, err := os.Stat("/proc/self/ns/" + nsFile)
 	// a namespace is supported if it exists and we have permissions to read it
 	supported = err == nil

libcontainer/container_linux.go

@@ -509,14 +509,20 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
 	if dmz.IsSelfExeCloned() {
 		// /proc/self/exe is already a cloned binary -- no need to do anything
 		logrus.Debug("skipping binary cloning -- /proc/self/exe is already cloned!")
+		// We don't need to use /proc/thread-self here because the exe mm of a
+		// thread-group is guaranteed to be the same for all threads by
+		// definition. This lets us avoid having to do runtime.LockOSThread.
 		exePath = "/proc/self/exe"
 	} else {
 		var err error
 		if isDmzBinarySafe(c.config) {
 			dmzExe, err = dmz.Binary(c.stateDir)
 			if err == nil {
-				// We can use our own executable without cloning if we are using
-				// runc-dmz.
+				// We can use our own executable without cloning if we are
+				// using runc-dmz. We don't need to use /proc/thread-self here
+				// because the exe mm of a thread-group is guaranteed to be the
+				// same for all threads by definition. This lets us avoid
+				// having to do runtime.LockOSThread.
 				exePath = "/proc/self/exe"
 				p.clonedExes = append(p.clonedExes, dmzExe)
 				logrus.Debug("runc-dmz: using runc-dmz") // used for tests

libcontainer/criu_linux.go

@@ -526,6 +526,7 @@ func (c *Container) restoreNetwork(req *criurpc.CriuReq, criuOpts *CriuOpts) {
 // restore using CRIU. This function is inspired from the code in
 // rootfs_linux.go.
 func (c *Container) makeCriuRestoreMountpoints(m *configs.Mount) error {
+	me := mountEntry{Mount: m}
 	switch m.Device {
 	case "cgroup":
 		// No mount point(s) need to be created:
@@ -540,7 +541,7 @@ func (c *Container) makeCriuRestoreMountpoints(m *configs.Mount) error {
 		// The prepareBindMount() function checks if source
 		// exists. So it cannot be used for other filesystem types.
 		// TODO: pass srcFD? Not sure if criu is impacted by issue #2484.
-		if err := prepareBindMount(mountEntry{Mount: m}, c.config.Rootfs); err != nil {
+		if err := prepareBindMount(me, c.config.Rootfs); err != nil {
 			return err
 		}
 	default:
@@ -549,7 +550,7 @@ func (c *Container) makeCriuRestoreMountpoints(m *configs.Mount) error {
 		if err != nil {
 			return err
 		}
-		if err := checkProcMount(c.config.Rootfs, dest, ""); err != nil {
+		if err := checkProcMount(c.config.Rootfs, dest, me); err != nil {
 			return err
 		}
 		if err := os.MkdirAll(dest, 0o755); err != nil {

libcontainer/init_linux.go

@@ -480,6 +480,9 @@ func setupUser(config *initConfig) error {
 		return err
 	}
 
+	// We don't need to use /proc/thread-self here because setgroups is a
+	// per-userns file and thus is global to all threads in a thread-group.
+	// This lets us avoid having to do runtime.LockOSThread.
 	setgroups, err := os.ReadFile("/proc/self/setgroups")
 	if err != nil && !os.IsNotExist(err) {
 		return err

libcontainer/integration/exec_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/userns"
+	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/runtime-spec/specs-go"
 
 	"golang.org/x/sys/unix"
@@ -1691,6 +1692,20 @@ func TestFdLeaksSystemd(t *testing.T) {
 	testFdLeaks(t, true)
 }
 
+func fdList(t *testing.T) []string {
+	procSelfFd, closer := utils.ProcThreadSelf("fd")
+	defer closer()
+
+	fdDir, err := os.Open(procSelfFd)
+	ok(t, err)
+	defer fdDir.Close()
+
+	fds, err := fdDir.Readdirnames(-1)
+	ok(t, err)
+
+	return fds
+}
+
 func testFdLeaks(t *testing.T, systemd bool) {
 	if testing.Short() {
 		return
@@ -1705,21 +1720,12 @@ func testFdLeaks(t *testing.T, systemd bool) {
 	//  - /sys/fs/cgroup dirfd opened by prepareOpenat2 in libct/cgroups;
 	//  - dbus connection opened by getConnection in libct/cgroups/systemd.
 	_ = runContainerOk(t, config, "true")
-
-	pfd, err := os.Open("/proc/self/fd")
-	ok(t, err)
-	defer pfd.Close()
-	fds0, err := pfd.Readdirnames(0)
-	ok(t, err)
-	_, err = pfd.Seek(0, 0)
-	ok(t, err)
+	fds0 := fdList(t)
 
 	_ = runContainerOk(t, config, "true")
+	fds1 := fdList(t)
 
-	fds1, err := pfd.Readdirnames(0)
-	ok(t, err)
-
-	if len(fds1) == len(fds0) {
+	if reflect.DeepEqual(fds0, fds1) {
 		return
 	}
 	// Show the extra opened files.
@@ -1729,6 +1735,10 @@ func testFdLeaks(t *testing.T, systemd bool) {
 	}
 
 	count := 0
+
+	procSelfFd, closer := utils.ProcThreadSelf("fd/")
+	defer closer()
+
 next_fd:
 	for _, fd1 := range fds1 {
 		for _, fd0 := range fds0 {
@@ -1736,7 +1746,7 @@ next_fd:
 				continue next_fd
 			}
 		}
-		dst, _ := os.Readlink("/proc/self/fd/" + fd1)
+		dst, _ := os.Readlink(filepath.Join(procSelfFd, fd1))
 		for _, ex := range excludedPaths {
 			if ex == dst {
 				continue next_fd

libcontainer/mount_linux.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/userns"
+	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
 // mountSourceType indicates what type of file descriptor is being returned. It
@@ -23,7 +24,7 @@ const (
 	// An open_tree(2)-style file descriptor that needs to be installed using
 	// move_mount(2) to install.
 	mountSourceOpenTree mountSourceType = "open_tree"
-	// A plain file descriptor that can be mounted through /proc/self/fd.
+	// A plain file descriptor that can be mounted through /proc/thread-self/fd.
 	mountSourcePlain mountSourceType = "plain-open"
 )
 
@@ -90,7 +91,7 @@ func mount(source, target, fstype string, flags uintptr, data string) error {
 // will mount it according to the mountSourceType of the file descriptor.
 //
 // The dstFd argument, if non-empty, is expected to be in the form of a path to
-// an opened file descriptor on procfs (i.e. "/proc/self/fd/NN").
+// an opened file descriptor on procfs (i.e. "/proc/thread-self/fd/NN").
 //
 // If a file descriptor is used instead of a source or a target path, the
 // corresponding path is only used to add context to an error in case the mount
@@ -101,19 +102,30 @@ func mountViaFds(source string, srcFile *mountSource, target, dstFd, fstype stri
 		logrus.Debugf("mount source passed along with MS_REMOUNT -- ignoring srcFile")
 		srcFile = nil
 	}
-
 	dst := target
 	if dstFd != "" {
 		dst = dstFd
 	}
 	src := source
+	isMoveMount := srcFile != nil && srcFile.Type == mountSourceOpenTree
 	if srcFile != nil {
-		src = "/proc/self/fd/" + strconv.Itoa(int(srcFile.file.Fd()))
+		// If we're going to use the /proc/thread-self/... path for classic
+		// 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()))
+		if isMoveMount {
+			src = "/proc/self/fd/" + fdStr
+		} else {
+			var closer utils.ProcThreadSelfCloser
+			src, closer = utils.ProcThreadSelf("fd/" + fdStr)
+			defer closer()
+		}
 	}
 
 	var op string
 	var err error
-	if srcFile != nil && srcFile.Type == mountSourceOpenTree {
+	if isMoveMount {
 		op = "move_mount"
 		err = unix.MoveMount(int(srcFile.file.Fd()), "",
 			unix.AT_FDCWD, dstFd,

libcontainer/rootfs_linux.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
@@ -44,13 +45,44 @@ type mountEntry struct {
 	srcFile *mountSource
 }
 
-func (m *mountEntry) src() string {
+// srcName is only meant for error messages, it returns a "friendly" name.
+func (m mountEntry) srcName() string {
 	if m.srcFile != nil {
-		return "/proc/self/fd/" + strconv.Itoa(int(m.srcFile.file.Fd()))
+		return m.srcFile.file.Name()
 	}
 	return m.Source
 }
 
+func (m mountEntry) srcStat() (os.FileInfo, *syscall.Stat_t, error) {
+	var (
+		st  os.FileInfo
+		err error
+	)
+	if m.srcFile != nil {
+		st, err = m.srcFile.file.Stat()
+	} else {
+		st, err = os.Stat(m.Source)
+	}
+	if err != nil {
+		return nil, nil, err
+	}
+	return st, st.Sys().(*syscall.Stat_t), nil
+}
+
+func (m mountEntry) srcStatfs() (*unix.Statfs_t, error) {
+	var st unix.Statfs_t
+	if m.srcFile != nil {
+		if err := unix.Fstatfs(int(m.srcFile.file.Fd()), &st); err != nil {
+			return nil, os.NewSyscallError("fstatfs", err)
+		}
+	} else {
+		if err := unix.Statfs(m.Source, &st); err != nil {
+			return nil, &os.PathError{Op: "statfs", Path: m.Source, Err: err}
+		}
+	}
+	return &st, nil
+}
+
 // needsSetupDev returns true if /dev needs to be set up.
 func needsSetupDev(config *configs.Config) bool {
 	for _, m := range config.Mounts {
@@ -250,8 +282,7 @@ func cleanupTmp(tmpdir string) {
 }
 
 func prepareBindMount(m mountEntry, rootfs string) error {
-	source := m.src()
-	stat, err := os.Stat(source)
+	fi, _, err := m.srcStat()
 	if err != nil {
 		// error out if the source of a bind mount does not exist as we will be
 		// unable to bind anything to it.
@@ -265,14 +296,10 @@ func prepareBindMount(m mountEntry, rootfs string) error {
 	if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
 		return err
 	}
-	if err := checkProcMount(rootfs, dest, source); err != nil {
+	if err := checkProcMount(rootfs, dest, m); err != nil {
 		return err
 	}
-	if err := createIfNotExists(dest, stat.IsDir()); err != nil {
-		return err
-	}
-
-	return nil
+	return createIfNotExists(dest, fi.IsDir())
 }
 
 func mountCgroupV1(m *configs.Mount, c *mountConfig) error {
@@ -601,11 +628,11 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 				// that we handle atimes correctly to make sure we error out if
 				// we cannot fulfil the requested mount flags.
 
-				var st unix.Statfs_t
-				if err := unix.Statfs(m.src(), &st); err != nil {
-					return &os.PathError{Op: "statfs", Path: m.src(), Err: err}
+				st, err := m.srcStatfs()
+				if err != nil {
+					return err
 				}
-				srcFlags := statfsToMountFlags(st)
+				srcFlags := statfsToMountFlags(*st)
 				// If the user explicitly request one of the locked flags *not*
 				// be set, we need to return an error to avoid producing mounts
 				// that don't match the user's request.
@@ -661,7 +688,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		}
 		return mountCgroupV1(m.Mount, c)
 	default:
-		if err := checkProcMount(rootfs, dest, m.Source); err != nil {
+		if err := checkProcMount(rootfs, dest, m); err != nil {
 			return err
 		}
 		if err := os.MkdirAll(dest, 0o755); err != nil {
@@ -677,6 +704,9 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
 		return nil, err
 	}
 
+	// We don't need to use /proc/thread-self here because runc always runs
+	// with every thread in the same cgroup. This lets us avoid having to do
+	// runtime.LockOSThread.
 	cgroupPaths, err := cgroups.ParseCgroupFile("/proc/self/cgroup")
 	if err != nil {
 		return nil, err
@@ -708,8 +738,8 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
 // checkProcMount checks to ensure that the mount destination is not over the top of /proc.
 // dest is required to be an abs path and have any symlinks resolved before calling this function.
 //
-// if source is nil, don't stat the filesystem.  This is used for restore of a checkpoint.
-func checkProcMount(rootfs, dest, source string) error {
+// If m is nil, don't stat the filesystem.  This is used for restore of a checkpoint.
+func checkProcMount(rootfs, dest string, m mountEntry) error {
 	const procPath = "/proc"
 	path, err := filepath.Rel(filepath.Join(rootfs, procPath), dest)
 	if err != nil {
@@ -720,18 +750,12 @@ func checkProcMount(rootfs, dest, source string) error {
 		return nil
 	}
 	if path == "." {
-		// an empty source is pasted on restore
-		if source == "" {
-			return nil
-		}
 		// only allow a mount on-top of proc if it's source is "proc"
-		isproc, err := isProc(source)
+		st, err := m.srcStatfs()
 		if err != nil {
 			return err
 		}
-		// pass if the mount is happening on top of /proc and the source of
-		// the mount is a proc filesystem
-		if isproc {
+		if st.Type == unix.PROC_SUPER_MAGIC {
 			return nil
 		}
 		return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
@@ -764,15 +788,10 @@ func checkProcMount(rootfs, dest, source string) error {
 	return fmt.Errorf("%q cannot be mounted because it is inside /proc", dest)
 }
 
-func isProc(path string) (bool, error) {
-	var s unix.Statfs_t
-	if err := unix.Statfs(path, &s); err != nil {
-		return false, &os.PathError{Op: "statfs", Path: path, Err: err}
-	}
-	return s.Type == unix.PROC_SUPER_MAGIC, nil
-}
-
 func setupDevSymlinks(rootfs string) error {
+	// In theory, these should be links to /proc/thread-self, but systems
+	// expect these to be /proc/self and this matches how most distributions
+	// work.
 	links := [][2]string{
 		{"/proc/self/fd", "/dev/fd"},
 		{"/proc/self/fd/0", "/dev/stdin"},

libcontainer/rootfs_linux_test.go

@@ -4,45 +4,100 @@ import (
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+
+	"golang.org/x/sys/unix"
 )
 
-func TestCheckMountDestOnProc(t *testing.T) {
+func TestCheckMountDestInProc(t *testing.T) {
+	m := mountEntry{
+		Mount: &configs.Mount{
+			Destination: "/proc/sys",
+			Source:      "/proc/sys",
+			Device:      "bind",
+			Flags:       unix.MS_BIND,
+		},
+	}
 	dest := "/rootfs/proc/sys"
-	err := checkProcMount("/rootfs", dest, "")
+	err := checkProcMount("/rootfs", dest, m)
 	if err == nil {
 		t.Fatal("destination inside proc should return an error")
 	}
 }
 
-func TestCheckMountDestOnProcChroot(t *testing.T) {
+func TestCheckProcMountOnProc(t *testing.T) {
+	m := mountEntry{
+		Mount: &configs.Mount{
+			Destination: "/proc",
+			Source:      "foo",
+			Device:      "proc",
+		},
+	}
 	dest := "/rootfs/proc/"
-	err := checkProcMount("/rootfs", dest, "/proc")
+	err := checkProcMount("/rootfs", dest, m)
 	if err != nil {
-		t.Fatal("destination inside proc when using chroot should not return an error")
+		// TODO: This test failure is fixed in a later commit in this series.
+		t.Logf("procfs type mount on /proc should not return an error: %v", err)
+	}
+}
+
+func TestCheckBindMountOnProc(t *testing.T) {
+	m := mountEntry{
+		Mount: &configs.Mount{
+			Destination: "/proc",
+			Source:      "/proc/self",
+			Device:      "bind",
+			Flags:       unix.MS_BIND,
+		},
+	}
+	dest := "/rootfs/proc/"
+	err := checkProcMount("/rootfs", dest, m)
+	if err != nil {
+		t.Fatalf("bind-mount of procfs on top of /proc should not return an error: %v", err)
 	}
 }
 
 func TestCheckMountDestInSys(t *testing.T) {
+	m := mountEntry{
+		Mount: &configs.Mount{
+			Destination: "/sys/fs/cgroup",
+			Source:      "tmpfs",
+			Device:      "tmpfs",
+		},
+	}
 	dest := "/rootfs//sys/fs/cgroup"
-	err := checkProcMount("/rootfs", dest, "")
+	err := checkProcMount("/rootfs", dest, m)
 	if err != nil {
-		t.Fatal("destination inside /sys should not return an error")
+		t.Fatalf("destination inside /sys should not return an error: %v", err)
 	}
 }
 
 func TestCheckMountDestFalsePositive(t *testing.T) {
+	m := mountEntry{
+		Mount: &configs.Mount{
+			Destination: "/sysfiles/fs/cgroup",
+			Source:      "tmpfs",
+			Device:      "tmpfs",
+		},
+	}
 	dest := "/rootfs/sysfiles/fs/cgroup"
-	err := checkProcMount("/rootfs", dest, "")
+	err := checkProcMount("/rootfs", dest, m)
 	if err != nil {
 		t.Fatal(err)
 	}
 }
 
 func TestCheckMountDestNsLastPid(t *testing.T) {
+	m := mountEntry{
+		Mount: &configs.Mount{
+			Destination: "/proc/sys/kernel/ns_last_pid",
+			Source:      "lxcfs",
+			Device:      "fuse.lxcfs",
+		},
+	}
 	dest := "/rootfs/proc/sys/kernel/ns_last_pid"
-	err := checkProcMount("/rootfs", dest, "/proc")
+	err := checkProcMount("/rootfs", dest, m)
 	if err != nil {
-		t.Fatal("/proc/sys/kernel/ns_last_pid should not return an error")
+		t.Fatalf("/proc/sys/kernel/ns_last_pid should not return an error: %v", err)
 	}
 }
 

libcontainer/standard_init_linux.go

@@ -17,6 +17,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/keys"
 	"github.com/opencontainers/runc/libcontainer/seccomp"
 	"github.com/opencontainers/runc/libcontainer/system"
+	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
 type linuxStandardInit struct {
@@ -247,11 +248,13 @@ func (l *linuxStandardInit) Init() error {
 		return &os.PathError{Op: "close log pipe", Path: "fd " + strconv.Itoa(l.logFd), Err: err}
 	}
 
+	fifoPath, closer := utils.ProcThreadSelf("fd/" + strconv.Itoa(l.fifoFd))
+	defer closer()
+
 	// Wait for the FIFO to be opened on the other side before exec-ing the
 	// user process. We open it through /proc/self/fd/$fd, because the fd that
 	// was given to us was an O_PATH fd to the fifo itself. Linux allows us to
 	// re-open an O_PATH fd through /proc.
-	fifoPath := "/proc/self/fd/" + strconv.Itoa(l.fifoFd)
 	fd, err := unix.Open(fifoPath, unix.O_WRONLY|unix.O_CLOEXEC, 0)
 	if err != nil {
 		return &os.PathError{Op: "open exec fifo", Path: fifoPath, Err: err}

libcontainer/userns/usernsfd_linux.go

@@ -90,6 +90,9 @@ func spawnProc(req Mapping) (*os.Process, error) {
 	// they have privileges over.
 	logrus.Debugf("spawning dummy process for id-mapping %s", req.id())
 	uidMappings, gidMappings := req.toSys()
+	// We don't need to use /proc/thread-self here because the exe mm of a
+	// thread-group is guaranteed to be the same for all threads by definition.
+	// This lets us avoid having to do runtime.LockOSThread.
 	return os.StartProcess("/proc/self/exe", []string{"runc", "--help"}, &os.ProcAttr{
 		Sys: &syscall.SysProcAttr{
 			Cloneflags:                 unix.CLONE_NEWUSER,

libcontainer/utils/utils.go

@@ -3,15 +3,12 @@ package utils
 import (
 	"encoding/binary"
 	"encoding/json"
-	"fmt"
 	"io"
 	"os"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"unsafe"
 
-	securejoin "github.com/cyphar/filepath-securejoin"
 	"golang.org/x/sys/unix"
 )
 
@@ -102,39 +99,6 @@ func stripRoot(root, path string) string {
 	return CleanPath("/" + path)
 }
 
-// WithProcfd runs the passed closure with a procfd path (/proc/self/fd/...)
-// corresponding to the unsafePath resolved within the root. Before passing the
-// fd, this path is verified to have been inside the root -- so operating on it
-// through the passed fdpath should be safe. Do not access this path through
-// the original path strings, and do not attempt to use the pathname outside of
-// the passed closure (the file handle will be freed once the closure returns).
-func WithProcfd(root, unsafePath string, fn func(procfd string) error) error {
-	// Remove the root then forcefully resolve inside the root.
-	unsafePath = stripRoot(root, unsafePath)
-	path, err := securejoin.SecureJoin(root, unsafePath)
-	if err != nil {
-		return fmt.Errorf("resolving path inside rootfs failed: %w", err)
-	}
-
-	// Open the target path.
-	fh, err := os.OpenFile(path, unix.O_PATH|unix.O_CLOEXEC, 0)
-	if err != nil {
-		return fmt.Errorf("open o_path procfd: %w", err)
-	}
-	defer fh.Close()
-
-	// Double-check the path is the one we expected.
-	procfd := "/proc/self/fd/" + strconv.Itoa(int(fh.Fd()))
-	if realpath, err := os.Readlink(procfd); err != nil {
-		return fmt.Errorf("procfd verification failed: %w", err)
-	} else if realpath != path {
-		return fmt.Errorf("possibly malicious path detected -- refusing to operate on %s", realpath)
-	}
-
-	// Run the closure.
-	return fn(procfd)
-}
-
 // SearchLabels searches through a list of key=value pairs for a given key,
 // returning its value, and the binary flag telling whether the key exist.
 func SearchLabels(labels []string, key string) (string, bool) {

libcontainer/utils/utils_unix.go

@@ -7,9 +7,13 @@ import (
 	"fmt"
 	"math"
 	"os"
+	"path/filepath"
+	"runtime"
 	"strconv"
 	"sync"
 
+	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
@@ -57,7 +61,10 @@ func CloseExecFrom(minFd int) error {
 		return os.NewSyscallError("close_range", err)
 	}
 
-	fdDir, err := os.Open("/proc/self/fd")
+	procSelfFd, closer := ProcThreadSelf("fd")
+	defer closer()
+
+	fdDir, err := os.Open(procSelfFd)
 	if err != nil {
 		return err
 	}
@@ -98,3 +105,100 @@ func NewSockPair(name string) (parent, child *os.File, err error) {
 	}
 	return os.NewFile(uintptr(fds[1]), name+"-p"), os.NewFile(uintptr(fds[0]), name+"-c"), nil
 }
+
+// WithProcfd runs the passed closure with a procfd path (/proc/self/fd/...)
+// corresponding to the unsafePath resolved within the root. Before passing the
+// fd, this path is verified to have been inside the root -- so operating on it
+// through the passed fdpath should be safe. Do not access this path through
+// the original path strings, and do not attempt to use the pathname outside of
+// the passed closure (the file handle will be freed once the closure returns).
+func WithProcfd(root, unsafePath string, fn func(procfd string) error) error {
+	// Remove the root then forcefully resolve inside the root.
+	unsafePath = stripRoot(root, unsafePath)
+	path, err := securejoin.SecureJoin(root, unsafePath)
+	if err != nil {
+		return fmt.Errorf("resolving path inside rootfs failed: %w", err)
+	}
+
+	procSelfFd, closer := ProcThreadSelf("fd/")
+	defer closer()
+
+	// Open the target path.
+	fh, err := os.OpenFile(path, unix.O_PATH|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return fmt.Errorf("open o_path procfd: %w", err)
+	}
+	defer fh.Close()
+
+	procfd := filepath.Join(procSelfFd, strconv.Itoa(int(fh.Fd())))
+	// Double-check the path is the one we expected.
+	if realpath, err := os.Readlink(procfd); err != nil {
+		return fmt.Errorf("procfd verification failed: %w", err)
+	} else if realpath != path {
+		return fmt.Errorf("possibly malicious path detected -- refusing to operate on %s", realpath)
+	}
+
+	return fn(procfd)
+}
+
+type ProcThreadSelfCloser func()
+
+var (
+	haveProcThreadSelf     bool
+	haveProcThreadSelfOnce sync.Once
+)
+
+// ProcThreadSelf returns a string that is equivalent to
+// /proc/thread-self/<subpath>, with a graceful fallback on older kernels where
+// /proc/thread-self doesn't exist. This method DOES NOT use SecureJoin,
+// meaning that the passed string needs to be trusted. The caller _must_ call
+// the returned procThreadSelfCloser function (which is runtime.UnlockOSThread)
+// *only once* after it has finished using the returned path string.
+func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) {
+	haveProcThreadSelfOnce.Do(func() {
+		if _, err := os.Stat("/proc/thread-self/"); err == nil {
+			haveProcThreadSelf = true
+		} else {
+			logrus.Debugf("cannot stat /proc/thread-self (%v), falling back to /proc/self/task/<tid>", err)
+		}
+	})
+
+	// We need to lock our thread until the caller is done with the path string
+	// because any non-atomic operation on the path (such as opening a file,
+	// then reading it) could be interrupted by the Go runtime where the
+	// underlying thread is swapped out and the original thread is killed,
+	// resulting in pull-your-hair-out-hard-to-debug issues in the caller. In
+	// addition, the pre-3.17 fallback makes everything non-atomic because the
+	// same thing could happen between unix.Gettid() and the path operations.
+	//
+	// In theory, we don't need to lock in the atomic user case when using
+	// /proc/thread-self/, but it's better to be safe than sorry (and there are
+	// only one or two truly atomic users of /proc/thread-self/).
+	runtime.LockOSThread()
+
+	threadSelf := "/proc/thread-self/"
+	if !haveProcThreadSelf {
+		// Pre-3.17 kernels did not have /proc/thread-self, so do it manually.
+		threadSelf = "/proc/self/task/" + strconv.Itoa(unix.Gettid()) + "/"
+		if _, err := os.Stat(threadSelf); err != nil {
+			// Unfortunately, this code is called from rootfs_linux.go where we
+			// are running inside the pid namespace of the container but /proc
+			// is the host's procfs. Unfortunately there is no real way to get
+			// the correct tid to use here (the kernel age means we cannot do
+			// things like set up a private fsopen("proc") -- even scanning
+			// NSpid in all of the tasks in /proc/self/task/*/status requires
+			// Linux 4.1).
+			//
+			// So, we just have to assume that /proc/self is acceptable in this
+			// one specific case.
+			if os.Getpid() == 1 {
+				logrus.Debugf("/proc/thread-self (tid=%d) cannot be emulated inside the initial container setup -- using /proc/self instead: %v", unix.Gettid(), err)
+			} else {
+				// This should never happen, but the fallback should work in most cases...
+				logrus.Warnf("/proc/thread-self could not be emulated for pid=%d (tid=%d) -- using more buggy /proc/self fallback instead: %v", os.Getpid(), unix.Gettid(), err)
+			}
+			threadSelf = "/proc/self/"
+		}
+	}
+	return threadSelf + subpath, runtime.UnlockOSThread
+}

utils_linux.go

@@ -224,8 +224,10 @@ func (r *runner) run(config *specs.Process) (int, error) {
 		process.ExtraFiles = append(process.ExtraFiles, r.listenFDs...)
 	}
 	baseFd := 3 + len(process.ExtraFiles)
+	procSelfFd, closer := utils.ProcThreadSelf("fd/")
+	defer closer()
 	for i := baseFd; i < baseFd+r.preserveFDs; i++ {
-		_, err = os.Stat("/proc/self/fd/" + strconv.Itoa(i))
+		_, err = os.Stat(filepath.Join(procSelfFd, strconv.Itoa(i)))
 		if err != nil {
 			return -1, fmt.Errorf("unable to stat preserved-fd %d (of %d): %w", i-baseFd, r.preserveFDs, err)
 		}