Reopen cgroupRootHandle if it was unexpectedly opened to another file

Signed-off-by: Bin Tang <[email protected]>

Author: sctb512

file.go

@@ -101,6 +101,7 @@ var (
 
 	cgroupRootHandle *os.File
 	prepOnce         sync.Once
+	resetOnce        sync.Once
 	prepErr          error
 	resolveFlags     uint64
 )
@@ -134,6 +135,9 @@ func prepareOpenat2() error {
 			// cgroupv2 has a single mountpoint and no "cpu,cpuacct" symlinks
 			resolveFlags |= unix.RESOLVE_NO_XDEV | unix.RESOLVE_NO_SYMLINKS
 		}
+
+		// Make resetOnce ready for the next time to reset prepOnce.
+		resetOnce = sync.Once{}
 	})
 
 	return prepErr
@@ -170,11 +174,14 @@ func openFile(dir, file string, flags int) (*os.File, error) {
 		// (happens when this package is incorrectly used
 		// across the chroot/pivot_root/mntns boundary, or
 		// when /sys/fs/cgroup is remounted).
-		//
-		// TODO: if such usage will ever be common, amend this
-		// to reopen cgroupRootHandle and retry openat2.
 		fdDest, fdErr := os.Readlink("/proc/thread-self/fd/" + strconv.Itoa(int(cgroupRootHandle.Fd())))
 		if fdErr == nil && fdDest != cgroupfsDir {
+			// Only reset prepOnce once when the cgroupRootHandle is opened to another location.
+			resetOnce.Do(func() {
+				// It is difficult to recover the cgroupRootHandle in this case,
+				// so we reset prepOnce to reopen the cgroupRootHandle and retry openat2.
+				prepOnce = sync.Once{}
+			})
 			// Wrap the error so it is clear that cgroupRootHandle
 			// is opened to an unexpected/wrong directory.
 			err = fmt.Errorf("cgroupRootHandle %d unexpectedly opened to %s != %s: %w",

file_test.go

@@ -6,8 +6,13 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
+	"sync"
+	"syscall"
 	"testing"
 	"time"
+
+	"golang.org/x/sys/unix"
 )
 
 func TestWriteCgroupFileHandlesInterrupt(t *testing.T) {
@@ -70,6 +75,77 @@ func TestOpenat2(t *testing.T) {
 	}
 }
 
+func TestCgroupRootHandleOpenedToAnotherFile(t *testing.T) {
+	const (
+		memoryCgroupMount = "/sys/fs/cgroup/memory"
+		memoryLimit       = "memory.limit_in_bytes"
+	)
+	if _, err := os.Stat(memoryCgroupMount); err != nil {
+		// most probably cgroupv2
+		t.Skip(err)
+	}
+
+	cgroupName := fmt.Sprintf("test-eano-%d", time.Now().Nanosecond())
+	cgroupPath := filepath.Join(memoryCgroupMount, cgroupName)
+	if err := os.MkdirAll(cgroupPath, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(cgroupPath)
+
+	if _, err := os.Stat(filepath.Join(cgroupPath, memoryLimit)); err != nil {
+		// either cgroupv2, or memory controller is not available
+		t.Skip(err)
+	}
+
+	// The cgroupRootHandle is opened when the openFile is called.
+	if _, err := openFile(cgroupfsDir, filepath.Join("memory", cgroupName, memoryLimit), os.O_RDONLY); err != nil {
+		t.Fatal(err)
+	}
+
+	// Make sure the cgroupRootHandle is opened to another file.
+	if err := syscall.Close(int(cgroupRootHandle.Fd())); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := unix.Openat2(-1, "/tmp", &unix.OpenHow{Flags: unix.O_DIRECTORY | unix.O_PATH | unix.O_CLOEXEC}); err != nil {
+		t.Fatal(err)
+	}
+
+	var readErr *error
+	readErrLock := sync.Mutex{}
+	errCount := 0
+
+	// The openFile returns error (may be multiple times) and the prepOnce is reset only once.
+	var wg sync.WaitGroup
+	for i := 0; i < 100; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			_, err := openFile(cgroupfsDir, filepath.Join("memory", cgroupName, memoryLimit), os.O_RDONLY)
+			t.Logf("openFile attempt %d: %v\n", i, err)
+			if err != nil {
+				readErrLock.Lock()
+				readErr = &err
+				errCount++
+				readErrLock.Unlock()
+			}
+		}(i)
+	}
+	wg.Wait()
+
+	if errCount == 0 {
+		t.Fatal("At least one openFile should fail")
+	}
+
+	if !strings.Contains((*readErr).Error(), "unexpectedly opened to") {
+		t.Fatalf("openFile should fail with 'cgroupRootHandle %d unexpectedly opened to <another file>'", cgroupRootHandle.Fd())
+	}
+
+	// The openFile should work after prepOnce is reset because the cgroupRootHandle is updated.
+	if _, err := openFile(cgroupfsDir, filepath.Join("memory", cgroupName, memoryLimit), os.O_RDONLY); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func BenchmarkWriteFile(b *testing.B) {
 	TestMode = true
 	defer func() { TestMode = false }()