Merge pull request #3980 from cyphar/timens-cleanups

timens: minor cleanups

libcontainer/configs/validate/validator.go

@@ -120,6 +120,10 @@ func namespaces(config *configs.Config) error {
 		if _, err := os.Stat("/proc/self/timens_offsets"); os.IsNotExist(err) {
 			return errors.New("time namespaces aren't enabled in the kernel")
 		}
+	} else {
+		if config.TimeOffsets != nil {
+			return errors.New("time namespace offsets specified, but time namespace isn't enabled in the config")
+		}
 	}
 
 	return nil

libcontainer/configs/validate/validator_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runtime-spec/specs-go"
 	"golang.org/x/sys/unix"
 )
 
@@ -201,6 +202,40 @@ func TestValidateUsernamespaceWithoutUserNS(t *testing.T) {
 	}
 }
 
+func TestValidateTimeNamespace(t *testing.T) {
+	if _, err := os.Stat("/proc/self/ns/time"); os.IsNotExist(err) {
+		t.Skip("Test requires timens.")
+	}
+	config := &configs.Config{
+		Rootfs: "/var",
+		Namespaces: configs.Namespaces(
+			[]configs.Namespace{
+				{Type: configs.NEWTIME},
+			},
+		),
+	}
+
+	err := Validate(config)
+	if err != nil {
+		t.Errorf("expected error to not occur %+v", err)
+	}
+}
+
+func TestValidateTimeOffsetsWithoutTimeNamespace(t *testing.T) {
+	config := &configs.Config{
+		Rootfs: "/var",
+		TimeOffsets: map[string]specs.LinuxTimeOffset{
+			"boottime":  {Secs: 150, Nanosecs: 314159},
+			"monotonic": {Secs: 512, Nanosecs: 271818},
+		},
+	}
+
+	err := Validate(config)
+	if err == nil {
+		t.Error("Expected error to occur but it was nil")
+	}
+}
+
 // TestConvertSysctlVariableToDotsSeparator tests whether the sysctl variable
 // can be correctly converted to a dot as a separator.
 func TestConvertSysctlVariableToDotsSeparator(t *testing.T) {

libcontainer/container_linux.go

@@ -1192,7 +1192,7 @@ func (c *Container) bootstrapData(cloneFlags uintptr, nsMaps map[configs.Namespa
 	}
 
 	// write boottime and monotonic time ns offsets.
-	if c.config.Namespaces.Contains(configs.NEWTIME) && c.config.TimeOffsets != nil {
+	if c.config.TimeOffsets != nil {
 		var offsetSpec bytes.Buffer
 		for clock, offset := range c.config.TimeOffsets {
 			fmt.Fprintf(&offsetSpec, "%s %d %d\n", clock, offset.Secs, offset.Nanosecs)

libcontainer/nsenter/nsexec.c

@@ -758,15 +758,13 @@ void receive_idmapsources(int sockfd)
 	receive_fd_sources(sockfd, "_LIBCONTAINER_IDMAP_FDS");
 }
 
-static void update_timens(char *map, size_t map_len)
+static void update_timens_offsets(char *map, size_t map_len)
 {
 	if (map == NULL || map_len == 0)
 		return;
 	write_log(DEBUG, "update /proc/self/timens_offsets to '%s'", map);
-	if (write_file(map, map_len, "/proc/self/timens_offsets") < 0) {
-		if (errno != EPERM)
-			bail("failed to update /proc/self/timens_offsets");
-	}
+	if (write_file(map, map_len, "/proc/self/timens_offsets") < 0)
+		bail("failed to update /proc/self/timens_offsets");
 }
 
 void nsexec(void)
@@ -1174,6 +1172,7 @@ void nsexec(void)
 			 * was broken, so we'll just do it the long way anyway.
 			 */
 			try_unshare(config.cloneflags & ~CLONE_NEWCGROUP, "remaining namespaces (except cgroupns)");
+			update_timens_offsets(config.timensoffset, config.timensoffset_len);
 
 			/* Ask our parent to send the mount sources fds. */
 			if (config.mountsources) {
@@ -1207,11 +1206,6 @@ void nsexec(void)
 					bail("failed to sync with parent: SYNC_MOUNT_IDMAP_ACK: got %u", s);
 			}
 
-			/*
-			 * set boottime and monotonic timens offsets.
-			 */
-			update_timens(config.timensoffset, config.timensoffset_len);
-
 			/*
 			 * TODO: What about non-namespace clone flags that we're dropping here?
 			 *

libcontainer/specconv/spec_linux.go

@@ -422,6 +422,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 		config.ReadonlyPaths = spec.Linux.ReadonlyPaths
 		config.MountLabel = spec.Linux.MountLabel
 		config.Sysctl = spec.Linux.Sysctl
+		config.TimeOffsets = spec.Linux.TimeOffsets
 		if spec.Linux.Seccomp != nil {
 			seccomp, err := SetupSeccomp(spec.Linux.Seccomp)
 			if err != nil {
@@ -436,9 +437,6 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 				MemBwSchema:   spec.Linux.IntelRdt.MemBwSchema,
 			}
 		}
-
-		// update timens offsets
-		config.TimeOffsets = spec.Linux.TimeOffsets
 	}
 
 	// Set the host UID that should own the container's cgroup.

tests/integration/helpers.bash

@@ -429,6 +429,11 @@ function requires() {
 				skip_me=1
 			fi
 			;;
+		timens)
+			if [ ! -e "/proc/self/ns/time" ]; then
+				skip_me=1
+			fi
+			;;
 		cgroups_v1)
 			init_cgroup_paths
 			if [ ! -v CGROUP_V1 ]; then

tests/integration/timens.bats

@@ -0,0 +1,55 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	setup_busybox
+}
+
+function teardown() {
+	teardown_bundle
+}
+
+@test "runc run [timens offsets with no timens]" {
+	requires timens
+
+	update_config '.process.args = ["cat", "/proc/self/timens_offsets"]'
+	update_config '.linux.namespaces = .linux.namespace | map(select(.type != "time"))'
+	update_config '.linux.timeOffsets = {
+			"monotonic": { "secs": 7881, "nanosecs": 2718281 },
+			"boottime": { "secs": 1337, "nanosecs": 3141519 }
+		}'
+
+	runc run test_busybox
+	[ "$status" -ne 0 ]
+}
+
+@test "runc run [timens with no offsets]" {
+	requires timens
+
+	update_config '.process.args = ["cat", "/proc/self/timens_offsets"]'
+	update_config '.linux.namespaces += [{"type": "time"}]
+		| .linux.timeOffsets = null'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	# Default offsets are 0.
+	grep -E '^monotonic\s+0\s+0$' <<<"$output"
+	grep -E '^boottime\s+0\s+0$' <<<"$output"
+}
+
+@test "runc run [simple timens]" {
+	requires timens
+
+	update_config '.process.args = ["cat", "/proc/self/timens_offsets"]'
+	update_config '.linux.namespaces += [{"type": "time"}]
+		| .linux.timeOffsets = {
+			"monotonic": { "secs": 7881, "nanosecs": 2718281 },
+			"boottime": { "secs": 1337, "nanosecs": 3141519 }
+		}'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	grep -E '^monotonic\s+7881\s+2718281$' <<<"$output"
+	grep -E '^boottime\s+1337\s+3141519$' <<<"$output"
+}