diff --git a/libcontainer/apparmor/apparmor_linux.go b/libcontainer/apparmor/apparmor_linux.go index 9bb4fb2cd2d..f7870d217a8 100644 --- a/libcontainer/apparmor/apparmor_linux.go +++ b/libcontainer/apparmor/apparmor_linux.go @@ -11,21 +11,14 @@ import ( "github.com/opencontainers/runc/internal/pathrs" ) -var ( - appArmorEnabled bool - checkAppArmor sync.Once -) - // isEnabled returns true if apparmor is enabled for the host. -func isEnabled() bool { - checkAppArmor.Do(func() { - if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil { - buf, err := os.ReadFile("/sys/module/apparmor/parameters/enabled") - appArmorEnabled = err == nil && len(buf) > 1 && buf[0] == 'Y' - } - }) - return appArmorEnabled -} +var isEnabled = sync.OnceValue(func() bool { + if _, err := os.Stat("/sys/kernel/security/apparmor"); err != nil { + return false + } + buf, err := os.ReadFile("/sys/module/apparmor/parameters/enabled") + return err == nil && len(buf) > 0 && buf[0] == 'Y' +}) func setProcAttr(attr, value string) error { attr = pathrs.LexicallyCleanPath(attr) diff --git a/libcontainer/configs/validate/intelrdt.go b/libcontainer/configs/validate/intelrdt.go index 7b6015b826f..f3d06b23ed3 100644 --- a/libcontainer/configs/validate/intelrdt.go +++ b/libcontainer/configs/validate/intelrdt.go @@ -1,41 +1,12 @@ package validate import ( - "sync" - "github.com/opencontainers/runc/libcontainer/intelrdt" ) -// Cache the result of intelrdt IsEnabled functions to avoid repeated sysfs -// access and enable mocking for unit tests. -type intelRdtStatus struct { - sync.Once - rdtEnabled bool - catEnabled bool - mbaEnabled bool -} - -var intelRdt = &intelRdtStatus{} - -func (i *intelRdtStatus) init() { - i.Do(func() { - i.rdtEnabled = intelrdt.IsEnabled() - i.catEnabled = intelrdt.IsCATEnabled() - i.mbaEnabled = intelrdt.IsMBAEnabled() - }) -} - -func (i *intelRdtStatus) isEnabled() bool { - i.init() - return i.rdtEnabled -} - -func (i *intelRdtStatus) isCATEnabled() bool { - i.init() - return i.catEnabled -} - -func (i *intelRdtStatus) isMBAEnabled() bool { - i.init() - return i.mbaEnabled -} +// Allow mocking for TestValidateIntelRdt. +var ( + intelRdtIsEnabled = intelrdt.IsEnabled + intelRdtIsCATEnabled = intelrdt.IsCATEnabled + intelRdtIsMBAEnabled = intelrdt.IsMBAEnabled +) diff --git a/libcontainer/configs/validate/intelrdt_test.go b/libcontainer/configs/validate/intelrdt_test.go index ddee2562559..a89b1554f45 100644 --- a/libcontainer/configs/validate/intelrdt_test.go +++ b/libcontainer/configs/validate/intelrdt_test.go @@ -4,12 +4,10 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/intelrdt" ) func TestValidateIntelRdt(t *testing.T) { - // Call init to trigger the sync.Once and enable overriding the rdt status - intelRdt.init() - testCases := []struct { name string rdtEnabled bool @@ -90,11 +88,17 @@ func TestValidateIntelRdt(t *testing.T) { }, }, } + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - intelRdt.rdtEnabled = tc.rdtEnabled - intelRdt.catEnabled = tc.catEnabled - intelRdt.mbaEnabled = tc.mbaEnabled + t.Cleanup(func() { + intelRdtIsEnabled = intelrdt.IsEnabled + intelRdtIsCATEnabled = intelrdt.IsCATEnabled + intelRdtIsMBAEnabled = intelrdt.IsMBAEnabled + }) + intelRdtIsEnabled = func() bool { return tc.rdtEnabled } + intelRdtIsCATEnabled = func() bool { return tc.catEnabled } + intelRdtIsMBAEnabled = func() bool { return tc.mbaEnabled } config := &configs.Config{ Rootfs: "/var", diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index dc61349917e..251900cb5df 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -306,22 +306,22 @@ func sysctl(config *configs.Config) error { } func intelrdtCheck(config *configs.Config) error { - if config.IntelRdt != nil { - if !intelRdt.isEnabled() { - return fmt.Errorf("intelRdt is specified in config, but Intel RDT is not enabled") - } - - switch clos := config.IntelRdt.ClosID; { - case clos == ".", clos == "..", len(clos) > 1 && strings.Contains(clos, "/"): - return fmt.Errorf("invalid intelRdt.ClosID %q", clos) - } + if config.IntelRdt == nil { + return nil + } + if !intelRdtIsEnabled() { + return fmt.Errorf("intelRdt is specified in config, but Intel RDT is not enabled") + } - if !intelRdt.isCATEnabled() && config.IntelRdt.L3CacheSchema != "" { - return errors.New("intelRdt.l3CacheSchema is specified in config, but Intel RDT/CAT is not enabled") - } - if !intelRdt.isMBAEnabled() && config.IntelRdt.MemBwSchema != "" { - return errors.New("intelRdt.memBwSchema is specified in config, but Intel RDT/MBA is not enabled") - } + switch clos := config.IntelRdt.ClosID; { + case clos == ".", clos == "..", len(clos) > 1 && strings.Contains(clos, "/"): + return fmt.Errorf("invalid intelRdt.ClosID %q", clos) + } + if !intelRdtIsCATEnabled() && config.IntelRdt.L3CacheSchema != "" { + return errors.New("intelRdt.l3CacheSchema is specified in config, but Intel RDT/CAT is not enabled") + } + if !intelRdtIsMBAEnabled() && config.IntelRdt.MemBwSchema != "" { + return errors.New("intelRdt.memBwSchema is specified in config, but Intel RDT/MBA is not enabled") } return nil diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 40e738b031b..4068c394fa7 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -162,7 +162,7 @@ func NewManager(config *configs.Config, id, path string) *Manager { return nil } - rootPath, err := Root() + rootPath, err := root() if err != nil { return nil } @@ -180,12 +180,6 @@ func NewManager(config *configs.Config, id, path string) *Manager { } } - return newManager(config, id, path) -} - -// newManager is the same as NewManager, except it does not check if the feature -// is actually available. Used by unit tests that mock intelrdt paths. -func newManager(config *configs.Config, id, path string) *Manager { return &Manager{ config: config, id: id, @@ -203,47 +197,42 @@ var ( // The flag to indicate if Intel RDT/MBA is enabled mbaEnabled bool - // For Intel RDT initialization - initOnce sync.Once - errNotFound = errors.New("Intel RDT not available") ) // Check if Intel RDT sub-features are enabled in featuresInit() -func featuresInit() { - initOnce.Do(func() { - // 1. Check if Intel RDT "resource control" filesystem is available. - // The user guarantees to mount the filesystem. - root, err := Root() - if err != nil { - return - } +var featuresInit = sync.OnceFunc(func() { + // 1. Check if Intel RDT "resource control" filesystem is available. + // The user guarantees to mount the filesystem. + root, err := root() + if err != nil { + return + } - // 2. Check if Intel RDT sub-features are available in "resource - // control" filesystem. Intel RDT sub-features can be - // selectively disabled or enabled by kernel command line - // (e.g., rdt=!l3cat,mba) in 4.14 and newer kernel - if _, err := os.Stat(filepath.Join(root, "info", "L3")); err == nil { - catEnabled = true - } - if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil { - mbaEnabled = true - } - if _, err := os.Stat(filepath.Join(root, "info", "L3_MON")); err != nil { - return - } - enabledMonFeatures, err = getMonFeatures(root) - if err != nil { - return - } - if enabledMonFeatures.mbmTotalBytes || enabledMonFeatures.mbmLocalBytes { - mbmEnabled = true - } - if enabledMonFeatures.llcOccupancy { - cmtEnabled = true - } - }) -} + // 2. Check if Intel RDT sub-features are available in "resource + // control" filesystem. Intel RDT sub-features can be + // selectively disabled or enabled by kernel command line + // (e.g., rdt=!l3cat,mba) in 4.14 and newer kernel + if _, err := os.Stat(filepath.Join(root, "info", "L3")); err == nil { + catEnabled = true + } + if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil { + mbaEnabled = true + } + if _, err := os.Stat(filepath.Join(root, "info", "L3_MON")); err != nil { + return + } + enabledMonFeatures, err = getMonFeatures(root) + if err != nil { + return + } + if enabledMonFeatures.mbmTotalBytes || enabledMonFeatures.mbmLocalBytes { + mbmEnabled = true + } + if enabledMonFeatures.llcOccupancy { + cmtEnabled = true + } +}) // findIntelRdtMountpointDir returns the mount point of the Intel RDT "resource control" filesystem. func findIntelRdtMountpointDir() (string, error) { @@ -264,13 +253,6 @@ func findIntelRdtMountpointDir() (string, error) { return mi[0].Mountpoint, nil } -// For Root() use only. -var ( - intelRdtRoot string - intelRdtRootErr error - rootOnce sync.Once -) - // The kernel creates this (empty) directory if resctrl is supported by the // hardware and kernel. The user is responsible for mounting the resctrl // filesystem, and they could mount it somewhere else if they wanted to. @@ -278,29 +260,27 @@ const defaultResctrlMountpoint = "/sys/fs/resctrl" // Root returns the Intel RDT "resource control" filesystem mount point. func Root() (string, error) { - rootOnce.Do(func() { - // Does this system support resctrl? - var statfs unix.Statfs_t - if err := unix.Statfs(defaultResctrlMountpoint, &statfs); err != nil { - if errors.Is(err, unix.ENOENT) { - err = errNotFound - } - intelRdtRootErr = err - return - } + return root() +} - // Has the resctrl fs been mounted to the default mount point? - if statfs.Type == unix.RDTGROUP_SUPER_MAGIC { - intelRdtRoot = defaultResctrlMountpoint - return +var root = sync.OnceValues(func() (string, error) { + // Does this system support resctrl? + var statfs unix.Statfs_t + if err := unix.Statfs(defaultResctrlMountpoint, &statfs); err != nil { + if errors.Is(err, unix.ENOENT) { + err = errNotFound } + return "", err + } - // The resctrl fs could have been mounted somewhere nonstandard. - intelRdtRoot, intelRdtRootErr = findIntelRdtMountpointDir() - }) + // Has the resctrl fs been mounted to the default mount point? + if statfs.Type == unix.RDTGROUP_SUPER_MAGIC { + return defaultResctrlMountpoint, nil + } - return intelRdtRoot, intelRdtRootErr -} + // The resctrl fs could have been mounted somewhere nonstandard. + return findIntelRdtMountpointDir() +}) // Gets a single uint64 value from the specified file. func getIntelRdtParamUint(path, file string) (uint64, error) { @@ -331,7 +311,7 @@ func getIntelRdtParamString(path, file string) (string, error) { func getL3CacheInfo() (*L3CacheInfo, error) { l3CacheInfo := &L3CacheInfo{} - rootPath, err := Root() + rootPath, err := root() if err != nil { return l3CacheInfo, err } @@ -361,7 +341,7 @@ func getL3CacheInfo() (*L3CacheInfo, error) { func getMemBwInfo() (*MemBwInfo, error) { memBwInfo := &MemBwInfo{} - rootPath, err := Root() + rootPath, err := root() if err != nil { return memBwInfo, err } @@ -394,7 +374,7 @@ func getMemBwInfo() (*MemBwInfo, error) { // Get diagnostics for last filesystem operation error from file info/last_cmd_status func getLastCmdStatus() (string, error) { - rootPath, err := Root() + rootPath, err := root() if err != nil { return "", err } @@ -425,7 +405,7 @@ func WriteIntelRdtTasks(dir string, pid int) error { // IsEnabled checks if Intel RDT is enabled. func IsEnabled() bool { - fsroot, err := Root() + fsroot, err := root() return err == nil && fsroot != "" } @@ -544,7 +524,7 @@ func (m *Manager) GetStats() (*Stats, error) { defer m.mu.Unlock() stats := newStats() - rootPath, err := Root() + rootPath, err := root() if err != nil { return nil, err } diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index 1ed05860e38..94b3bc0a34e 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -105,15 +105,20 @@ func TestIntelRdtSet(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - helper := NewIntelRdtTestUtil(t) - helper.config.IntelRdt = tc.config + intelRdtRoot := fakeRoot(t) + config := &configs.Config{ + IntelRdt: tc.config, + } - intelrdt := newManager(helper.config, "", helper.IntelRdtPath) - if err := intelrdt.Set(helper.config); err != nil { + intelrdt := &Manager{ + config: config, + path: intelRdtRoot, + } + if err := intelrdt.Set(config); err != nil { t.Fatal(err) } - tmpStrings, err := getIntelRdtParamString(helper.IntelRdtPath, "schemata") + tmpStrings, err := getIntelRdtParamString(intelRdtRoot, "schemata") if err != nil { t.Fatalf("Failed to parse file 'schemata' - %s", err) } @@ -183,7 +188,7 @@ func TestApply(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - NewIntelRdtTestUtil(t) + intelRdtRoot := fakeRoot(t) id := "abcd-1234" closPath := filepath.Join(intelRdtRoot, id) if tt.config.ClosID != "" { @@ -195,7 +200,11 @@ func TestApply(t *testing.T) { t.Fatal(err) } } - m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath) + m := &Manager{ + config: &configs.Config{IntelRdt: &tt.config}, + id: id, + path: closPath, + } err := m.Apply(pid) if tt.isError && err == nil { t.Fatal("expected error, got nil") @@ -281,8 +290,7 @@ func TestDestroy(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - NewIntelRdtTestUtil(t) - + intelRdtRoot := fakeRoot(t) id := "abcd-1234" closPath := filepath.Join(intelRdtRoot, id) if tt.config.ClosID != "" { @@ -292,7 +300,11 @@ func TestDestroy(t *testing.T) { t.Fatal(err) } } - m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath) + m := &Manager{ + config: &configs.Config{IntelRdt: &tt.config}, + id: id, + path: closPath, + } if err := m.Apply(1234); err != nil { t.Fatalf("Apply() failed: %v", err) } diff --git a/libcontainer/intelrdt/util_test.go b/libcontainer/intelrdt/util_test.go index 1771a231b4a..daa647fe9be 100644 --- a/libcontainer/intelrdt/util_test.go +++ b/libcontainer/intelrdt/util_test.go @@ -8,35 +8,25 @@ import ( "os" "path/filepath" "testing" - - "github.com/opencontainers/runc/libcontainer/configs" ) -type intelRdtTestUtil struct { - config *configs.Config - - // Path to the mock Intel RDT "resource control" filesystem directory - IntelRdtPath string - - t *testing.T -} - -// Creates a new test util -func NewIntelRdtTestUtil(t *testing.T) *intelRdtTestUtil { - config := &configs.Config{ - IntelRdt: &configs.IntelRdt{}, - } - +// fakeRoot creates a new fake root for tests and returns its path. +// Once this is called, Root() returns the same path. +func fakeRoot(t *testing.T) string { // Assign fake intelRtdRoot value, returned by Root(). - intelRdtRoot = t.TempDir() - // Make sure Root() won't even try to parse mountinfo. - rootOnce.Do(func() {}) + intelRdtRoot := filepath.Join(t.TempDir(), "resctrl") + if err := os.MkdirAll(intelRdtRoot, 0o755); err != nil { + t.Fatal(err) + } - testIntelRdtPath := filepath.Join(intelRdtRoot, "resctrl") + origRoot := root + t.Cleanup(func() { + root = origRoot + }) - // Ensure the full mock Intel RDT "resource control" filesystem path exists - if err := os.MkdirAll(testIntelRdtPath, 0o755); err != nil { - t.Fatal(err) + root = func() (string, error) { + return intelRdtRoot, nil } - return &intelRdtTestUtil{config: config, IntelRdtPath: testIntelRdtPath, t: t} + + return intelRdtRoot } diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index 91264f0d3ad..53131a79f04 100644 --- a/libcontainer/utils/utils_unix.go +++ b/libcontainer/utils/utils_unix.go @@ -18,29 +18,21 @@ import ( "github.com/opencontainers/runc/internal/pathrs" ) -var ( - haveCloseRangeCloexecBool bool - haveCloseRangeCloexecOnce sync.Once -) +var haveCloseRangeCloexec = sync.OnceValue(func() bool { + // Make sure we're not closing a random file descriptor. + tmpFd, err := unix.FcntlInt(0, unix.F_DUPFD_CLOEXEC, 0) + if err != nil { + return false + } + defer unix.Close(tmpFd) -func haveCloseRangeCloexec() bool { - haveCloseRangeCloexecOnce.Do(func() { - // Make sure we're not closing a random file descriptor. - tmpFd, err := unix.FcntlInt(0, unix.F_DUPFD_CLOEXEC, 0) - if err != nil { - return - } - defer unix.Close(tmpFd) - - err = unix.CloseRange(uint(tmpFd), uint(tmpFd), unix.CLOSE_RANGE_CLOEXEC) - // Any error means we cannot use close_range(CLOSE_RANGE_CLOEXEC). - // -ENOSYS and -EINVAL ultimately mean we don't have support, but any - // other potential error would imply that even the most basic close - // operation wouldn't work. - haveCloseRangeCloexecBool = err == nil - }) - return haveCloseRangeCloexecBool -} + err = unix.CloseRange(uint(tmpFd), uint(tmpFd), unix.CLOSE_RANGE_CLOEXEC) + // Any error means we cannot use close_range(CLOSE_RANGE_CLOEXEC). + // -ENOSYS and -EINVAL ultimately mean we don't have support, but any + // other potential error would imply that even the most basic close + // operation wouldn't work. + return err == nil +}) type fdFunc func(fd int) @@ -162,10 +154,13 @@ func WithProcfdFile(file *os.File, fn func(procfd string) error) error { type ProcThreadSelfCloser func() -var ( - haveProcThreadSelf bool - haveProcThreadSelfOnce sync.Once -) +var haveProcThreadSelf = sync.OnceValue(func() bool { + if _, err := os.Stat("/proc/thread-self/"); err != nil { + logrus.Debugf("cannot stat /proc/thread-self (%v), falling back to /proc/self/task/", err) + return false + } + return true +}) // ProcThreadSelf returns a string that is equivalent to // /proc/thread-self/, with a graceful fallback on older kernels where @@ -174,14 +169,6 @@ var ( // 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/", 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 @@ -196,7 +183,7 @@ func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) { runtime.LockOSThread() threadSelf := "/proc/thread-self/" - if !haveProcThreadSelf { + 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 {