libct/cg: introduce and use fscommon.OpenFile

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

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

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 {

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 {

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

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
 	}

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
 	}

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
 	}

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

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)
+}

libcontainer/cgroups/utils.go

@@ -367,14 +367,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
 		}