From 2c328a4ac12262771861b2be6522acbfa5bbadb6 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscrivan@redhat.com>
Date: Tue, 12 Jan 2021 16:08:14 +0100
Subject: [PATCH] specgen: improve heuristic for /sys bind mount

partially revert 95c45773d7dbca2880152de681c81f0a2afec99b

restrict the cases where /sys is bind mounted from the host.

The heuristic doesn't detect all the cases where the bind mount is not
necessary, but it is an improvement on the previous version where /sys
was always bind mounted for rootless containers unless --net none was
specified.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
---
 pkg/specgen/generate/oci.go | 39 ++++++++++++++++++++-----------------
 test/e2e/run_ns_test.go     |  8 ++++++++
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go
index 7dc32a3145..e621312447 100644
--- a/pkg/specgen/generate/oci.go
+++ b/pkg/specgen/generate/oci.go
@@ -138,10 +138,23 @@ func makeCommand(ctx context.Context, s *specgen.SpecGenerator, img *image.Image
 	return finalCommand, nil
 }
 
+// canMountSys is a best-effort heuristic to detect whether mounting a new sysfs is permitted in the container
+func canMountSys(isRootless, isNewUserns bool, s *specgen.SpecGenerator) bool {
+	if s.NetNS.IsHost() && (isRootless || isNewUserns) {
+		return false
+	}
+	if isNewUserns {
+		switch s.NetNS.NSMode {
+		case specgen.Slirp, specgen.Private, specgen.NoNetwork, specgen.Bridge:
+			return true
+		default:
+			return false
+		}
+	}
+	return true
+}
+
 func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, rtc *config.Config, newImage *image.Image, mounts []spec.Mount, pod *libpod.Pod, finalCmd []string) (*spec.Spec, error) {
-	var (
-		inUserNS bool
-	)
 	cgroupPerm := "ro"
 	g, err := generate.New("linux")
 	if err != nil {
@@ -151,23 +164,11 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	g.RemoveMount("/dev/shm")
 	g.HostSpecific = true
 	addCgroup := true
-	canMountSys := true
 
 	isRootless := rootless.IsRootless()
-	if isRootless {
-		inUserNS = true
-	}
-	if !s.UserNS.IsHost() {
-		if s.UserNS.IsContainer() || s.UserNS.IsPath() {
-			inUserNS = true
-		}
-		if s.UserNS.IsPrivate() {
-			inUserNS = true
-		}
-	}
-	if inUserNS && s.NetNS.NSMode != specgen.NoNetwork {
-		canMountSys = false
-	}
+	isNewUserns := s.UserNS.IsContainer() || s.UserNS.IsPath() || s.UserNS.IsPrivate()
+
+	canMountSys := canMountSys(isRootless, isNewUserns, s)
 
 	if s.Privileged && canMountSys {
 		cgroupPerm = "rw"
@@ -232,6 +233,8 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 		g.AddMount(devPts)
 	}
 
+	inUserNS := isRootless || isNewUserns
+
 	if inUserNS && s.IpcNS.IsHost() {
 		g.RemoveMount("/dev/mqueue")
 		devMqueue := spec.Mount{
diff --git a/test/e2e/run_ns_test.go b/test/e2e/run_ns_test.go
index 51657cb1e5..29d2d43954 100644
--- a/test/e2e/run_ns_test.go
+++ b/test/e2e/run_ns_test.go
@@ -105,6 +105,14 @@ var _ = Describe("Podman run ns", func() {
 		Expect(session).To(ExitWithError())
 	})
 
+	It("podman run mounts fresh cgroup", func() {
+		session := podmanTest.Podman([]string{"run", fedoraMinimal, "grep", "cgroup", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		output := session.OutputToString()
+		Expect(output).ToNot(ContainSubstring(".."))
+	})
+
 	It("podman run --ipc=host --pid=host", func() {
 		SkipIfRootlessCgroupsV1("Not supported for rootless + CGroupsV1")
 		cmd := exec.Command("ls", "-l", "/proc/self/ns/pid")