From 6ce2d63a5db6898aa657e2d4f6bf50ef31ef2690 Mon Sep 17 00:00:00 2001
From: Peter Hunt <pehunt@redhat.com>
Date: Tue, 6 Apr 2021 13:48:16 -0400
Subject: [PATCH] libct/init_linux: retry chdir to fix EPERM

Alas, the EPERM on chdir saga continues...

Unfortunately, the there were two releases between when https://github.com/opencontainers/runc/commit/5e0e67d76cc99d76c8228d48f38f37034503f315  was released
and when the workaround https://github.com/opencontainers/runc/pull/2712 was added.

Between this, folks started relying on the ability to have a workdir that the container user doesn't have access to.

Since this case was previously valid, we should continue support for it.

Now, we retry the chdir:
Once at the top of the function (to catch cases where the runc user has access, but container user does not)
and once after we setup user (to catch cases where the container user has access, and the runc user does not)

Add a test case for this as well.

Signed-off-by: Peter Hunt <pehunt@redhat.com>
---
 libcontainer/init_linux.go | 26 ++++++++++++++++++++++----
 tests/integration/cwd.bats | 18 +++++++++++++++++-
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go
index 00f28e1f7eb..91fb794663b 100644
--- a/libcontainer/init_linux.go
+++ b/libcontainer/init_linux.go
@@ -131,6 +131,26 @@ func finalizeNamespace(config *initConfig) error {
 		return errors.Wrap(err, "close exec fds")
 	}
 
+	// we only do chdir if it's specified
+	doChdir := config.Cwd != ""
+	if doChdir {
+		// First, attempt the chdir before setting up the user.
+		// This could allow us to access a directory that the user running runc can access
+		// but the container user cannot.
+		err := unix.Chdir(config.Cwd)
+		switch {
+		case err == nil:
+			doChdir = false
+		case os.IsPermission(err):
+			// If we hit an EPERM, we should attempt again after setting up user.
+			// This will allow us to successfully chdir if the container user has access
+			// to the directory, but the user running runc does not.
+			// This is useful in cases where the cwd is also a volume that's been chowned to the container user.
+		default:
+			return fmt.Errorf("chdir to cwd (%q) set in config.json failed: %v", config.Cwd, err)
+		}
+	}
+
 	caps := &configs.Capabilities{}
 	if config.Capabilities != nil {
 		caps = config.Capabilities
@@ -152,10 +172,8 @@ func finalizeNamespace(config *initConfig) error {
 	if err := setupUser(config); err != nil {
 		return errors.Wrap(err, "setup user")
 	}
-	// Change working directory AFTER the user has been set up.
-	// Otherwise, if the cwd is also a volume that's been chowned to the container user (and not the user running runc),
-	// this command will EPERM.
-	if config.Cwd != "" {
+	// Change working directory AFTER the user has been set up, if we haven't done it yet.
+	if doChdir {
 		if err := unix.Chdir(config.Cwd); err != nil {
 			return fmt.Errorf("chdir to cwd (%q) set in config.json failed: %v", config.Cwd, err)
 		}
diff --git a/tests/integration/cwd.bats b/tests/integration/cwd.bats
index 784c70bc92c..114efb99153 100644
--- a/tests/integration/cwd.bats
+++ b/tests/integration/cwd.bats
@@ -30,7 +30,7 @@ function teardown() {
 
 # Verify a cwd owned by the container user can be chdir'd to,
 # even if runc doesn't have the privilege to do so.
-@test "runc create sets up user before chdir to cwd" {
+@test "runc create sets up user before chdir to cwd if needed" {
 	requires rootless rootless_idmap
 
 	# Some setup for this test (AUX_DIR and AUX_UID) is done
@@ -56,3 +56,19 @@ function teardown() {
 	runc run test_busybox
 	[ "$status" -eq 0 ]
 }
+
+# Verify a cwd not owned by the container user can be chdir'd to,
+# if runc does have the privilege to do so.
+@test "runc create can chdir if runc has access" {
+	requires root
+
+	mkdir -p rootfs/home/nonroot
+	chmod 700 rootfs/home/nonroot
+
+	update_config '	  .process.cwd = "/root"
+			| .process.user.uid = 42
+			| .process.args |= ["ls", "/tmp"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+}