From 0228226e6d470e4d7dcfb616e7d1c4644ba97001 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Tue, 22 Sep 2020 19:59:18 -0700
Subject: [PATCH] libcontainer/cgroups/fscommon: introduce OpenFile

Move the functionality of opening a cgroup file into a separate
function, OpenFile, which, similar to ReadFile and WriteFile,
use separate dir and file arguments.

Change ReadFile and WriteFile to rely on OpenFile, and use lower-level
read and write instead of ones from ioutil.

It changes the semantics of WriteFile a bit -- it no longer uses
O_CREAT flag. This is good for real cgroup as there is no need to try
creating the files in there, but can potentially break WriteFile users
-- previously, EPERM error was returned for non-existing files, and
now it's ENOENT.

This also breaks the fs/fs2 unit tests since they write to pseudo-cgroup
files inside a test directory (not to a real cgroup fs), and now
fscommon.WriteFile do not create or truncate files, so we have to add a
variable that is set by the unit tests.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/util_test.go      |  4 +++
 libcontainer/cgroups/fscommon/fscommon.go | 28 +++++++++----------
 libcontainer/cgroups/fscommon/open.go     | 33 +++++++++++++++++++++++
 3 files changed, 49 insertions(+), 16 deletions(-)
 create mode 100644 libcontainer/cgroups/fscommon/open.go

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