diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index 031a6bbda..4d5e5beb4 100644 --- a/libcontainer/cgroups/fs/blkio.go +++ b/libcontainer/cgroups/fs/blkio.go @@ -6,7 +6,6 @@ import ( "bufio" "fmt" "os" - "path/filepath" "strconv" "strings" @@ -105,9 +104,9 @@ func splitBlkioStatLine(r rune) bool { return r == ' ' || r == ':' } -func getBlkioStat(path string) ([]cgroups.BlkioStatEntry, error) { +func getBlkioStat(dir, file string) ([]cgroups.BlkioStatEntry, error) { var blkioStats []cgroups.BlkioStatEntry - f, err := os.Open(path) + f, err := fscommon.OpenFile(dir, file, os.O_RDONLY) if err != nil { if os.IsNotExist(err) { return blkioStats, nil @@ -125,7 +124,7 @@ func getBlkioStat(path string) ([]cgroups.BlkioStatEntry, error) { // skip total line continue } else { - return nil, fmt.Errorf("Invalid line found while parsing %s: %s", path, sc.Text()) + return nil, fmt.Errorf("Invalid line found while parsing %s/%s: %s", dir, file, sc.Text()) } } @@ -159,7 +158,7 @@ func getBlkioStat(path string) ([]cgroups.BlkioStatEntry, error) { func (s *BlkioGroup) GetStats(path string, stats *cgroups.Stats) error { // Try to read CFQ stats available on all CFQ enabled kernels first - if blkioStats, err := getBlkioStat(filepath.Join(path, "blkio.io_serviced_recursive")); err == nil && blkioStats != nil { + if blkioStats, err := getBlkioStat(path, "blkio.io_serviced_recursive"); err == nil && blkioStats != nil { return getCFQStats(path, stats) } return getStats(path, stats) // Use generic stats as fallback @@ -169,42 +168,42 @@ func getCFQStats(path string, stats *cgroups.Stats) error { var blkioStats []cgroups.BlkioStatEntry var err error - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.sectors_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.sectors_recursive"); err != nil { return err } stats.BlkioStats.SectorsRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.io_service_bytes_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.io_service_bytes_recursive"); err != nil { return err } stats.BlkioStats.IoServiceBytesRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.io_serviced_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.io_serviced_recursive"); err != nil { return err } stats.BlkioStats.IoServicedRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.io_queued_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.io_queued_recursive"); err != nil { return err } stats.BlkioStats.IoQueuedRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.io_service_time_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.io_service_time_recursive"); err != nil { return err } stats.BlkioStats.IoServiceTimeRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.io_wait_time_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.io_wait_time_recursive"); err != nil { return err } stats.BlkioStats.IoWaitTimeRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.io_merged_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.io_merged_recursive"); err != nil { return err } stats.BlkioStats.IoMergedRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.time_recursive")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.time_recursive"); err != nil { return err } stats.BlkioStats.IoTimeRecursive = blkioStats @@ -216,12 +215,12 @@ func getStats(path string, stats *cgroups.Stats) error { var blkioStats []cgroups.BlkioStatEntry var err error - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.throttle.io_service_bytes")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.throttle.io_service_bytes"); err != nil { return err } stats.BlkioStats.IoServiceBytesRecursive = blkioStats - if blkioStats, err = getBlkioStat(filepath.Join(path, "blkio.throttle.io_serviced")); err != nil { + if blkioStats, err = getBlkioStat(path, "blkio.throttle.io_serviced"); err != nil { return err } stats.BlkioStats.IoServicedRecursive = blkioStats diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go index 6fb1c2078..1d5f455d6 100644 --- a/libcontainer/cgroups/fs/cpu.go +++ b/libcontainer/cgroups/fs/cpu.go @@ -6,7 +6,6 @@ import ( "bufio" "fmt" "os" - "path/filepath" "strconv" "github.com/opencontainers/runc/libcontainer/cgroups" @@ -87,7 +86,7 @@ func (s *CpuGroup) Set(path string, cgroup *configs.Cgroup) error { } func (s *CpuGroup) GetStats(path string, stats *cgroups.Stats) error { - f, err := os.Open(filepath.Join(path, "cpu.stat")) + f, err := fscommon.OpenFile(path, "cpu.stat", os.O_RDONLY) if err != nil { if os.IsNotExist(err) { return nil diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go index 9ee1119cc..129de6a01 100644 --- a/libcontainer/cgroups/fs/cpuacct.go +++ b/libcontainer/cgroups/fs/cpuacct.go @@ -135,7 +135,7 @@ func getPercpuUsageInModes(path string) ([]uint64, []uint64, error) { usageKernelMode := []uint64{} usageUserMode := []uint64{} - file, err := os.Open(filepath.Join(path, cgroupCpuacctUsageAll)) + file, err := fscommon.OpenFile(path, cgroupCpuacctUsageAll, os.O_RDONLY) if os.IsNotExist(err) { return usageKernelMode, usageUserMode, nil } else if err != nil { diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 2a2f0dd6c..7fde663d9 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -7,7 +7,6 @@ import ( "fmt" "math" "os" - "path" "path/filepath" "strconv" "strings" @@ -160,7 +159,7 @@ func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error { func (s *MemoryGroup) GetStats(path string, stats *cgroups.Stats) error { // Set stats from memory.stat. - statsFile, err := os.Open(filepath.Join(path, "memory.stat")) + statsFile, err := fscommon.OpenFile(path, "memory.stat", os.O_RDONLY) if err != nil { if os.IsNotExist(err) { return nil @@ -280,7 +279,7 @@ func getMemoryData(path, name string) (cgroups.MemoryData, error) { func getPageUsageByNUMA(cgroupPath string) (cgroups.PageUsageByNUMA, error) { stats := cgroups.PageUsageByNUMA{} - file, err := os.Open(path.Join(cgroupPath, cgroupMemoryPagesByNuma)) + file, err := fscommon.OpenFile(cgroupPath, cgroupMemoryPagesByNuma, os.O_RDONLY) if os.IsNotExist(err) { return stats, nil } else if err != nil { diff --git a/libcontainer/cgroups/fs/util_test.go b/libcontainer/cgroups/fs/util_test.go index 2c50d6f3a..32cbdfc94 100644 --- a/libcontainer/cgroups/fs/util_test.go +++ b/libcontainer/cgroups/fs/util_test.go @@ -17,6 +17,10 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) +func init() { + fscommon.TestMode = true +} + type cgroupTestUtil struct { // cgroup data to use in tests. CgroupData *cgroupData diff --git a/libcontainer/cgroups/fs2/cpu.go b/libcontainer/cgroups/fs2/cpu.go index edf1060ec..0dffe6648 100644 --- a/libcontainer/cgroups/fs2/cpu.go +++ b/libcontainer/cgroups/fs2/cpu.go @@ -5,7 +5,6 @@ package fs2 import ( "bufio" "os" - "path/filepath" "strconv" "github.com/opencontainers/runc/libcontainer/cgroups" @@ -50,7 +49,7 @@ func setCpu(dirPath string, cgroup *configs.Cgroup) error { return nil } func statCpu(dirPath string, stats *cgroups.Stats) error { - f, err := os.Open(filepath.Join(dirPath, "cpu.stat")) + f, err := fscommon.OpenFile(dirPath, "cpu.stat", os.O_RDONLY) if err != nil { return err } diff --git a/libcontainer/cgroups/fs2/io.go b/libcontainer/cgroups/fs2/io.go index bbe3ac064..e01ea001b 100644 --- a/libcontainer/cgroups/fs2/io.go +++ b/libcontainer/cgroups/fs2/io.go @@ -5,7 +5,6 @@ package fs2 import ( "bufio" "os" - "path/filepath" "strconv" "strings" @@ -60,8 +59,7 @@ func setIo(dirPath string, cgroup *configs.Cgroup) error { func readCgroup2MapFile(dirPath string, name string) (map[string][]string, error) { ret := map[string][]string{} - p := filepath.Join(dirPath, name) - f, err := os.Open(p) + f, err := fscommon.OpenFile(dirPath, name, os.O_RDONLY) if err != nil { return nil, err } diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go index 68f2546c7..1c6913bf0 100644 --- a/libcontainer/cgroups/fs2/memory.go +++ b/libcontainer/cgroups/fs2/memory.go @@ -5,7 +5,6 @@ package fs2 import ( "bufio" "os" - "path/filepath" "strconv" "github.com/opencontainers/runc/libcontainer/cgroups" @@ -75,7 +74,7 @@ func setMemory(dirPath string, cgroup *configs.Cgroup) error { func statMemory(dirPath string, stats *cgroups.Stats) error { // Set stats from memory.stat. - statsFile, err := os.Open(filepath.Join(dirPath, "memory.stat")) + statsFile, err := fscommon.OpenFile(dirPath, "memory.stat", os.O_RDONLY) if err != nil { return err } diff --git a/libcontainer/cgroups/fscommon/fscommon.go b/libcontainer/cgroups/fscommon/fscommon.go index 8f7298c88..d19673f77 100644 --- a/libcontainer/cgroups/fscommon/fscommon.go +++ b/libcontainer/cgroups/fscommon/fscommon.go @@ -3,10 +3,9 @@ package fscommon import ( - "io/ioutil" + "bytes" "os" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -15,14 +14,12 @@ import ( // WriteFile writes data to a cgroup file in dir. // It is supposed to be used for cgroup files only. func WriteFile(dir, file, data string) error { - if dir == "" { - return errors.Errorf("no directory specified for %s", file) - } - path, err := securejoin.SecureJoin(dir, file) + fd, err := OpenFile(dir, file, unix.O_WRONLY) if err != nil { return err } - if err := retryingWriteFile(path, []byte(data), 0700); err != nil { + defer fd.Close() + if err := retryingWriteFile(fd, data); err != nil { return errors.Wrapf(err, "failed to write %q", data) } return nil @@ -31,22 +28,21 @@ func WriteFile(dir, file, data string) error { // ReadFile reads data from a cgroup file in dir. // It is supposed to be used for cgroup files only. func ReadFile(dir, file string) (string, error) { - if dir == "" { - return "", errors.Errorf("no directory specified for %s", file) - } - path, err := securejoin.SecureJoin(dir, file) + fd, err := OpenFile(dir, file, unix.O_RDONLY) if err != nil { return "", err } - data, err := ioutil.ReadFile(path) - return string(data), err + var buf bytes.Buffer + + _, err = buf.ReadFrom(fd) + return buf.String(), err } -func retryingWriteFile(filename string, data []byte, perm os.FileMode) error { +func retryingWriteFile(fd *os.File, data string) error { for { - err := ioutil.WriteFile(filename, data, perm) + _, err := fd.Write([]byte(data)) if errors.Is(err, unix.EINTR) { - logrus.Infof("interrupted while writing %s to %s", string(data), filename) + logrus.Infof("interrupted while writing %s to %s", data, fd.Name()) continue } return err diff --git a/libcontainer/cgroups/fscommon/open.go b/libcontainer/cgroups/fscommon/open.go new file mode 100644 index 000000000..90560f548 --- /dev/null +++ b/libcontainer/cgroups/fscommon/open.go @@ -0,0 +1,33 @@ +package fscommon + +import ( + "os" + + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/pkg/errors" +) + +var ( + // Set to true by fs unit tests + TestMode bool +) + +// OpenFile opens a cgroup file in a given dir with given flags. +// It is supposed to be used for cgroup files only. +func OpenFile(dir, file string, flags int) (*os.File, error) { + if dir == "" { + return nil, errors.Errorf("no directory specified for %s", file) + } + mode := os.FileMode(0) + if TestMode && flags&os.O_WRONLY != 0 { + // "emulate" cgroup fs for unit tests + flags |= os.O_TRUNC | os.O_CREATE + mode = 0o600 + } + path, err := securejoin.SecureJoin(dir, file) + if err != nil { + return nil, err + } + + return os.OpenFile(path, flags, mode) +} diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 05018bd4b..4ca2de609 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -368,14 +368,14 @@ func WriteCgroupProc(dir string, pid int) error { return nil } - cgroupProcessesFile, err := os.OpenFile(filepath.Join(dir, CgroupProcesses), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0700) + file, err := fscommon.OpenFile(dir, CgroupProcesses, os.O_WRONLY) if err != nil { return fmt.Errorf("failed to write %v to %v: %v", pid, CgroupProcesses, err) } - defer cgroupProcessesFile.Close() + defer file.Close() for i := 0; i < 5; i++ { - _, err = cgroupProcessesFile.WriteString(strconv.Itoa(pid)) + _, err = file.WriteString(strconv.Itoa(pid)) if err == nil { return nil }