fix: Pass CleanupHost boolean to starter correctly

CHANGELOG.md

@@ -1,5 +1,11 @@
 # SingularityCE Changelog
 
+## Changes Since Last Release
+
+### Bug Fixes
+
+- Correctly launch CleanupHost process only when needed in `--sif-fuse` flow.
+
 ## 3.10.0-rc.1 \[2022-05-04\]
 
 This is the first release candidate for the upcoming SingularityCE 3.10 release.

cmd/internal/cli/actions_linux.go

@@ -791,6 +791,10 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri
 		}
 	}
 
+	if SIFFUSE && !(UserNamespace || insideUserNs) {
+		sylog.Warningf("--sif-fuse is not supported without user namespace, ignoring.")
+	}
+
 	// setuid workflow set RLIMIT_STACK to its default value,
 	// get the original value to restore it before executing
 	// container process
@@ -835,6 +839,7 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri
 			starter.WithStdout(stdout),
 			starter.WithStderr(stderr),
 			starter.LoadOverlayModule(loadOverlay),
+			starter.CleanupHost(engineConfig.GetImageFuse()),
 		)
 
 		if sylog.GetLevel() != 0 {
@@ -866,6 +871,7 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri
 			cfg,
 			starter.UseSuid(useSuid),
 			starter.LoadOverlayModule(loadOverlay),
+			starter.CleanupHost(engineConfig.GetImageFuse()),
 		)
 		sylog.Fatalf("%s", err)
 	}

cmd/starter/c/include/starter.h

@@ -161,9 +161,6 @@ struct starter {
 
     /* bounding capability set will include caps needed by nvidia-container-cli */
     bool nvCCLICaps;
-
-    /* is a CLEANUP_HOST process require outside of namespaces for SIF FUSE cleanup */
-    bool cleanupHost;
 };
 
 /* engine configuration */

cmd/starter/c/starter.c

@@ -1291,17 +1291,13 @@ __attribute__((constructor)) static void init(void) {
     /* retrieve engine configuration from environment variables */
     read_engine_config(&sconfig->engine);
 
-    /* cleanup environment variables */
-    cleanenv();
-
-    /* fix I/O streams to point to /dev/null if they are closed */
-    fix_streams();
-
-    /* set an invalid value for check */
-    sconfig->starter.workingDirectoryFd = -1;
-
     // Unpriv host cleanup in calling namespaces for SIF FUSE mount
-    if ( !sconfig->starter.cleanupHost ) {   
+    if ( getenv("CLEANUP_HOST") != NULL ) {
+        // FUSE SIF mount isn't supported in setuid flow at present.
+        // We should never have a CleanupHost process in setuid mode - enforce this.
+        if ( sconfig->starter.isSuid ) {
+           fatalf("CleanupHost process requested in setuid mode. Not permitted.\n");
+        }
         debugf("Create socketpair for cleanup communication channel\n");
             if ( socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, cleanup_socket) < 0 ) {
                 fatalf("Failed to create communication socket: %s\n", strerror(errno));
@@ -1315,12 +1311,23 @@ __attribute__((constructor)) static void init(void) {
             verbosef("Spawn CleanupHost\n");
             goexecute = CLEANUP_HOST;
             return;
-        }else{
+        } else {
             // In master - Close child end of cleanup socket
             close(cleanup_socket[1]);
         }
+    } else {
+        debugf("CleanupHost not requested\n");
     }
 
+    /* cleanup environment variables */
+    cleanenv();
+
+    /* fix I/O streams to point to /dev/null if they are closed */
+    fix_streams();
+
+    /* set an invalid value for check */
+    sconfig->starter.workingDirectoryFd = -1;
+
     /* save opened file descriptors that won't be closed when stage 1 exits */
     master_fds = list_fd();
 

e2e/actions/actions.go

@@ -2352,6 +2352,30 @@ func (c actionTests) actionSIFFUSE(t *testing.T) {
 	}
 }
 
+// Verify that the FUSE mounts, and the CleanupHost() process are not seen when
+// --sif-fuse should not be in effect.
+func (c actionTests) actionNoSIFFUSE(t *testing.T) {
+	e2e.EnsureImage(t, c.env)
+
+	profiles := []e2e.Profile{e2e.UserProfile, e2e.RootProfile, e2e.FakerootProfile, e2e.UserNamespaceProfile}
+
+	for _, p := range profiles {
+		c.env.RunSingularity(
+			t,
+			e2e.AsSubtest(p.String()),
+			e2e.WithProfile(p),
+			e2e.WithCommand("exec"),
+			e2e.WithGlobalOptions("-d"),
+			e2e.WithArgs(c.env.ImagePath, "mount"),
+			e2e.ExpectExit(
+				0,
+				e2e.ExpectError(e2e.UnwantedContainMatch, "squashfuse"),
+				e2e.ExpectError(e2e.UnwantedContainMatch, "CleanupHost()"),
+			),
+		)
+	}
+}
+
 func countSquashfuseMounts(t *testing.T) int {
 	count := 0
 
@@ -2413,5 +2437,6 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests {
 		"compat":                c.actionCompat,        // test --compat
 		"invalidRemote":         np(c.invalidRemote),   // GHSA-5mv9-q7fq-9394
 		"SIFFUSE":               np(c.actionSIFFUSE),   // test --sif-fuse
+		"NoSIFFUSE":             np(c.actionNoSIFFUSE), // test absence of squashfs and CleanupHost()
 	}
 }

internal/pkg/runtime/engine/config/starter/starter_linux.go

@@ -200,17 +200,6 @@ func (c *Config) SetAllowSetgroups(allow bool) {
 	}
 }
 
-// SetCleanupHost sets the flag to tell starter container setup
-// to spawn a cleanup process for SIF FUSE mounts early, in the
-// original host namespaces.
-func (c *Config) SetCleanupHost(enabled bool) {
-	if enabled {
-		c.config.starter.cleanupHost = C.true
-	} else {
-		c.config.starter.cleanupHost = C.false
-	}
-}
-
 // GetJSONConfig returns pointer to the engine's JSON configuration.
 // A copy of the original bytes allocated on C heap is returned.
 func (c *Config) GetJSONConfig() []byte {

internal/pkg/runtime/engine/singularity/prepare_linux.go

@@ -195,10 +195,6 @@ func (e *EngineOperations) PrepareConfig(starterConfig *starter.Config) error {
 	if e.EngineConfig.GetNvCCLI() {
 		starterConfig.SetNvCCLICaps(true)
 	}
-	// If using a SIF FUSE mount, we need to cleanup in host namespaces.
-	if e.EngineConfig.GetImageFuse() {
-		starterConfig.SetCleanupHost(true)
-	}
 
 	return nil
 }

internal/pkg/util/starter/starter.go

@@ -74,6 +74,16 @@ func LoadOverlayModule(load bool) CommandOp {
 	}
 }
 
+// CleanupHost sets CLEANUP_HOST environment variable
+// which telsl starter to spawn the unprivileged host cleanup process.
+func CleanupHost(spawn bool) CommandOp {
+	return func(c *Command) {
+		if spawn {
+			c.env = append(c.env, "CLEANUP_HOST=1")
+		}
+	}
+}
+
 // Command a starter command to execute.
 type Command struct {
 	path   string