specgen: improve heuristic for /sys bind mount

partially revert 95c4577 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 <[email protected]>
containers · Jan 15, 2021 · 2c328a4 · 2c328a4
1 parent 3fcf346
commit 2c328a4
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 18 deletions.
diff --git 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
@@ -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")