merge branch 'pr-2855'

Kir Kolyshkin (2):
  start: don't kill runc init too early
  libct/configs/validator: add some cgroup support

LGTMs: @AkihiroSuda @cyphar
Closes #2855

libcontainer/configs/validate/validator.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/intelrdt"
 	selinux "github.com/opencontainers/selinux/go-selinux"
@@ -55,6 +56,10 @@ func (v *ConfigValidator) Validate(config *configs.Config) error {
 			return err
 		}
 	}
+	if err := v.cgroups(config); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -223,6 +228,35 @@ func (v *ConfigValidator) intelrdt(config *configs.Config) error {
 	return nil
 }
 
+func (v *ConfigValidator) cgroups(config *configs.Config) error {
+	c := config.Cgroups
+	if c == nil {
+		return nil
+	}
+
+	if (c.Name != "" || c.Parent != "") && c.Path != "" {
+		return fmt.Errorf("cgroup: either Path or Name and Parent should be used, got %+v", c)
+	}
+
+	r := c.Resources
+	if r == nil {
+		return nil
+	}
+
+	if !cgroups.IsCgroup2UnifiedMode() && r.Unified != nil {
+		return cgroups.ErrV1NoUnified
+	}
+
+	if cgroups.IsCgroup2UnifiedMode() {
+		_, err := cgroups.ConvertMemorySwapToCgroupV2Value(r.MemorySwap, r.Memory)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func isHostNetNS(path string) (bool, error) {
 	const currentProcessNetns = "/proc/self/ns/net"
 

libcontainer/nsenter/nsenter_test.go

@@ -3,6 +3,7 @@ package nsenter
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -48,6 +49,7 @@ func TestNsenterValidPaths(t *testing.T) {
 	if err := cmd.Start(); err != nil {
 		t.Fatalf("nsenter failed to start %v", err)
 	}
+
 	// write cloneFlags
 	r := nl.NewNetlinkRequest(int(libcontainer.InitMsg), 0)
 	r.AddData(&libcontainer.Int32msg{
@@ -62,6 +64,8 @@ func TestNsenterValidPaths(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	initWaiter(t, parent)
+
 	decoder := json.NewDecoder(parent)
 	var pid *pid
 
@@ -118,6 +122,7 @@ func TestNsenterInvalidPaths(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	initWaiter(t, parent)
 	if err := cmd.Wait(); err == nil {
 		t.Fatalf("nsenter exits with a zero exit status")
 	}
@@ -158,6 +163,7 @@ func TestNsenterIncorrectPathType(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	initWaiter(t, parent)
 	if err := cmd.Wait(); err == nil {
 		t.Fatalf("nsenter exits with a zero exit status")
 	}
@@ -206,6 +212,8 @@ func TestNsenterChildLogging(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	initWaiter(t, parent)
+
 	logsDecoder := json.NewDecoder(logread)
 	var logentry *logentry
 
@@ -235,3 +243,19 @@ func newPipe() (parent *os.File, child *os.File, err error) {
 	}
 	return os.NewFile(uintptr(fds[1]), "parent"), os.NewFile(uintptr(fds[0]), "child"), nil
 }
+
+// initWaiter reads back the initial \0 from runc init
+func initWaiter(t *testing.T, r io.Reader) {
+	inited := make([]byte, 1)
+	n, err := r.Read(inited)
+	if err == nil {
+		if n < 1 {
+			err = errors.New("short read")
+		} else if inited[0] != 0 {
+			err = fmt.Errorf("unexpected %d != 0", inited[0])
+		} else {
+			return
+		}
+	}
+	t.Fatalf("waiting for init preliminary setup: %v", err)
+}

libcontainer/nsenter/nsexec.c

@@ -598,6 +598,13 @@ void nsexec(void)
 	if (ensure_cloned_binary() < 0)
 		bail("could not ensure we are a cloned binary");
 
+	/*
+	 * Inform the parent we're past initial setup.
+	 * For the other side of this, see initWaiter.
+	 */
+	if (write(pipenum, "", 1) != 1)
+		bail("could not inform the parent we are past initial setup");
+
 	write_log(DEBUG, "nsexec started");
 
 	/* Parse all of the netlink configuration. */

libcontainer/process_linux.go

@@ -98,23 +98,34 @@ func (p *setnsProcess) start() (retErr error) {
 	if err != nil {
 		return newSystemErrorWithCause(err, "starting setns process")
 	}
+
+	waitInit := initWaiter(p.messageSockPair.parent)
 	defer func() {
 		if retErr != nil {
 			if newOom, err := p.manager.OOMKillCount(); err == nil && newOom != oom {
 				// Someone in this cgroup was killed, this _might_ be us.
 				retErr = newSystemErrorWithCause(retErr, "possibly OOM-killed")
 			}
+			werr := <-waitInit
+			if werr != nil {
+				logrus.WithError(werr).Warn()
+			}
 			err := ignoreTerminateErrors(p.terminate())
 			if err != nil {
 				logrus.WithError(err).Warn("unable to terminate setnsProcess")
 			}
 		}
 	}()
+
 	if p.bootstrapData != nil {
 		if _, err := io.Copy(p.messageSockPair.parent, p.bootstrapData); err != nil {
 			return newSystemErrorWithCause(err, "copying bootstrap data to pipe")
 		}
 	}
+	err = <-waitInit
+	if err != nil {
+		return err
+	}
 	if err := p.execSetns(); err != nil {
 		return newSystemErrorWithCause(err, "executing setns process")
 	}
@@ -326,9 +337,13 @@ func (p *initProcess) start() (retErr error) {
 		p.process.ops = nil
 		return newSystemErrorWithCause(err, "starting init process command")
 	}
+
+	waitInit := initWaiter(p.messageSockPair.parent)
 	defer func() {
 		if retErr != nil {
-			// init might be killed by the kernel's OOM killer.
+			// Find out if init is killed by the kernel's OOM killer.
+			// Get the count before killing init as otherwise cgroup
+			// might be removed by systemd.
 			oom, err := p.manager.OOMKillCount()
 			if err != nil {
 				logrus.WithError(err).Warn("unable to get oom kill count")
@@ -346,7 +361,12 @@ func (p *initProcess) start() (retErr error) {
 				}
 			}
 
-			// terminate the process to ensure we can remove cgroups
+			werr := <-waitInit
+			if werr != nil {
+				logrus.WithError(werr).Warn()
+			}
+
+			// Terminate the process to ensure we can remove cgroups.
 			if err := ignoreTerminateErrors(p.terminate()); err != nil {
 				logrus.WithError(err).Warn("unable to terminate initProcess")
 			}
@@ -372,6 +392,11 @@ func (p *initProcess) start() (retErr error) {
 	if _, err := io.Copy(p.messageSockPair.parent, p.bootstrapData); err != nil {
 		return newSystemErrorWithCause(err, "copying bootstrap data to pipe")
 	}
+	err = <-waitInit
+	if err != nil {
+		return err
+	}
+
 	childPid, err := p.getChildPid()
 	if err != nil {
 		return newSystemErrorWithCause(err, "getting the final child's pid from pipe")
@@ -674,3 +699,28 @@ func (p *Process) InitializeIO(rootuid, rootgid int) (i *IO, err error) {
 	}
 	return i, nil
 }
+
+// initWaiter returns a channel to wait on for making sure
+// runc init has finished the initial setup.
+func initWaiter(r io.Reader) chan error {
+	ch := make(chan error, 1)
+	go func() {
+		defer close(ch)
+
+		inited := make([]byte, 1)
+		n, err := r.Read(inited)
+		if err == nil {
+			if n < 1 {
+				err = errors.New("short read")
+			} else if inited[0] != 0 {
+				err = fmt.Errorf("unexpected %d != 0", inited[0])
+			} else {
+				ch <- nil
+				return
+			}
+		}
+		ch <- newSystemErrorWithCause(err, "waiting for init preliminary setup")
+	}()
+
+	return ch
+}