diff --git a/file.go b/file.go index c1b8f5c..a007201 100644 --- a/file.go +++ b/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", diff --git a/file_test.go b/file_test.go index 9c3bf18..09e4bcf 100644 --- a/file_test.go +++ b/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 '", 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 }()