From 247969fe0854c8f9fc14e0e9786950611c642629 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 27 Feb 2023 15:40:38 -0800
Subject: [PATCH] Fix runc run "permission denied" when rootless

Since commit 957d97bcf43f41beef96 was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: #3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] https://github.com/opencontainers/runc/issues/3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] https://github.com/opencontainers/runc/issues/3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/eaccess_go119.go       | 14 ++++++++++++++
 libcontainer/eaccess_stub.go        |  7 +++++++
 libcontainer/standard_init_linux.go | 11 ++++++-----
 libcontainer/system/linux.go        | 19 -------------------
 4 files changed, 27 insertions(+), 24 deletions(-)
 create mode 100644 libcontainer/eaccess_go119.go
 create mode 100644 libcontainer/eaccess_stub.go

diff --git a/libcontainer/eaccess_go119.go b/libcontainer/eaccess_go119.go
new file mode 100644
index 00000000000..d7ecf5734a6
--- /dev/null
+++ b/libcontainer/eaccess_go119.go
@@ -0,0 +1,14 @@
+//go:build !go1.20
+
+package libcontainer
+
+func eaccess(path string) error {
+	// This check is similar to access(2) with X_OK except for
+	// setuid/setgid binaries where it checks against the effective
+	// (rather than real) uid and gid. It is not needed in go 1.20
+	// and beyond and will be removed later.
+
+	// Relies on code added in https://go-review.googlesource.com/c/sys/+/468877
+	// and older CLs linked from there.
+	return unix.Faccessat(unix.AT_FDCWD, path, unix.X_OK, unix.AT_EACCESS)
+}
diff --git a/libcontainer/eaccess_stub.go b/libcontainer/eaccess_stub.go
new file mode 100644
index 00000000000..2e815447edf
--- /dev/null
+++ b/libcontainer/eaccess_stub.go
@@ -0,0 +1,7 @@
+//go:build go1.20
+
+package libcontainer
+
+func eaccess(path string) error {
+	return nil
+}
diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go
index 6ad25c9a4df..32de78eb20d 100644
--- a/libcontainer/standard_init_linux.go
+++ b/libcontainer/standard_init_linux.go
@@ -198,11 +198,12 @@ func (l *linuxStandardInit) Init() error {
 	if err != nil {
 		return err
 	}
-	// exec.LookPath might return no error for an executable residing on a
-	// file system mounted with noexec flag, so perform this extra check
-	// now while we can still return a proper error.
-	if err := system.Eaccess(name); err != nil {
-		return &os.PathError{Op: "exec", Path: name, Err: err}
+	// exec.LookPath in Go < 1.20 might return no error for an executable
+	// residing on a file system mounted with noexec flag, so perform this
+	// extra check now while we can still return a proper error.
+	// TODO: remove this once go < 1.20 is not supported.
+	if err := eaccess(name); err != nil {
+		return &os.PathError{Op: "eaccess", Path: name, Err: err}
 	}
 
 	// Set seccomp as close to execve as possible, so as few syscalls take
diff --git a/libcontainer/system/linux.go b/libcontainer/system/linux.go
index 039059a444c..e1d6eb18034 100644
--- a/libcontainer/system/linux.go
+++ b/libcontainer/system/linux.go
@@ -31,25 +31,6 @@ func (p ParentDeathSignal) Set() error {
 	return SetParentDeathSignal(uintptr(p))
 }
 
-// Eaccess is similar to unix.Access except for setuid/setgid binaries
-// it checks against the effective (rather than real) uid and gid.
-func Eaccess(path string) error {
-	err := unix.Faccessat2(unix.AT_FDCWD, path, unix.X_OK, unix.AT_EACCESS)
-	if err != unix.ENOSYS && err != unix.EPERM { //nolint:errorlint // unix errors are bare
-		return err
-	}
-
-	// Faccessat2() not available; check if we are a set[ug]id binary.
-	if os.Getuid() == os.Geteuid() && os.Getgid() == os.Getegid() {
-		// For a non-set[ug]id binary, use access(2).
-		return unix.Access(path, unix.X_OK)
-	}
-
-	// For a setuid/setgid binary, there is no fallback way
-	// so assume we can execute the binary.
-	return nil
-}
-
 func Execv(cmd string, args []string, env []string) error {
 	name, err := exec.LookPath(cmd)
 	if err != nil {