diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb9b1e1c6..6b594f7835 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/cmd/internal/cli/actions_linux.go b/cmd/internal/cli/actions_linux.go index 878a5d68b2..889107a6e0 100644 --- a/cmd/internal/cli/actions_linux.go +++ b/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) } diff --git a/cmd/starter/c/include/starter.h b/cmd/starter/c/include/starter.h index f58d90164a..c8f12161c8 100644 --- a/cmd/starter/c/include/starter.h +++ b/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 */ diff --git a/cmd/starter/c/starter.c b/cmd/starter/c/starter.c index 68b4f3982b..81f1708215 100644 --- a/cmd/starter/c/starter.c +++ b/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(); diff --git a/e2e/actions/actions.go b/e2e/actions/actions.go index 7b8c6c3621..6547c6ddbd 100644 --- a/e2e/actions/actions.go +++ b/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() } } diff --git a/internal/pkg/runtime/engine/config/starter/starter_linux.go b/internal/pkg/runtime/engine/config/starter/starter_linux.go index 355a106a1d..033e8ea5b8 100644 --- a/internal/pkg/runtime/engine/config/starter/starter_linux.go +++ b/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 { diff --git a/internal/pkg/runtime/engine/singularity/prepare_linux.go b/internal/pkg/runtime/engine/singularity/prepare_linux.go index 11d5ca8ef8..9c41aeb585 100644 --- a/internal/pkg/runtime/engine/singularity/prepare_linux.go +++ b/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 } diff --git a/internal/pkg/util/starter/starter.go b/internal/pkg/util/starter/starter.go index 3af597e15e..73edcf9c9d 100644 --- a/internal/pkg/util/starter/starter.go +++ b/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